[Skydiver]

I'm just curious why people think that multiple operations in a line is considered bad code, yet there are other examples that are considered "elegant". Do you have to be bad to be good?

Bad code:

// compute GCD
while((remainder = numerator % denominator) != 0)
{
    numerator = denominator;
    denominator = remainder;
}
return numerator;

// Copy strig
dst[count = strlen(src)] = 0;
while(i-- > 0)
    dst[i] = src[i];

Good code:

// Read and store characters
char *dst = buffer;
while((ch = fgetc(inputfile)) != EOF)
    *dst++ = ch;
*dst++ = '\0'

// Copy n characters or less from a C-string
while (*src && n-- > 0)
    *dst++ = *src++;
*dst++ = '\0';

Is this good or bad code?

// Make month, day, year integers look more formal
while (cin >> month >> day >> year)
    cout << AddSuffix(day) << " day of " << LookupName(month) << ", " << MakeFourDigit(year) << endl;

[jimblumberg]

Quote

I'm just curious why people think that multiple operations in a line is considered bad code, yet there are other examples that are considered "elegant".

I don't necessarily always consider multiple operations on a single line bad code. However for most people posting questions on this board I quite often recommend single operations on a single line. This is because having those single operations on a single line make it much easier to troubleshoot problems.

What I do object to is multiple statements on a single line. In my opinion this is a bad practice, it tends to obscure the program for the sake of a little vertical space.

int i = 0; float c = 10.0;
...
if(c == 2) dosomething();
... 

I also usually recommend breaking long calculations into smaller, more observable pieces to make troubleshooting easier. However after debugging is completed, combining these calculations onto one line, without the use of the intermediate variables, is acceptable.

Quote

Is this good or bad code?

// Make month, day, year integers look more formal
while (cin >> month >> day >> year)
    cout << AddSuffix(day) << " day of " << LookupName(month) << ", " << MakeFourDigit(year) << endl;

Without seeing more content I will say the snippet is acceptable, if you're consistent. However one problem you may encounter is that the functions in your cout call can be evaluated in any order so if any of your functions modify variables used in the other functions you may have problems. So many times using intermediate variables is recommended to eliminate this kind of problem. See this link for a better explanation. C++ Notes Function call order.



Jim

[NickDMax]

Personally I think there are a few reasons to stay away from overloading too many operations in one line of code. The most important aspect is clarity -- Code should be readable and doing too much within one line of code can overload the reader who may miss something in the line.

Compounding operations in expressions can also rely upon tricky things like order of operation. This again can make code a little hard to follow if the reader is not quite as familiar with the rules as the code author.

Sometimes the apparent order of operation is actually an undefined property. For example you see people mixing side effects by adding multiple x++'s in the same line and they debug their code on computer X with compiler Y and it works great but then crashes on other platforms.

Not that it is really any different than separate lines, but one has to keep in mind not just what happens in a bit of code when working but also what happens when an error occurs or an exception is thrown. Often the logical flow is hard to keep track of in one-liners.

Another point is that complex expressions can be a little harder to debug because most debuggers (as far as I know) don't let you put break points mid-expression, and sometimes when tracing though such code it can be difficult to know which function is called (or you just hit the wrong "jump into"/"step over" buttons etc.) -- i.e. maybe I am just particularly dumb but sometimes I get lost in the debugger on one-liners.

Of course I say all of this and I am a HUGE offender on this point. I do it all the time, and I think as functional programming (i.e. think lambdas) becomes more and more a part of C++ you will find more and more of this.

Like most "is it bad?" -- the answer ends up being, "it depends".

The mark of bad programming is when it is done just to "be fancy" or try and be "sophisticated". Beginners should avoid it because it clouds clarity. However, I think it is something that we will see more and more of in the coming years.

This post has been edited by NickDMax: 13 August 2012 - 10:21 AM

[NickDMax]

Also one more note: History -- There used to be these great contests to achieve as much as one could in one line. C is particularly good at this. The result was pretty horrible obfuscated code. From this grew the IOCCC Competition. (or is the last C already Competition?)

When things get carried away and swing too far there is often a push back to go the other way. So then even a little violation ends up getting an over-reaction because people can't let the past go.

[vividexstance]

I do agree with the points made about code clarity especially for beginners, and how the order of evaluation can cause problems. Here's a quote from "The C++ Programming Language Special Edition" by Bjarne Stroustrup in chapter 6, section 1.2, page 112. He's talking about assigning a value to a variable and returning that variable in the same line:

Quote

Assignment is an operator, and the result of the assignment is the value of the variable assigned to. This allows me to assign the value END to curr_tok and return it in the same statement. Having a single-statement rather than two is useful in maintenance. If the assignment and the return became separated in the code, a programmer might update the one and forget to update the other.

I know this is a simple example, but he does make a good point. Now I wouldn't recommend writing obfuscated code, but sometimes having more than 1 operation on a line has its benefits as well as its drawbacks.

[jimblumberg]

Could you provide the actual snippet that he is talking about? Not everyone has a copy of that book.

Jim

[baavgai]

Perhaps it is meant to be ironic? All your good examples have many multiple operations per...

// char *dst = buffer;
char *dst;
dst = buffer;
//while((ch = fgetc(inputfile)) != EOF)
ch = fgetc(inputfile);
while(ch != EOF) {
	// *dst++ = ch;
	*dst = ch;
	dst++;
	ch = fgetc(inputfile);
}
// *dst++ = '\0' // you probably don't need that ++...
*dst = '\0';

To a C programmer, *dst++ = *src++; is probably reasonably clear. Probably because they've been inflicted with it a few times. Is it really clear? You certainly have to understand the order of operations. On the other hand, *dst = *src; dst++; src++; is clear even if you don't know how such ops are ordered.

What I'm say is that convoluted one liners may not seem so convoluted after repeated exposure.

[vividexstance]

jimblumberg, on 13 August 2012 - 12:05 PM, said:

Could you provide the actual snippet that he is talking about? Not everyone has a copy of that book.

Jim

This is the snippet right above the quote I posted. If you want the whole function let me know.

Token_value get_token()
{
    char ch = 0;
    cin >> ch;
    
    switch(ch) {
    case 0:
        return curr_tok = END;   // assign and return

[Skydiver]

LOL! I had very similar code in a parser I once wrote. The code reviewers did not like code I'd written exactly as line 8. The irony is that all members of that code review team almost held Bjarne in almost religious reverence. Alas, the code review was in 1994 and the book was published in 2000.

[NickDMax]

The only thing that really bothers me about the code examples given is the lack of brackets { }.

I really despise these:

while(condition)
    statement;

if(condition)
    statement;
else
    someother;

(and no working in Python has not helped either)

I once new a "rock star" (in his own mind at least) programmer who seemed to pride himself on finding ways to leave off brackets or any any other program-structure he considered "unnecessary". It drove me batty and it has stuck ever since. Oddly enough though he also liked to use the conditional operator which I think I probably picked up from him.

Point is that often we get in to these "religious wars" about style and really they are more reflections of ourselves than necessities.

This post has been edited by NickDMax: 13 August 2012 - 03:24 PM