Jump to content

Foreach error?


Karaethon

Recommended Posts

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?

Link to comment
Share on other sites

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 by Barand
Link to comment
Share on other sites

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.

Link to comment
Share on other sites

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;
		}
	}

 

Link to comment
Share on other sites

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".

Link to comment
Share on other sites

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...

Link to comment
Share on other sites

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.

Link to comment
Share on other sites

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 by Karaethon
Link to comment
Share on other sites

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.

 

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.