0

I have recently started programming in C and was wondering about a more concise way to write a simple function that returns 1 or -1 depending on the equality of two int values. What I wrote:

int valueCompare(int i, int j) {
    if (i != j) {
        return -1;
    }
    else {
        return 1;
    }
}

It's readable but seems inefficient. I have seen return statements that utilizes a colon and question mark however am not familiar with that style. Does anyone have any recommendations on how to write a more efficient function for this without a loss of readability?

Lundin
  • 195,001
  • 40
  • 254
  • 396
rbecca
  • 53
  • 2
  • 6
  • 6
    you can use a ternary operator for that logic and write return i != j ? -1 : 1; – SadiRubaiyet Jan 20 '16 at 06:02
  • 2
    `write a simple function that returns 1 or -1 depending on the equality of two int values` Just a comment, but in the `C` world it would be a rather unusual design choice (or requirement) for what is essentially a boolean function to return `-1` or `1`. Far more common would be to return `0` or `1` (or anything `!= 0` for that matter) in which case it would all reduce to `return i == j;`. – dxiv Jan 20 '16 at 06:19
  • @dxiv Thank you for the constructive and valid criticism; you are right—I should be using either a return 0 or 1 value given that my main function does return a 0 – rbecca Jan 20 '16 at 06:33
  • @SadiRubaiyet Many thanks for naming the term that I could not think of (ternary operator). Given the prevalence of return statements utilizing it (from what I can see on Google), I think this would be a perfectly readable choice to utilize in my C code in the future. – rbecca Jan 20 '16 at 06:36
  • 'It's readable but seems inefficient' - inefficient how? You should stick with 'readable'. Coding is very easy, testing is less so and debugging is very hard. Go with the coding style that reduces overall work. 'Clever' coding is often just bad coding. – Martin James Jan 20 '16 at 08:04
  • @MartinJames "Inefficient" was perhaps the incorrect term to use—perhaps "lacking in style" would be a better term in this case as in my current course we are penalized for any written function over 5 lines (not including switch statements). I was therefore trying to think of the best way to write a function that was both readable, functional, and concise with minimal trade-offs – rbecca Jan 20 '16 at 08:17
  • @rbecca Ask your prof/TA if that penalty was intended to instruct students to write code that is unclear and difficult to debug! A line with only a block racket, or the like, should not count towards your 'five a day'. I'm giving your course a failed 'F' grade :) – Martin James Jan 20 '16 at 09:37

4 Answers4

5

You can use the ternary operator:

int valueCompare(int i, int j) 
{
    return (i == j) ? 1 : -1;
}
Mitch Wheat
  • 295,962
  • 43
  • 465
  • 541
  • 2
    i was about the say the same thing – SadiRubaiyet Jan 20 '16 at 06:03
  • The question title clearly states: "A More Concise Return Statement". and that's what this shows. – Mitch Wheat Jan 20 '16 at 06:10
  • 1
    The ternary operator is common place. I would disagree that it is less readable in this case. – Mitch Wheat Jan 20 '16 at 06:41
  • The conditional operator has weird side effects though, so it shouldn't be blindly used as some "cure all". Consider the following scenario: `int a = -1; unsigned int b = 1; long long c = -1;` and then do `if( (true ? a : b) != c) puts("-1 != -1");` will print `-1 != -1`. The bug is caused by the conditional operator's unintuitive promotion between the 2nd and 3rd operand, which takes place no matter if it is the 2nd or 3rd operand is actually evaluated. – Lundin Jan 20 '16 at 07:59
3

For readability, consider rewriting the function to use boolean logic instead:

#include <stdbool.h>

bool isEqual (int i, int j) {
  return i == j;
}

This is, in my opinion, the most readable form possible.


Or alternatively for generic programming/portability/usefulness purposes, you should make a C programming de facto standard "functor" for value comparison:

int compare_int (const void* i1, const void* i2)
{
  const int* ip1 = i1;
  const int* ip2 = i2;
  return *ip1 - *ip2;
}

This has plenty of advantages:

  • Generic function format.
  • Standard. Can be passed to functions like bsearch, qsort etc.
  • Mixes equality, lesser than and greater than checks in one single function.
  • Compatible with dinosaur compilers.
Lundin
  • 195,001
  • 40
  • 254
  • 396
2

It is readable and it is the compiler's responsibility to make it efficient. Any "clever" way of writing it will be less readable and will not be more efficient.

Basically I would leave it at omitting the braces when there is just a single statement and even that is a matter of taste:

if(i != j)
    return -1;
else
    return 1;

vs.

if(i != j) {
    return -1;
} else {
    return 1;
}

vs.

if(i != j)
{
    return -1;
}
else
{
    return 1;
}

Of course I can think of many "clever" way of writing it from

return (i != j) ? -1 : 1;

that is only appropriate for simple expressions (i.e. this is ok, but throw in a couple of function calls and it will be totally unreadable) to

return 2 * (i == j) - 1;

but this is barely readable. You should, however, be able to understand it, because you might see this kind of thing in the wild.

And there is absolutely nothing on either to make them more efficient.

Jan Hudec
  • 73,652
  • 13
  • 125
  • 172
  • Ternary opertor is common place. I would disagree that it is less readable in this case. – Mitch Wheat Jan 20 '16 at 06:07
  • @MitchWheat, if it's just a simple comparison, yes. But I suppose it is just a contrived example for something that would be rather complex in practice. – Jan Hudec Jan 20 '16 at 06:09
  • 2
    Well, we can only answer the question asked, not some hypothetical problem. – Mitch Wheat Jan 20 '16 at 06:09
  • @MitchWheat, that's what I am doing. Answering the question asked. I just assume it is a _generic_ question, so I am answering it _generically_. – Jan Hudec Jan 20 '16 at 06:11
  • @JanHudec It slipped my mind that braces are not necessary if there is only one statement after the if/else if/else. Thank you for the reminder! – rbecca Jan 20 '16 at 06:40
  • Codes who use such styles will eventually get around to shoving compound expressions in each section of the ternary and so make debug stepping through their code as difficult as possible for anyone unfortunate enough to need to maintain their code and greatly increasing the need for rewriting/retesting for minor enhancement requirements. – Martin James Jan 20 '16 at 12:01
  • Honestly, every time I see multiple, compound expressions in an if, while or return, strung together with nested ternaries, boolean operators, comma operators and no brackets, I want to find the original coder and shove them through a wood chipper. – Martin James Jan 20 '16 at 12:04
  • Note that I'm not actually advocating murder for bad coders here, (though it would clear out the PHP and Android rooms). – Martin James Jan 20 '16 at 12:05
0

How about

#define SAME_VALUE 1
#define DIFFERENT_VALUE -1

Or the like ? and how about

 int valueCompare (int i, int j){
         return i == j;
    }
Lundin
  • 195,001
  • 40
  • 254
  • 396
Friedrich
  • 5,916
  • 25
  • 45