Jump to content

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?

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites
Posted (edited)

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

Share this post


Link to post
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.

Share this post


Link to post
Share on other sites

Does this mean your problem is resolved?

Share this post


Link to post
Share on other sites
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...

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites
Posted (edited)

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

Edited by Karaethon

Share this post


Link to post
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;
		}
	}

 

Share this post


Link to post
Share on other sites

The last part could use some cleanup.

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

 

 

Share this post


Link to post
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".

Share this post


Link to post
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...

Share this post


Link to post
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.

Share this post


Link to post
Share on other sites
Posted (edited)
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

Share this post


Link to post
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.

 

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites

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.