-2
\$\begingroup\$

I know there's a more compact way of doing the following code but I haven't been coding PHP in a while and I completely forgot how. I'm pretty sure it's to do with : and ?'s but I'm unsure.

Basically, if $bind has a value add $bind inside of execute() otherwise just don't.

//Execute Binded Query;
if($bind != false) {
    $query->execute($bind);
} else {
    $query->execute();
}
\$\endgroup\$
5
  • \$\begingroup\$ What about $query->execute ($bind ? $bind : array ()); \$\endgroup\$
    – JnxF
    Apr 6 '16 at 7:46
  • \$\begingroup\$ @JnxF Is there a way to test that without loading the page on my server as its currently down. Do you know if it works? Also what exactly is that operator called? \$\endgroup\$
    – user100593
    Apr 6 '16 at 7:51
  • \$\begingroup\$ I would normally call ?: the ternary operator, although apparently others call it the conditional operator. \$\endgroup\$
    – mdfst13
    Apr 6 '16 at 22:42
  • \$\begingroup\$ Stop editing my damn question I dont like the Title your putting and your taking out stuff I dont want you to stop. \$\endgroup\$
    – user100593
    Apr 7 '16 at 6:40
  • \$\begingroup\$ I've locked the post to avoid further unnecessary edits. Let's talk when you have a minute. \$\endgroup\$ Apr 8 '16 at 14:52
0
\$\begingroup\$

Try the ternary operator for a one-liner.

$query->execute($bind ?: array());

Expression expr1 ?: expr3 returns expr1 if expr1 evaluates to TRUE, and expr3 otherwise. So in this case, if bind is not false (thus true), it will return bind, and else empty array.

\$\endgroup\$
4
  • \$\begingroup\$ Could you explain how this works? Also can you confirm that it will work as I have no way of actually testing it for a while. \$\endgroup\$
    – user100593
    Apr 6 '16 at 8:08
  • \$\begingroup\$ @ShinyMK For the explanation of the ternary operator, check the PHP documentation. I linked to it in my answer. It's impossible for me, or anyone else for that matter, to confirm that this will work for you. There is just too little context available. However, I can tell you that the syntax is written correctly. \$\endgroup\$ Apr 6 '16 at 8:13
  • \$\begingroup\$ Ok may I ask whats difference between @JnxF's answer and yours? I read up and found "The expression (expr1) ? (expr2) : (expr3) evaluates to expr2 if expr1 evaluates to TRUE, and expr3 if expr1 evaluates to FALSE." wouldnt the correct syntax be: $bind ? $bind : array() \$\endgroup\$
    – user100593
    Apr 6 '16 at 8:20
  • 1
    \$\begingroup\$ @ShinyMK That is also explained in the documentation: "Expression expr1 ?: expr3 returns expr1 if expr1 evaluates to TRUE, and expr3 otherwise. " \$\endgroup\$ Apr 6 '16 at 8:22
3
\$\begingroup\$

I know theres a faster/easier more inline/1liner way of doing the following code

Can I and should I are two completely different things.

Can you make this a one-liner? Yes.

Should you? No.

When calling a function as a result of a condition, you are dealing with flow control. The Ternary Operator makes this harder to read. The if statement makes it obvious.

This is very clear:

if (condition) {
    doSomething();
} else {
    doSomethingElse();
}

Whaaaaaaa... (scratches head)

(condition ? doSomething() : doSomethingElse());

Using a Ternary Operator in an assignment can make things easier to read and shorter as well:

$bind = condition ? array() : $otherArray;

$query->execute($bind);

Cramming the Ternary Operator in the argument to a method call just obfuscates your code. I would recommend using the Ternary Operator in your assignment to the $bind variable, or another if statement earlier in your code and just blindly calling $query->execute($bind).

