Whoever wrote that code was being too clever for their own good. It's cruel and irresponsible to write code like that and hand it off. It works, but it's needlessly unclear.
There's a quote by Brian Kernighan that's important to keep in mind when you're a programmer:
"Everyone knows that debugging is twice as hard as writing a program in the first place. So if you're as clever as you can be when you write it, how will you ever debug it?"
Here's a try at code that works the same but is more clear:
function doQuery($sql)
{
$this->counter++;
if ($this->unbuffered) {
$resultmode = MYSQLI_USE_RESULT;
} else {
$resultmode = MYSQLI_STORE_RESULT;
}
$result = mysqli_query($this->db, $sql, $resultmode);
if ($result === false) {
$this->error = true;
$this->errormessage = mysqli_error($this->db);
}
return new ResultSet($result);
}
EDIT: fixed the error = true
line.
Of course I have not tested this code in your project. So if there's something I didn't notice about the way other code in the project works, I wouldn't know that. If you try this out, I recommend you keep a copy of your current code first, so if this doesn't work, you can restore the previous code easily.
Explaining some of the annoying cleverness the original code.
The ? :
operator is like an if/then/else
structure, except you can use it like an expression. In other words, you can assign the value of the expression to another variable, unlike if/then/else
.
$var = (1 == 2) ? "yes" : "no";
The way this works is that the first express (1 == 2)
is evaluated. If it's true, then the result is the expression after ?
. If the result of the first expression is false, then the result is the expression after :
.
The variable is assigned the value "no" in this example.
Frankly, although this format seems clever, the more experience I get as a programmer, the less often I like to use it. Stuffing more logic into a single expression can reduce the lines of code, but it makes the code hard to debug, hard to change, and doesn't actually make it any more speedy than if you had spelled it all out with if/then/else
structures.
This part uses another trick:
mysqli_query($this->db, $sql, MYSQLI_USE_RESULT) or $this->error = true and $this->errormessage = mysqli_error($this->db)
It relies on boolean short-circuiting which means if you write an expression like A OR B
then first A
is evaluated, and if it's true (or something like true), then B
doesn't need to be evaluated, because true OR anything
would end up being true anyway.
In this case, mysqli_query()
will return a result resource which is kind of "true" or at least it's not literally false. But if there's an error in the SQL query, then mysqli_query()
definitely returns false (it says so in the documentation). So the stuff to the right of or
in this expression will be evaluated only if mysqli_query()
has an error.
The last trick is that even though =
is an assignment (not ==
which is equality comparison), it's legitimate to use it in an expression. What this means is that the expression returns the value assigned, but the expression also has a side-effect that it has assigned values to variables.
So the code you inherited is using the ternary operator, boolean short-circuiting, and assignment side-effects inside expressions, all for the dubious goal of writing as much logic as possible in one line of code. Programmers who do this are just being show-offs, and they are revealing that they don't care about code readability or maintainability.
You also asked about this in comments below:
isset($_GET["sort"]) ? $sort = $_GET["sort"] : $sort = "name";
That also uses the ternary operator. It's the same as the following:
if (isset($_GET["sort"])) {
$sort = $_GET["sort"];
} else {
$sort = "name";
}
This example of the ternary operator could also be written this way:
$sort = isset($_GET["sort"]) ? $_GET["sort"] : "name";
Because the whole value of the ternary expression is either the second or third expression, you can assign the result once to a variable.
Finally, PHP introduced another shorthand notation because this kind of task is so common.
$sort = $_GET["sort"] ?: "name";
This means if there is a "sort"
key in the $_GET
array, then read that, but if the key is not present in the array, fall back to the value "name"
.