8

Is it bad practice to add false or ... or true and ... for the sake of promoting code genericness and/or ease of use?

As in:

SELECT *
FROM table
WHERE TRUE
  AND IsEnabled
  AND SomeField = some_value
  AND SomeOtherField != some_value

in SQL or in javascript for example:

if (false
      || prop === 'category'
      || prop === 'subCategory'
      || prop === 'productLogisticGroup'
      || prop === 'productName'
      || prop === 'color'
      || prop === 'configuration'
)

This lets adding or removing conditions a little easier by removing the thinking process whether the condition is on the edge or not. Is this bad in someway?

11
  • 5
    Aside: I would change the second example to if (set(...).contains(prop))
    – Caleth
    Commented May 5, 2020 at 11:04
  • 11
    Any code whose intention requires explanation about why you're doing it to someone reading or reviewing it the code seems undesirable to me. You've mentioned that your intention is to make the code more generic and easier to use, but even with that explanation I have to say I still don't understand what the point is, and honestly can't see how it makes the code more generic or easier to use. Maybe I'm just not smart enough to understand, but then it's often a good idea to assume your audience isn't very smart and keep in mind the Principle of least astonishment. Commented May 5, 2020 at 11:17
  • 1
    In fact, I'd say if your only reason for doing that is to communicate to other developers, then a far more direct and less cryptic way of doing that would be to add a comment which lays out your intention explicitly - for example /* Add new conditions here */ Commented May 5, 2020 at 11:21
  • 3
    I would not recommend it. It always provokes a WTF by the next maintenance programmer.
    – Doc Brown
    Commented May 5, 2020 at 12:03
  • 2
    For SQL alignment and debugging purposes it's also common to write WHERE 1 = 1 ..., WHERE 1 = 0 ... respectively. Commented May 5, 2020 at 12:31

5 Answers 5

8

There are some useful corner cases but your examples do not look particularly meaningful to me. I use it mostly for debugging, if I temporarily want to switch of a block or I want to force myself into some block. I also use it for continuous loops like thread methods where the break condition is not that simple or the break condition depends on an internal calculation rather than an external boolean. But then the true or false is actually significant. In your examples they are purely esthetic and are likely to raise some eye brows if someone else reads your code.

It won't hurt much though, if this is how you roll, more power to you.

3

I think the compiler will usually remove it, so it's doing no harm. I don't think it's adding anything useful either though.

However, most of the time when I've seen this is when someone is building up a SQL query string at run time, and that is usually bad practice. It's easy to introduce bugs (or SQL injection vulnerabilities) and hard to test or debug.

FWIW, here's how I'd write your examples:

SELECT *
FROM table
WHERE IsEnabled
  AND SomeField = some_value
  AND SomeOtherField != some_value


switch (prop)
{
     case 'category';
     case 'subCategory';
     case 'productLogisticGroup';
     case 'productName';
     case 'color';
     case 'configuration';
         DoTheThing();
         Break;
}
3

If you're writing this code yourself, then no; I wouldn't recommend it.

However, if you're writing another piece of code that will generate this code, then it might be more acceptable. An example of this might be a SQL Query builder that produces SQL based on the given User inputs. Including the leading, fixed condition means that your code only has to add the "extra" bits, as supplied by the User. Without the leading, fixed bit, you'd have to do [a tiny bit] more, to set up the syntax correctly.
(Actually, in your first example, you could probably drop the TRUE clause because, I suspect, the "IsEnabled" check will always be there and is just as boog a place to start the WHERE clause).

1

I've seen one situation where you actually (kind of) need to add a true or false: In Swift, you can declare conditionals on the command line. But if you write

if condition {
    statement
}

and "condition" is a false constant, you get a warning that the code will never be executed. Which is useful because the developer might not be aware. Change it to

if false || condition {
    statement
}

the warning goes away. Apparently the compiler assumes because of the "false || " that you know what you are doing, and that you are aware that the statement might be dead code, and removes the warning. Clang does that in other situations, for example in C or C++

if (false) {
    statement;
}

gives a warning, but

if ((false)) {
    statement;
}

doesn't. So warnings are generated not just depending on the semantics of a statement, but also depending on how you write it.

0

I'd tend to write

if (   prop === 'category'
    || prop === 'subCategory'
    || prop === 'productLogisticGroup'
    || prop === 'productName'
    || prop === 'color'
    || prop === 'configuration')

because Xcode handles it well, and because you can delete every line easily. First and last line ar slightly trickier.

3
  • Every except first and last
    – RiaD
    Commented May 5, 2020 at 21:14
  • This is exactly the point though, I'm trying to get rid of the little hassle of first and /or last line replacements.
    – Nae
    Commented May 6, 2020 at 13:05
  • First and last line are only a tiny bit more difficult: Instead of selecting a whole line you select for example from just before the first "prop" to just before the second "prop".
    – gnasher729
    Commented Mar 1, 2022 at 9:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.