\$\endgroup\$
4
  • \$\begingroup\$ Sorry dude but In my opinion doing a one liner and learning new PHP is better then sticking with old non-optimized code. \$\endgroup\$
    – user100593
    Apr 6 '16 at 21:01
  • 2
    \$\begingroup\$ Optimized code, in most of the cases, does not imply doing things shorter, but easier to read. I.e. you care about code length when you program in assembly code (1s and 0s and all this stuf), but otherwise, your main priority is to build code that is easy to read, efficient and well-structured. Code length falls in a second level priority. \$\endgroup\$
    – JnxF
    Apr 6 '16 at 21:58
  • \$\begingroup\$ @JnxF Yes thats true but to me using If/Else's are just way too over the top for such a simple function. In my Code I never like repeating the same or some of the same code just for Conditions. \$\endgroup\$
    – user100593
    Apr 7 '16 at 6:39
  • 1
    \$\begingroup\$ @ShinyMK you are getting REALLY good advice here but don't seem to want to take it. You get no bonus points for making your code as short as possible. If a developer on my team were getting this sort of code review advice on a consistent basis and consistently ignoring it, he would not be a developer on my team for long. In a professional coding environment you do want concise code, but more importantly you want clear, understandable code that is easy to maintain. \$\endgroup\$
    – Mike Brant
    Aug 6 '16 at 3:06
1
\$\begingroup\$

As previously mentioned by @GregBurghardt, just because you can doesn't mean you should. All of his reasoning holds true, and code readability should be your primary concern when writing code. Code shortness is one of the methods you achieve this, but if the code becomes too dense it is actually detrimental to the readability. Which, in turn, will lead to more problems with bugs and inefficient coding(!).

I recommend doing it in the following manner. It will keep the repeated code to a minimum, while still ensuring that there are no warning/notices about missing variables or erroneous data. Plus it's very clear what the code does, and easier to understand why. (Comments can/should be added to further clarify why.)

if (!isset ($bind) || typeof ($bind) != 'array') {
    $bind = array ();
}
$query->execute ($bind);

What you are attempting is called "premature optimization", and is quite discouraged in programming. Not only because it has no benefit for the runtime of the code, or anything else really, but also because the high risk of making the code consume even more resources.

In this particular case the IF-construct is faster than the ternary operator. Not by much, granted, but it is still faster. Which makes the change proposed not only more difficult to read, but also contrary to the desired effect. Basically it will be neither faster nor easier, though it will be more "inline/one-liner". Though, I don't think that sacrificing both speed and simplicity is worth it to get it on one line.

Example: Which of these two methods do you think is the fastest, and why?

// Method 1:
function HK3 ($search, $string) {
    if (empty ($search)) {
        return $string;
    }
    $string = trim ($search);
    $string = preg_quote ($string);
    $string = explode (" ", $string);
    $string = array_map ("custom_highlight_string_fixer", $string)
    return preg_replace ($string, '$1<b>$2</b>', $string);
}

function custom_highlight_string_fixer ($String) {
    return "/([\\s><])($String)(?=[\\s><\\n\\r,.!?]|\\z)/i";
}

// Method 2:
function BTH_HK ($needle, $haystack) {
    if (empty($needle)) return $haystack;
    return preg_replace ('/([\\s><])(' . str_replace(' ', '|', preg_quote ($needle)) . ')(?=[\\s><\\n\\r,.!?]|\\z)/i', '$1<b>$2</b>', $haystack);
}

If we go by your initial presumption, that anything in one line is faster than something that spans multiple lines, the second method should be loads faster than the first. Even if we look at the number of functions calls, that should be true too.
In reality, however, it's the first method that is the fastest one. This is because it's written with the knowledge of how every line of code works in the inner bowels of PHP, and an understanding of how that affects the data which is fed to it. Of course, that also means that it can vary who's fastest depending upon the exact data which you feed them.

Which is the only way to properly optimize code, and why it is generally discouraged to attempt for those who are still learning.

\$\endgroup\$