Production Code and Good Practices.

Hello,
I find too many lines makes a function much longer and adds to the entire project.
Is it a bad idea to use these ?

Code:

---

x = a < 3;


return i == -3;

---

Is it a better idea to use these ?

Code:

---

if (a < 3) {
    x = 1;
} else {
    x = 0;
}


if (i == -3) {
    x = 1;
} else {
    x = 0;
}
return x;

---

How about the conditional operator instead?

Code:

---

return (i == -3) ? 1: 0;

---

Quote:

---

Originally Posted by ZTik

Is it a bad idea to use these ?

---

I'd say it is a good idea to use them if x or the return type of the function is bool or conceptually boolean. If they are supposed to be integers, then there might be the consideration of whether you want the code to also cleanly compile as C++. If so, then wbpribs's suggestion would be better as it avoids a possible warning from the difference in result type between the C operators and their C++ versions.

Code:

---

return (i == -3) ? 1: 0;

---

That line is not a bad practice, but a terrible and horrible one.

Quote:

---

Originally Posted by ZTik

Code:

---

return (i == -3) ? 1: 0;

---

That line is not a bad practice, but a terrible and horrible one.

---

Why is that?

Lots of thoughts:

Code tends to cycle around the 'idea'. Different people have different ideas of what 'ideal' code looks like, and everybody nudges code a little to their idea of the ideal solution.

Codebases have 'dialects' - that is the way things are done in this code base. People who are unwilling to change because of some idea that their way is the right way are usually wrong.

Code should be written for the reader/maintainer. If you know you are working on a 'simple' part of the code, or one which isn't performance critical don't go out of your way to use advanced techniques when simple ones will do..

There is good code, there is bad code, and there is evil code. Don't write evil code. This is evil code:

Code:

---

double average(double total, int count) {
  return total/count;
}

---

You should write code as if you are the one who has to debug it at 2am while pulling an all-nighter. If you do this you will never have to debug your code at 2am, if you don't you will.

Don't write code that only works for your use-case, write it to work for all valid parameters - e.g. this code is evil:

Code:

---

  char name[9]
  sprintf(name,"%04i.dat");  // Build a filename "nnnn.dat"

---

Make your code the ideal house guest - it should always clean up after itself, and should always let you know if anything is broken.

Try to write code that works, but has the complexity of code written by a 10 year old. This is (I think) perfectly valid:

Code:

---

  printf("file '%s' %s\n",  fname, (file == NULL && !(file=fopen(fname,"wb"))) ? "failed" : "is open");

---

But I would far rather see:

Code:

---

  if(f == NULL) {
  file=fopen(fname, "wb");
  }

  if(f == NULL) {
  printf("file '%s' failed\n",  fname);
  } else {
  printf("file '%s' is open\n",  fname);
  }

---

After 40 years, and maybe two generations of C programmers the good patterns are most likely the ones in use. Don't bother to needlessly invent new ones.

Trying to do something to "outsmart the compiler" is dumb. Compiler writers are most likely as smart as you, and been in the business for longer. If you really must do this, then prove it works.

10% of the code is responsible for 90% of the execution time. When it comes to optimization, choose your battles wisely.

Everything is a trade-off. Most of the time wasting some resources is OK, be it CPU time, memory, system calls or I/O.

Don't lay land mines for those who follow, build bridges.

Code:

---

if(x)
    return NULL;

---

vs

Code:

---

if(x) {
    // spot to put logging when debugging
    return NULL;
}

---

I believe that returning the result of a comparison or boolean operation is exactly the right thing to do, if the function you are writing is a predicate.

That is:

Code:

---

    bool is_lower(int ch) {
      return 'a' <= ch && ch <= 'z';
    }

---

This makes perfect sense, because the function is a predicate -- that is, it answers a question "is this lowercase?" So returning a logical-and expression, which is a boolean value, is a sensible thing to do for a predicate.

On the other hand, if you start mixing "integer" with "boolean", things start getting hard to read. Beware of that.

In general, remember that you are writing the code to be read by some poor victim six months from now (possibly that poor victim is you!) and you should try to be nice to them. Make things easy to read, easy to remember, etc.

The good thing about having those small things as functions is that they can be passed to another function via a pointer and make flexible functions

Consider the implementation of qsort where the user can decide what makes a<b