[Last Call] Learn about multicloud storage options and how to improve your company's cloud strategy. Register Now

x
?
Solved

Trying to add conditional 'where' clause into query builder laravel

Posted on 2016-07-28
9
Medium Priority
?
263 Views
Last Modified: 2016-07-28
Hi,
I have  a where clause I'm trying to insert that is conditional. I get a syntax error: "unexpected 'if' (T_IF)".
Not sure how to write this?
$vehicles = DB::table('used_vehicles')
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        	if (isset(Auth::user()->dealership->name))
        	{
        		$vehicles->where('used_vehicles.dealer_id', Auth::user()->dealership_id);
        	}
       	->where('used_vehicles.archived', 0)
        ->orderBy( 'added', 'DESC') 
	->get();

Open in new window

0
Comment
Question by:tjyoung
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 4
  • 2
  • 2
  • +1
9 Comments
 
LVL 59

Expert Comment

by:Julian Hansen
ID: 41732843
You can't put an if in a chain - you can do this
$vehicles = DB::table('used_vehicles')
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id');
// Terminate the chain 
// Now do the if    
if (isset(Auth::user()->dealership->name)) {
  $vehicles->where('used_vehicles.dealer_id', Auth::user()->dealership_id);
}
// Finally do the get
$vehicles->get();

Open in new window

1
 
LVL 111

Expert Comment

