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); }
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jan 17, 2015 at 16:04
\$\endgroup\$
2
  • 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\$ Commented Jan 17, 2015 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\$ Commented Jan 17, 2015 at 16:26

5 Answers 5

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.

answered Jan 17, 2015 at 16:15
\$\endgroup\$
4
  • 2
    \$\begingroup\$ It would be slightly better to arrange the comparisons as low <= value && value <= high \$\endgroup\$ Commented 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\$ Commented Jan 17, 2015 at 16:23
  • \$\begingroup\$ It might be interesting to read but certainly not profitable to use. \$\endgroup\$ Commented Jan 17, 2015 at 21:13
  • \$\begingroup\$ @janos That's something I'd consider opinion based, I think value >= low is better, personally. \$\endgroup\$ Commented Jan 17, 2015 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.

answered Jan 17, 2015 at 16:27
\$\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.

answered Jan 17, 2015 at 16:30
\$\endgroup\$
0
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.

answered Jan 17, 2015 at 19:16
\$\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?

answered Jan 17, 2015 at 16:47
\$\endgroup\$
2
  • \$\begingroup\$ vegasoft.site90.com/graph.png \$\endgroup\$ Commented Jan 17, 2015 at 16:52
  • \$\begingroup\$ "Is n between x and y?" \$\endgroup\$ Commented Jan 17, 2015 at 17:39

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.