3
\$\begingroup\$

This is a function to determine if a number is between x and y. It is part of a library of mine called EOL or "Everything in One Line".

#include <stdbool.h>

#ifndef _WINDOWS_H
    #include <windows.h>
#endif

bool between (WORD n, WORD x, WORD y) { return (n >= x && n <= y) ? (true) : (false); }
\$\endgroup\$
  • 1
    \$\begingroup\$ For what it's worth, EOL is a common abbreviation of end of line, such as \n in Unix and \r\n in windows... \$\endgroup\$ – Stop ongoing harm to Monica Jan 17 '15 at 16:24
  • 1
    \$\begingroup\$ Exactly. That's the irony plus the relationship between "Every function ends in one line" and "End of Line" \$\endgroup\$ – Edenia Jan 17 '15 at 16:26
7
\$\begingroup\$

There's no need to use the conditional operator here. This would be enough:

bool between (WORD n, WORD x, WORD y) { 
    return n >= x && n <= y;
}

Other than this, the variable names are not descriptive. It is not clear that x specifies the lower bound and y the higher bound.

bool between (WORD value, WORD low, WORD high) { 
    return value >= low && value <= high;
}

Additionally, I have to question a little bit the usage for a "Everything in One Line" library. If the code that you are writing is trivial one-liners, it might be easier for other programmers to quickly re-write the actual code than to use your library. Most of the times I could use a between function, I would probably write the actual code in-line, without using an additional function for it at all. However, writing a "Everything in One Line"-library can of course be an educating experience for yourself.

\$\endgroup\$
  • 2
    \$\begingroup\$ It would be slightly better to arrange the comparisons as low <= value && value <= high \$\endgroup\$ – Stop ongoing harm to Monica Jan 17 '15 at 16:21
  • \$\begingroup\$ Just imagine the phrase "20 lines of 20 useful functions". How isn't that good personal experience? Programmers must not only watch for readability but also stare to eliminate pointless lines and gain the ability to shrink the code. Oh wait.. that's still the "readability" \$\endgroup\$ – Edenia Jan 17 '15 at 16:23
  • \$\begingroup\$ It might be interesting to read but certainly not profitable to use. \$\endgroup\$ – itsbruce Jan 17 '15 at 21:13
  • \$\begingroup\$ @janos That's something I'd consider opinion based, I think value >= low is better, personally. \$\endgroup\$ – Simon Forsberg Jan 17 '15 at 22:58
4
\$\begingroup\$

Strictly speaking, you only need to #include <windef.h> here, though including all of <windows.h> is harmless. The #ifndef shouldn't be necessary in either case, as each included header should be responsible for guarding against its own double-inclusion.

\$\endgroup\$
3
\$\begingroup\$

The parentheses around the true and false are superfluous. It makes no difference to the compiler and doesn't really improve the readability here. It may be warranted if the expressions were somewhat more complex.

\$\endgroup\$
3
\$\begingroup\$

There is no reason to include windows.h at all, as there is nothing Windows-specific in this code (aside from the WORD type, which is not really a good way to do things in this day and age).

Doing such things as macros is dangerous and unnecessary; parameters aren't necessarily single values without side effects. And if it's in a header or if your compiler supports link-time optimization, the compiler can optimize it into an inline anyway. The only thing a macro buys you in this case is type-independence but it's not doing it in a particularly safe way here, and you're specifying the type in the macro so you lose that as well.

You might want to improve the condition to allow for the extents to be in any order, e.g.:

static bool between(int n, int x, int y) {
    return (x <= n && n <= y) || (y <= n && n <= x);
}

(static bool is if this implementation is kept in a header, so that non-optimizing compilers won't have multiple instances of the same symbol at link time.)

And, finally, for something so simple and obvious as this, it's not really necessary to put into a library.

\$\endgroup\$
2
\$\begingroup\$
between (WORD n, WORD x, WORD y)

In languages that have such a comparison built in, the order of operands would be x <= n <= y. Is there a reason you chose NXY instead of XNY for your parameter order?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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