by:Ray Paseur
ID: 41732848
I would write it in two completely separate calls to the DB::table() facade.  Putting the if() statement into the middle of the chained methods seems like a logically good idea, but it bumps into PHP syntax.  Maybe something like this (I don't have a Laravel instance to test, but it seems OK).
/**
 * IF THE DEALERSHIP IS LOGGED IN
 */
if (isset(Auth::user()->dealership->name))
{
    $vehicles = DB::table('used_vehicles')
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        
        // ADDITIONAL WHERE CLAUSE TO RESTRICT RESULTS ACCORDING TO DEALERSHIP ID
        ->where('used_vehicles.dealer_id', Auth::user()->dealership_id)
        ->where('used_vehicles.archived', 0)
        ->orderBy( 'added', 'DESC') 
    ->get();
}

/**
 * IF THE DEALERSHIP IS _NOT_ LOGGED IN
 */
else
{
    $vehicles = DB::table('used_vehicles')
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        
        // NO ADDITIONAL WHERE CLAUSE
        ->where('used_vehicles.archived', 0)
        ->orderBy( 'added', 'DESC') 
    ->get();
}

Open in new window

0
 
LVL 59

Expert Comment

by:Julian Hansen
ID: 41732858
@Ray - I see where you are going and under less complex conditions I would agree with you - however I see a potential for time bomb in your implementation.

If the structure of the query changes in the future - you now have two points of change to the query logic - forget to update one or update it incorrectly and you have inconsistent results.

In this case the two conditions are mostly the same - so I would keep like code common and simply add the where in based on the condition.
0
Fill in the form and get your FREE NFR key NOW!

Veeam® is happy to provide a FREE NFR server license to certified engineers, trainers, and bloggers.  It allows for the non‑production use of Veeam Agent for Microsoft Windows. This license is valid for five workstations and two servers.

 
LVL 111

Expert Comment

by:Ray Paseur
ID: 41732870
@Julian: I certainly take your point.  I was thinking about this as I was writing the if() statement.  If() adds an element of cyclomatic complexity that makes it necessary to keep the unit tests organized in parallel, too.  I was assuming, maybe incorrectly, that this would be the only place in the script that sets $vehicles.  If so, a single if() doesn't seem too problematic.  If this is not the only code point to set $vehicles, it certainly argues for refactoring this DB call out to a common code block.

My instinct would be to keep the logged-in experience completely separate from the not-logged-in experience.  But without seeing the rest of the application, I'm not sure whether the addition of an if() statement represents a step toward spaghetti code, or just a blip in an otherwise well-structured application.

It's not 100% clear to me whether the order of chained methods matters to the execution of the query.  Would be nice if we had that freedom of movement!
https://laravel.com/docs/5.1/queries
0
 
LVL 7

Expert Comment

by:Gauthier
ID: 41732902
Julian answer is good, he just forget 2 lines before the get()
if you get a lot of these conditional, it may be worth to write a scope function to the UsedVehicles Model.

function scopeAuthDealer($query) {
    if (isset(Auth::user()->dealership->name)) {
        $query->where('dealer_id', Auth::user()->dealership_id);
    }
    return $query;
}

Open in new window


which you can use in the query chain as:
...->authDealer()->...
0
 
LVL 59

Accepted Solution

by:
Julian Hansen earned 2000 total points
ID: 41732927
@Gauthier - nice catch

My Laravel is a bit rusty but I seem to remember you can pass an Array to the where() method - (assuming you are AND'ing your where clauses) in which case you could do this
$vehicles = DB::table('used_vehicles')

// Construct the Where Clause array
// Using conditional to add additional
// AND clause

$whereClause = array(
  'used_vehicles.archived' => 0,
);
if (isset(Auth::user()->dealership->name)) {
   $whereClause['used_vehicles.dealer_id'] = Auth::user()->dealership_id;
}
// Now proceed as before
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        ->where($whereClause)
        ->orderBy( 'added', 'DESC') 
  ->get();

Open in new window

0
 
LVL 1

Author Comment

by:tjyoung
ID: 41732967
Hi,
Trying with Julian's code above but getting:

FatalErrorException in InventoryController.php line 30: syntax error, unexpected '$whereClause' (T_VARIABLE)

My exact query:
	public function readAll(ReadAllInventoryRequest $request)
	{

		$vehicles = DB::table('used_vehicles')
	
		$whereClause = array(
			'used_vehicles.archived' => 0,
		);

		if (isset(Auth::user()->dealership->name)) {
			$whereClause['used_vehicles.dealer_id'] = Auth::user()->dealership_id;
		}
	
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        ->where($whereClause)
        ->orderBy( 'added', 'DESC') 
		->get();
			
		return response()->json($vehicles, 200);
	}

Open in new window

0
 
LVL 1

Author Comment

by:tjyoung
ID: 41732979
Hi,
Just moved the condition before the query and all is fine:

	public function readAll(ReadAllInventoryRequest $request)
	{
	
		$whereClause = array(
			'used_vehicles.archived' => 0,
		);

		if (isset(Auth::user()->dealership->name)) {
			$whereClause['used_vehicles.dealer_id'] = Auth::user()->dealership_id;
		}
		

		$vehicles = DB::table('used_vehicles')
		->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        ->where($whereClause)
        ->orderBy( 'added', 'DESC') 
		->get();
			
		return response()->json($vehicles, 200);
	}

Open in new window

0
 
LVL 59

Expert Comment

by:Julian Hansen
ID: 41732993
Apologies - typo on my  part - cut and paste was supposed to go before the chain. EE not the best code editor ...
0

Featured Post

Efficient way to get backups off site to Azure

This user guide provides instructions on how to deploy and configure both a StoneFly Scale Out NAS Enterprise Cloud Drive virtual machine and Veeam Cloud Connect in the Microsoft Azure Cloud.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

When table data gets too large to manage or queries take too long to execute the solution is often to buy bigger hardware or assign more CPUs and memory resources to the machine to solve the problem. However, the best, cheapest and most effective so…
Many old projects have bad code, but the budget doesn't exist to rewrite the codebase. You can update this code to be safer by introducing contemporary input validation, sanitation, and safer database queries.
Learn how to match and substitute tagged data using PHP regular expressions. Demonstrated on Windows 7, but also applies to other operating systems. Demonstrated technique applies to PHP (all versions) and Firefox, but very similar techniques will w…
In this video, Percona Solution Engineer Rick Golba discuss how (and why) you implement high availability in a database environment. To discuss how Percona Consulting can help with your design and architecture needs for your database and infrastr…
Suggested Courses

650 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question