CODEWITHSUNDEEP
phpmysqlpdo
Bv202
Bv202

Reputation: 4044

Selecting everything with WHERE

I'm currently working on a PHP project where multiple access rights exists. The logic is simple:
- When your access level is 2, you can see a list of your own projects only
- When your access level is 3, you can see ALL projects

So in SQL:
First one:

SELECT project_name FROM projects

Second one:

SELECT project_name FROM projects WHERE user_id = user_id

The problem is that I'm using PDO with prepared statements and such queries are executed multiple times across the script. This is how it looks like:

if ($_SESSION['access_level'] == 3) {
    $sql = "SELECT project_name FROM projects";
  } else {
    $sql = "SELECT project_name FROM projects WHERE user_id = ?";
}

$res = $db->prepare($sql);

// Some more PHP 

if ($_SESSION['access_level'] == 3)
      $res->execute();
    else
      $res->execute(array($_SESSION['user_id']));

As I'm doing this in multiple parts of the script, it becomes a mess. Is there a better way to do this? Personally I was thinking of a WHERE-clause where every record is selected. That way this would be possible at the start of the script:

if ($_SESSION['access_level'] == 3)
      $id = *;
    else
      $id = $_SESSION['user_id'];

Now querying is much easier:

$res = $db->prepare("SELECT project_name FROM projects WHERE user_id = ?");
$res->execute(array($id));

(Now it will get all records when your access level is 3, but only your own when you're only level 2)

This looks like a pretty dump solution imo as I'm not really usng the WHERE clause how it should be used. Also, using * is just not possible.

What's the best option for this?

Thank you!

Upvotes: 3

Views: 146

Answers (6)

Your Common Sense
Your Common Sense

Reputation: 157940

You are looking for the solution in completely wrong way.

Every time you face a situation where you doing somethin in multiple parts of the script, and it become a mess, You have to create a function.

In fact, you are still writing all this ugly repeated code with prepare, execute, fetch, prepare, execute, fetch, prepare, execute, fetch - for the every query on the page. Doesn't it looks like a mess for you?

So, you have to create two functions.

general purpose one, just to fetch some value out of query without repeating useless code, to use it like this:

$proj_names_arr = $db->getColumn("SELECT project_name FROM projects");

and one mentioned by Raisen, based on the first one, to be used like this

$proj_names_arr = getProjects();

It will be the only real improvement of your code

As for the function, it's not that hard
a rough example:

function getColumn() {
  $args  = func_get_args();
  $query = array_shift($args);
  $res = $db->prepare($query);
  $res->execute($args);
  $data = array();
  while ($row = $res->fetch(PDO::FETCH_NUM)) {
    $data[] = $row[0];
  }
  return $data;
}

so, it can be called 

$proj_names = $db->getColumn("SELECT project_name FROM projects WHERE user_id = ?",
                             $_SESSION['user_id']);

Upvotes: 3

anon
anon

Reputation:

As I'm doing this in multiple parts of the script, it becomes a mess.

Don't Repeat Yourself. If you find yourself repeating blocks of code, that's a sign that you should be refactoring that common code into a function or method.

You can also get rid of some of the more minor repetitions by appending extra information to your query rather than rewriting it, and using something like bindParam() to add the additional parameter, rather than modifying your execute() statement:

$userOnly = ($_SESSION['access_level'] == 2);

$sql = "SELECT project_name FROM projects";

if ($userOnly) {
    $sql .= " WHERE user_id = ?";
}

$res = $db->prepare($sql);

if ($userOnly) {
 $res=>bindParam(bindParam(1, $$_SESSION['user_id'], PDO::PARAM_INT);
}

$res->execute(array($_SESSION['user_id']));

Personally, I think that's a bit neater than the original, since the parts that only apply to the lower access level are clearly marked and self-contained, not repeating code already presented.

Also, using a new variable with a descriptive name for the access level stops you from repeating magic numbers, and gives you a single point of change.

Hope that helps.

Upvotes: 0

Nicktar
Nicktar

Reputation: 5575

What about an SQL like that:

$sql = 'SELECT project_name FROM projects WHERE user_id = ? OR 3 = ?';
$res = $db->prepare($sql);
$res->execute(array($id, $access_level);

Upvotes: 2

Raisen
Raisen

Reputation: 4495

Create a function such as getProjects() where it will return the projects according to the user level.

Use Alvaro's code for example inside the function and return the resultset.

Upvotes: 1

Álvaro González
Álvaro González

Reputation: 146588

I think it's easier to keep the where clauses in a variable, as well as the bind parameters, and then compose the query when running it:

<?php

$sql = 'SELECT project_name FROM projects';
$where = array();
$parameters = array();


if ($_SESSION['access_level'] !== 3) {
    $where[] = 'user_id = ?';
    $parameters = array($_SESSION['user_id']);
}



if( !empty($where) ){
    $sql .= ' WHERE ' . implode(', ', $where);
}
$res = $db->prepare($sql);

$res->execute($parameters);

Upvotes: 2

KingCrunch
KingCrunch

Reputation: 132051

Did I understand: You want a WHERE-Statement, thats always true?

WHERE 1 = 1

Upvotes: 0

Related Questions