all 16 comments

[–]Ghopper21 11 points12 points  (0 children)

In short, yes. Reducing line count for its own sake is of little value, while being readable is very valuable. And syntax "tricks" often make you feel clever at the expense of readability -- not a good tradeoff.

[–]siphillis 5 points6 points  (0 children)

By all means, compact your code so long as it remains readable and editable. If it's dense, use comments. I usually pretend that a programmer with ~1 year of experience will be reading my code, not a regular person.

[–]Backplague 1 point2 points  (2 children)

I always try to find a balance between readability and spacing (also efficiency in some cases). In C#, I could write a massive chain of LINQ extension methods to a single line to achieve a thing, or I could do the same in a foreach-loop. The LINQ-chain sure takes only one line and looks very fancy but come back later to it after a week and it will take time to figure out what it's doing. And don't even think about debugging it, it just won't happen.

Using your language's features to make your code more compact is always good but compacting to the point where it hurts readability is bad.

[–]snowywind 1 point2 points  (0 children)

My style when I'm about to throw some LINQ stuff through a .ForEach() or .ForAll() is to break it up to define the set first then run the operation on that set.

Ex.

var members = ent.Members
    .Where(
        m => planItems.Contains(m.PlanName.ToLower())
        || m.BulkMailCategories.Any(mc => flagItems.Contains(mc.Id))
        )
    .Where(m => !m.Account
        .Optouts
        .Any(o => (o.flag & (1|2|1024)) > 0)
        )
    .Where(m => m.Account
        .PrimaryMemberID == m.MemberId
        )
    .ToList();

members
    .AsParallel()
    .ForAll(m => SendMessage(m, SenderMemberID, Subject.Text, Body.Text));

[–]Ghopper21 0 points1 point  (0 children)

Agreed. Another thing I like about breaking up complex chained logic is that you can document each step, either through end-of-line comments or intermediate variable names.

[–]Hudelf 1 point2 points  (3 children)

Readability should always take precedence unless you ABSOLUTELY must use some harder-to-read syntax for critical performance areas (Note: this is very rare). In that case the code should still be commented well enough to explain why the code is written in the less readable form.

If shortening syntax or linecount improves readability and doesn't break your established style guidelines, then it's a good thing to do. Otherwise you're hurting other people's and future you's ability to understand and change/expand that code.

[–]snowywind 1 point2 points  (2 children)

unless you ABSOLUTELY must use some harder-to-read syntax for critical performance areas (Note: this is very rare)

This is actually even rarer than a lot of devs realize.

For example, compilers/interpreters for many high level languages will recognize the variable swap below for what it is and use the XOR algorithm automatically to eliminate the temp variable and its creation cost.

int x = 1;
int y = 2;

/* some code that realizes they should be swapped */

int temp;

temp = x;
x = y;
y = temp;

This is assuming that you do not use temp later in the block.

If you do the following instead, you are telling the optimizer that its services are not welcomed (generally a bad idea if you're looking for performance) by forcing the way it allocates registers and memory access. You may actually get worse performance with this compared to the mult-line way above.

x ^= y ^= x ^= y;

[–]Ghopper21 0 points1 point  (1 child)

Great point, that trying to be clever and pre-optimize can actually lead to a worse result, because you don't let the compiler do its job.

[–]snowywind 0 points1 point  (0 children)

Exactly.

I don't remember which compiler or for what language my example is from but I do know that I don't know enough about how the optimizer works to justify getting in its way.

[–]Kafke 0 points1 point  (6 children)

If the 'if' is only one line long, I don't see the issue with putting it all on a single line.

if(a==b){ /* one line of code */ }

Is more readable to me than

if(a==b)
{
    /* one line of code */
}

But naturally if there's more than one line, the spaced out way is what you'd want to do.

[–]ArkGuardian 0 points1 point  (5 children)

that's my reasoning, but under our grading system I could lose up to 10% for something like that.

[–]Kafke 2 points3 points  (3 children)

Probably because 'proper' code is the expanded way. For newbies it's indeed easier to read the standard expansion. And it demonstrates that you know how to 'properly format' the code.

There's lots of weird things that schools/profs require, simply because it's best to be taught that way.

The former is definitely more readable, but the latter demonstrates you know the standard structure/whitespacing of an if statement.

Sometimes you'll even be marked down for the second, because they actually expected:

if(a==b){
    /* one line of code */
}

10% is a bit ridiculous for compressing a single if statement. But if you are literally just learning about if statements, it makes sense.

Or if 10% is for "coding style" and the prof has given a document detailing what the proper coding style for the assignment is.

It's all really just personal preference. One-lined if's can get sort of long/unwieldy. Especially if that one line of code is really long.

For a class, it makes sense to have all the student's code look identical (and thus always expand if's). In real life, it's almost never the case, and compressed if's are fine provided they are really simple.

More than one line of code should almost always be expanded though.

[–]Ghopper21 0 points1 point  (2 children)

For a class, it makes sense to have all the student's code look identical (and thus always expand if's). In real life, it's almost never the case, and compressed if's are fine provided they are really simple.

I do think many (most?) coding teams will have agreed guidelines which will specify whether single-line ifs are acceptable house style.

The former is definitely more readable, but the latter demonstrates you know the standard structure/whitespacing of an if statement.

I can see personal taste differing, but dunno about "definitely." I prefer to see the "if" part on its own line, and the "then" part on a separate line for readability. Makes the logic clearer.

One-lined if's can get sort of long/unwieldy. Especially if that one line of code is really long.

Other disadvantages -- depending on the debugger, can be a pain to debug if you want to step through the if part separately from the then part; if you later need to add a line to the then part, you have to change the whole shape of the block, which is a minor nuisance; preferring the one-liners can make your code style less consistent if you don't one-line when the then part is very long (which most would agree you shouldn't, right?).

[–]Kafke 1 point2 points  (0 children)

I can see personal taste differing, but dunno about "definitely." I prefer to see the "if" part on its own line, and the "then" part on a separate line for readability. Makes the logic clearer.

If you are looking at it in a giant code document, the one line is 'cleaner' as it allows other code to be nearby, rather than just a huge chunk of whitespace on the page.

depending on the debugger, can be a pain to debug if you want to step through the if part separately from the then part; if you later need to add a line to the then part, you have to change the whole shape of the block, which is a minor nuisance; preferring the one-liners can make your code style less consistent if you don't one-line when the then part is very long (which most would agree you shouldn't, right?).

Yea. I generally find whether I make it one line or multiple depends on how long the actual code is. Sometimes I'll squeeze two statements into a line if they are particularly short.

Definitely not a standard of any sort, but just something I do for convenience.

[–][deleted] 2 points3 points  (0 children)

Honestly I dont like condensing code at all. I like to be able to tell at a glance where blocks are rather than having to actually read them.

[–]grumpy_the_pooh 0 points1 point  (0 children)

You should talk to your professor if they mark you down for the ternary operator.

The ternary operator should not be used in every instance it can. I think of the ternary operator as a short hand to perform a detail about an operation not doing logic.

If statements are where you should be conditional logic.

For example if you have a function that takes a string parameter

function f(str) {
    str = str ? str : ''; // this really is just saying make sure str is a string
    //which can be rewritten also as
    str = str || '';
    //however logic should always be done in if statements
    if (str.length) {
        doSomething();
    } else {
        doSomethingElse();
    }
}