Foreach error?

[Karaethon]

I have the following function:

public function getData($criteria){
	$query = "SELECT * FROM {$this->table} WHERE ";
	$crit = "";
	foreach($criteria as $column => $value){
		if($column = 'bind'){
			$crit .= "{$value} ";
		}else{
			$crit .= "{$column} = {$value} ";
		}
	}
	echo $crit;
}

I'm testing it with 

getData(['test1'=>'test2', 'bind'=>'AND', 'test3'=>'test4']);

But instead of getting "test1 = test2 AND test3 = test4" the echo output is "test2 AND test4". I'm probably too close to see whats wrong, what do you see?

[Karaethon]

Argh! I see it now. i forgot = assigns and == compares!

[Barand]

That's only one of the problems.

• What is "$query = ... " there for? You don't use it.
• "test2" and "test4" are string values so should be quoted in the WHERE clause. (test1 = 'test2' AND test3 = 'test4'). Yours is only correct for numeric values.
• Your method should return the result, not echo it.
• [edit] You will probably hit further problems if you have a mix of bind values (AND/OR)

 

Edited June 28, 2019 by Barand

[Karaethon]

11 hours ago, Barand said:

That's only one of the problems.

• What is "$query = ... " there for? You don't use it.
• "test2" and "test4" are string values so should be quoted in the WHERE clause. (test1 = 'test2' AND test3 = 'test4'). Yours is only correct for numeric values.
• Your method should return the result, not echo it.
• [edit] You will probably hit further problems if you have a mix of bind values (AND/OR)

 

Yeah this was early testing, the final code doesnt have most of that. The $query is used as a mysql query, I just hadnt added that yet (this was testing to make sure foreach output was correct) the foreach is working fine now.

[ginerjm]

Does this mean your problem is resolved?

[Karaethon]

3 minutes ago, ginerjm said:

Does this mean your problem is resolved?

Yup, I noticed almost right after posting what was wrong. I asked an admin to remove it but...

[ginerjm]

But - as Barand noted - there were so many other things plain wrong with your code.  Wrong!

[Karaethon]

yes but that wasnt final code just quick test code. the things Barand pointed out are not in final code

Edited June 28, 2019 by Karaethon

[Karaethon]

Final code is:

public function getData($criteria){
		$query = "SELECT * FROM {$this->table} WHERE ";
		$crit = "";
		foreach($criteria as $column => $value){
			if(($column === 'bind')){
				$crit .= "{$value} ";
			}else{
				$crit .= "{$column} = {$value} ";
			}
		}
		$result = mysqli_query($this->link, $query.$crit);
		if(mysqli_num_rows($result) !== 0){
			return $result;
		}else{
			return false;
		}
	}

 

[benanamen]

The last part could use some cleanup.

if (mysqli_num_rows($result)) {
    return $result;
}
return false;

 

 

[requinix]

22 minutes ago, Karaethon said:

Yup, I noticed almost right after posting what was wrong. I asked an admin to remove it but...

...but look at what happens when answered threads don't get removed: the discussion continues. We don't close threads just because OP got their answer - we're not StackOverflow. You never know what else might arise after the problem is supposedly "solved".

[Karaethon]

15 minutes ago, requinix said:

...but look at what happens when answered threads don't get removed: the discussion continues. We don't close threads just because OP got their answer - we're not StackOverflow. You never know what else might arise after the problem is supposedly "solved".

Yeah, i see thst, i just felt a little stupid for not seeing it and wasting forum space and everyone elses time.

16 minutes ago, benanamen said:

The last part could use some cleanup.

if (mysqli_num_rows($result)) {
    return $result;
}
return false;

 

 

Do you have a suggestion? I know I could just return $result...

[Barand]

You can't really call it "final" until it's been tested.

With careful construction of your test data, avoiding string values, empty $criteria arrays and queries that return no records (still valid), it should be fine and let you sleep at night with the feeling you've done a good job.

[Karaethon]

4 minutes ago, Barand said:

You can't really call it "final" until it's been tested.

With careful construction of your test data, avoiding string values, empty $criteria arrays and queries that return no records (still valid), it should be fine and let you sleep at night with the feeling you've done a good job.

well yeah i guess final isnt the right word, the original code was used in a testing file not the final file, I test pieces separately first to make sure they are doing what I expect then move them to my working file which becomes the final after completion and testing.

Edited June 28, 2019 by Karaethon

[ginerjm]

I suggest that you add a line to your code at the end of the function that builds that query statement and echo it out.  See what you are actually building and then correct it.  As was already pointed out to you you have to add quotes around string values in a query and you have ignored that advice.

 

[Karaethon]

Yeah, I did once it was working. the final version of that function is in a class here where i was asking for help with a diffeent part.