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); }
5 Answers 5
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.
-
2\$\begingroup\$ It would be slightly better to arrange the comparisons as
low <= value && value <= high
\$\endgroup\$janos– janos2015年01月17日 16:21:36 +00:00Commented Jan 17, 2015 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– Edenia2015年01月17日 16:23:18 +00:00Commented Jan 17, 2015 at 16:23
-
\$\begingroup\$ It might be interesting to read but certainly not profitable to use. \$\endgroup\$itsbruce– itsbruce2015年01月17日 21:13:55 +00:00Commented Jan 17, 2015 at 21:13
-
\$\begingroup\$ @janos That's something I'd consider opinion based, I think
value >= low
is better, personally. \$\endgroup\$Simon Forsberg– Simon Forsberg2015年01月17日 22:58:24 +00:00Commented Jan 17, 2015 at 22:58
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.
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.
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.
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?
-
\$\begingroup\$ vegasoft.site90.com/graph.png \$\endgroup\$Edenia– Edenia2015年01月17日 16:52:31 +00:00Commented Jan 17, 2015 at 16:52
-
\$\begingroup\$ "Is
n
betweenx
andy
?" \$\endgroup\$Morwenn– Morwenn2015年01月17日 17:39:12 +00:00Commented Jan 17, 2015 at 17:39
EOL
is a common abbreviation of end of line, such as\n
in Unix and\r\n
in windows... \$\endgroup\$