Is fewer lines of code always better? [duplicate]

softwareengineering.stackexchange https://softwareengineering.stackexchange.com/questions/203684

Question

Which of these programming styles is better?

var result = methodOne(methodTwo(a, methodThree(b)), c, d);

or

var result3 = methodThree(b);
var result2 = methodTwo(a, result3);
var result = methodOne(result2, c, d);
Was it helpful?

Solution

In layman's words:

The important thing is not the numbers of lines but the readability of the code.

Any fool can write code that a computer can understand. Good programmers write code that humans can understand. (M. Fowler)

In the examples you gave, the second one is definitively easier to read.

Source code is for people to read.

Besides, intermediate values make the code easier to debug.

Code one-liners, on the other hand, are useful to show other people that you are "smart" and that you don't care.

OTHER TIPS

It depends, and your example is not useful in making the decision.

While fewer lines of code are not always better (at some point it leads to obfuscation), they usually are, simply because there's fewer things to keep track of when trying to understand the code.

In your specific example:

If the names of the intermediate values actually convey meaning that is not made obvious by the names of the methods and parameters used to compute those intermediate values, then they help in understanding the code.

However, if the names of the intermediate values do not add information, then they make the code harder to understand, since they're an additional state to keep track of. Even if they're immutable, they force you to consider whether they're used only once, or in different places.

Code with as few lines as possible is definitely the best code and every semi-colon you see is basically the developer admitting they weren't clever enough to use advanced constructions like the comma operator or short-circuiting operators to keep the line going as long as possible like you can say `(x++ && false) || y += 2` instead of `x++; y += 2` and the first variant is just so much more concise and clearer as you can tell because it contains no semi-colons honestly I almost threw up writing the second one I mean who wants to use a character whose name contains a word related to your butt that's just dumb and anyways semi-colons introduce lots of vulnerabilities like what if you typed `while(true) ;` that would just keep running forever and so its clearly better to have no semi-colons to avoid problems like that overall I'd say programmer salaries should be inversely proportional to the number of newlines and semi-colons in their code because everyone knows as soon as you start using a metric like that it starts working even better than before you incentivized it

Measuring programming progress by lines of code is like measuring aircraft building progress by weight. (Bill Gates)

Of course, fewer lines are not always better. But in my experience, fewer lines are often better than more lines in terms of readability and maintainability. There are exceptions, of course, and your example might be one such exception.

Weight is needed to build an aircraft. But it should be in the right places.

In the first version, it is hard to see which parameters are passed to which method. So the second version is clearly better. In other cases however may not be that obvious.

The main difference of the second version is that it allows you to name the intermediate values. There may be cases where this name is crucial for understanding the code.

If however the intermediate values are anyway just called something like varOne, you may also just omit them by in-lining them.

It looks like you're using JavaScript, which changes the paramaters ever so slightly.

Unless you have a reason to store those var's, you'd be better off not initalizing them at all. But, you should make the single-statement version as readable as possible, by adding whitespace and a comment.

//Do X, Y, Z....
var result = methodOne(
    methodTwo(
        a, 
        methodThree(b)
    ), 
    c, 
    d
);

Is fewer lines of code always better?

The short is simply no. The longer one, no, because it depends.

  • How long is the code I want to shorten? If the code doesn't fit the screen, shortening it often makes it more readable
  • How much is done in single line? If it's an access through 10 objects, it's surely good idea to split it to many lines
  • What is better? Write long standard version or use single tricky syntax sugar or library function? How much people know that sugar/function? A great example are regular expressions. With single regex you can achieve as much as with many lines of code - but it sometimes takes hours to understand single regex. However, there are some regex that are widely used and google searchable.

Which of these programming styles is better?

Well, the second one is much much better in one aspect. If you need to debug what happens, you have the variables to inspect and more possibilities to place breakpoint.

On the other hand, I don't like using too much local variables, because I must invent the names for them. I hate such names as result1/2/3, because after some time I don't know which is for what. But I don't want to spend too much time on inventing the names... The first example is (for me) actually more readable because of bad variable names in second example.

Usually I start with the first code, and convert it to second first when I need to debug.

However, if the expressions are more complex, I prefer keeping intermediate results in local variables - but those local variable must have self-explaining names. Only then the code is easier to read.

My rule of thumb is to put each operation on its own line. This is especially helpful when stepping through code with the debugger. I also like assigning the result of an operation to a variable before passing it along so that I can break and examine its contents. so instead of

var result = methodOne(methodTwo(a, methodThree(b)), c, d);

I would go 

var result3 = methodThree(b);
var result2 = methodTwo(a, result3);
var result = methodOne(result2, c, d);

If I'm not mistaken, the java and .Net JIT compilers would compile the two examples the same way.

alternatively, I would write a very detailed comment before the line explaining what it is I'm doing.

I suspect what everyone wants to hear is "no".

The fact of the matter is that your example isn't clear enough to give a solid answer.

resultThirdMethod is a horrible name. I find the first version much clearer than the second.

However, if you could give your variables names -- names that give more information than merely where the data came from -- then it could be better. It all depends on the actual situation.

In this particular example I don't find the first version harder to read, it's just as reading a simple arithmetical expression such has (a + f(b)) * c * d. The second version doesn't add any information that helps to understand it (unless the variables have meaningful names that help to understand a complicated process).

What I feel a bit problematic with the second version is that it introduces additional complexity when analyzing the code. A programmer who reads the code needs to think about if resultThirdMethod or resultSecondMethod is used somewhere else then just to compute result. And if you write a method full of such statements there is no clear line of computation, you have to check constantly what is computed from what and where. So in this case, if you want the more verbose version, I'd suggest to either encapsulate it into a separate function:

function methodCombined(a, b, c, d) 
    var resultThirdMethod = methodThree(b);
    var resultSecondMethod = methodTwo(a, resultThirdMethod);
    return methodOne(resultSecondMethod, c, d);
}
// ...
result = methodCombined(a, b, c, d);

or at least enclose it into a code block so that the scope of variables is immediately visible:

var result;
{
    var resultThirdMethod = methodThree(b);
    var resultSecondMethod = methodTwo(a, resultThirdMethod);
    result = methodOne(resultSecondMethod, c, d);
}

It depends on how familiar the reader is likely to be with these operators and their problem domain. Even if all the methods are arithmetic functions, it can help to break it up.

Breaking up a larger formula into small steps can quite spoil the readability:

(let* ((term1 (* b b))
       (term2 (* 4 a c))
       (discriminant (- term1 term2))
       (twoa (* 2 a))
       (top1 (+ b (sqrt discriminant)))
       (top2 (- b (sqrt discriminant)))
       (root1 (/ top1 twoa))
       (root2 (/ top2 twoa)))
  (list root1 root2))

versus

(let* ((root1 (/ (+ b (sqrt (- (* b b) (* 4 a c)))) 
                 (* 2 a))) 
       (root2 (/ (- b (sqrt (- (* b b) (* 4 a c))))
                 (* 2 a)))
  (list root1 root2))

Breaking up formulas into small steps is what a compiler does when it generates intermediate code. (Fortran: FORmula TRANslator.)

Breaking up expressions for readability helps, but don't "Fortran it" (go overboard).

Best style is the one which makes the code most readable (for a human, that is -- computers don't care). But "readable" depends a lot on who does the reading so that's context dependent. This is enough, though, to answer your exact question: "Is [whatever] always better" ? Then no, it depends on the context.

The first person who must read code is the developer himself. If you write code such that you will yourself be able to read and understand it four weeks from now, then you will have already done a good job -- a much better job than the majority of developers in the industry (alas).

One can add a few generalities, though. Reading code implies using the eye and brain. Human eyes tire faster when they have to move a lot, especially when they have to move vertically (lateral movement is less an issue). Using fewer lines of code, all other things being equal, will allow the reader to grasp the code with less vertical eye movement, so that's somewhat good.

On the other hand, the syntax analysis which goes in the brain will want to use as many "visual anchors" as possible, and will suffer from certain activities, in particular counting parentheses. Spreading the code over more lines, with indentation, helps the brain. It really is a trade-off between eye fatigue and brain fatigue, and, I insist, the right balance point is not the same for everybody.

Ultimately, it is a matter of "good taste". People who write ugly code are also people who are happy eating bad pizza.

You give a bad example.

The solution you give to solve a problem better have a good design so it can be understandable, and good enough to keep the code short.

If you can make code that is more understandable by making it longer, it's something to do, but there is a limit.

I'll always think that code which have both a good design AND is kept short, ensures more understandability the shorter it is.

You should focus on a good design, and then find ways to make the code short enough so that people that will read it can understand it without making them scroll and scroll.

So to sum up, there is no real good answer to this, what is important is how well you divide your code into functions and sections, and how you can make it both short and easy to understand.

The bill gates quote is good, but there's also the quote that talk about a perfect piece of code, which is one where you can't take anything out.

This is one of those issues I think were you get to decide which way that works for each separate case. Deciding if a particular result should be stored in an intermediate variable is part of being a programmer, part of the craft. You use the skills you have acquired together with coding standards at your workplace.

In the two contrived examples below, I would probably go with 1A and 2A, because of readability (too much noise and redundancy in 1B, too long and indirect usage of an anonymous array in 2B). But you can't really come up with any exhaustive list of patterns where you can stipulate one way over the other - new situations arise all the time as the art of programming progresses.

1A:
startManufacture(loadProductName(), isProductOk(loadProductState()));

vs.

1B:
String productName = loadProductName();
ProductState state = loadProductState();
boolean productIsOk = isProductOk(state);
startManufacture(productName, productIsOk);


2A:
boolean antennaCount = storage.getCount(currentDealer, items[ANTENNA_INDEX]);
boolean chassisCount = storage.getCount(currentDealer, items[CHASSIS_INDEX], EXTERNAL_SHIPPING);
orderManufacture(RADIO, antennaCount, chassisCount);

vs.

2B:
orderManufacture(RADIO, storage.getCount(currentDealer, items[ANTENNA_INDEX]), storage.getCount(currentDealer, items[CHASSIS_INDEX], EXTERNAL_SHIPPING));

I would say that both examples are equally bad, but that is just because of the insufficient example code. To be able to say if one of the two is better, you need to provide a real example.

There is absolutely no correlation between the number of source code lines, and the quality of the code.

Assuming that we disregard the obvious fact that the code needs to do what it is supposed to do, the most important aspect is that it is easily understood by the readers.

The "classic" way to accomplish this is to add code comments that describe the functionality of the code. In my personal experience, it is even better to use descriptive identifiers for methods, variables and constants.

If you for example see a source code sequence like this, you will immediately understand what it is supposed to do, and you can examine the individual methods and variables to (hopefully) just as easily determine that they do what they are supposed to do:

if (a_new_game_is_starting && hand_not_dealt_yet()) {
  deal_new_hand_to(all_players_at_table);
}

In the example above, there is no need to add any comments at all. What would a comment say that is not easily understood from the actual code? This is something to strive for - to not need to add comments to the code to describe what it does.

This is not something that is hard to do. All it requires is that you think about what you really are trying to accomplish, and name your identifiers from a functional point of view.

More important than readability is maintainability, because the main reason we read code is to maintain it. Maintaining often means making changes to the code because of small or large changes in requirements.

SO, think about what changes in requirements there might be, and try to make them as easy to do as possible. If you can lead the maintainer right to the relevant code, and let them make the changes with minimum editing, they will think you are a great programmer!

OTOH, if people are reading the code to verify its correctness, make sure your reasoning is clear as to why it is correct. If people doubt the correctness of your code, they are likely to re-write it, which is likely to break it. To prevent that, make sure the code explains itself.

Also, be sure to explain your performance-related decisions, because one way good code gets destroyed is by being re-written in a misguided attempt to make it faster.

return ($this->main->context != "ajax" || in_array($this->type, $this->definition->ajax)) ? eval('return method_exists($this,"Show'.ucfirst($this->type).'") ? $this->Show'.ucfirst($this->type).'('.(count($args) ? join(",",array_map(create_function('$a','return (is_numeric($a) || preg_match("/^array/",$a)) ? $a : "\"".$a."\"";'),$args)) : "").') : null;') : '';

I bumped into this code on a project I inherited some time ago. I think it nicely demonstrates why it is a bad idea to try and use as few lines as possible. It's completely unreadable and impossible to debug.

Spelling things out can help not only with readability but also writability.

This might seem like a silly interview question but I was asked once in an interview to write a function to rotate an image 90 degrees. The source and destination are provided (you don't need to do any memory allocation) and the width and height of the source. Yes, it's an easy problem. I suggest you do it before continuing and see if you have any trouble.

.

Did you? Did you test it? Since I was asked the question in an interview I used the question in interviews as well and what I found was many programmers would get confused. They'd write code like this

 void Rotate90(const Pixel* src, Pixel* dst, int width, int height) 
 {
    for (int y = 0; y < height; ++y) 
    {
      for (int x = 0; x < width; ++x) 
      {
         dst[???] = src[???];

And that's where they would get stuck. They would spend sometimes as much as 20 minutes on the whiteboard trying to figure out what goes in those ??? area. Lots of times they'd think they have the answer and write things like

         dst[x * width + (height - 1 - y)] = src[y * width + x];

getting the fact that using width with dst is wrong. Or often they'd get the x and y mixed up in one side or the other.

The problem is that x, y, width, and height all swap meaning because when rotating 90 degrees the destination becomes width tall and height wide which makes referring to those variables confusing. I always wished a programmer would add some more variables to help make it clearer for themselves, like this

 void Rotate90(const Pixel* src, Pixel* dst, int src_width, int src_height)
 {
   int dst_width = src_height;
   int dst_height = src_width;

   for (int src_y = 0; src_y < src_height; ++src_y) 
   {
     for (int src_x = 0; src_y < src_width; ++src_x) 
     {
       int dst_x = src_height - src_y - 1;
       int dst_y = src_x;
       dst[dst_y * dst_width + dst_x] = src[src_y * src_width + src_x];
     }
   }
 }

It seemed to me breaking things out into their explicit meanings would make it far easier to write the code and far easier to avoid mistakes. I felt like if I saw a programmer do this I'd think of them as wise and rate them higher.

Yes there are a few optimizations you can make. That's not the point. Yes, the math inside the last statement could also be broken out into separate lines. That would be fine as well. The point is, adding more lines made it easier to understand for the person writing the code, not just a person reading it later. In this case because the meaning of x, y, width and height are different in relation to dst vs src making separate variables for each makes the code far clearer to read AND write.

Continuing...similarly it's often useful to be able to inspect values before a function is called, especially if it's a function in a library or the system that you can't step into. If you write code like this

ctx.lineTo(x + Math.cos(angle) * radius, y + Math.sin(angle) * radius);

vs this

var circleX = x + Math.cos(angle) * radius;
var circleY = y + Math.sin(angle) * radius;
ctx.lineTo(circleX, circleY);

In the second style you can now stop the debugger at the ctx.lineTo line and inspect circleX and circleY. You might find they are NaN which would probably get you to check that x, angle, and radius are actually the correct names, something you wouldn't be able to do in the debugger. ("strict" mode probably catches that). It also allows you to easy log the values

console.log("cx: " + circleX = ", cy: " + circleY);

Yet another example, 

class Person 
{
public:
   ...
   const String& name() const { return m_name; }
   int age() const { return m_age; }

private:
   string m_name;
   int m_age;
};

vs

class Person 
{
public:
   ...
   const String& name() const 
   { 
     return m_name; 
   }

   int age() const 
   { 
     return m_age; 
   }

private:
   string m_name;
   int m_age;
};

I know some of you will scream at this example because your personal style wants getters and setters to be 1 line but hear me out ... when debugging, depending on the debugger, in the first style if you put a breakpoint on age or name the debugger will not be able to inspect this nor m_age nor m_name so you'll have no way to checking before the function returns what it's going to return or what object it's referencing. In the second style you can put breakpoints on the return lines and you'll be able to inspect everything.

The point of this answer is that more lines is often not only good for readability. It's also often good for writability and for debugging.

No.

In fact, I'd go further and say fewer lines of code are rarely better.

Now, that's not to say don't encapsulate method calls etc but in your example above, when debugging - it's going be very hard to see the outcome of the methodThree and methodTwo when it's all inlined like that.

Lots of code in a method doing lots of things is bad. Lots of small methods that get called and assign to local variables enhance readability, enhance maintainability and have no real cost as the compiler will happily optimise away.

Think from basics. Why we write code, when we want to add some functionality and hand it over to another developer, can he understand your code or not, why it's important to structure, why should one follow basic standard guidelines?

The answers are as follows:

Why we put comments in our code.
For developers (who can take advantage of this small documentation to understand the code and routine functionality), computers or decoding/encoding systems just bypass comments, and only deal with the actual code. For example:development bundle and minifest version,

If we use second option it's easy to read and extend and not much guidance is needed by a new developer extending our code. But if we use first option which is less documented only the person who wrote this code or can understand its purpose, and not the entire team. We are working on rapid development cycle where we need to work on different project and don't have much time to get co-ordinate with other departments, so in this case the second approach works well and another developer can easily extend functionality. It's all about dependency: we work on modularization of code where thousands of developers work on thousands of function and they know only what to return, what to pass to function and which variable is assigned to what, and for what.

For single user or small programs the first approach is ok when team are working together but when people are working remotely what to do, thats the point you likely to go with second option, optimization is another technique to shorten the code but only after we are happy with output.


computer is machine , only understand zero and one's language , but if we write code who itself guide other developer about programe like second approach , new people can also use this as guidance or to edit the program ,
computer understand 0 and 1 and human can understand comments
so i like second approach which help developer to understand whats going on inside loop or function
eg:**suppose if you are working on some project and you use 1st way and at some case you had another task and another developer is working on your code , does he understand this or not? its not question about simplicity or how to document , its all about **how to farward our code to our partner when he want to update the code with hassle free support.
second approach is well for partners and developers and 1st options work well when we are working on web platform , or writting script where size of the code matters

TL;DR:
Document your code. Write comments when you're writing it. It will save you valuable time three months down the line when you come back to it and can't understand what the code is doing. Also, your co-workers will sing praises of you when they can actually understand what you're doing, and are easily able to extend/refactor or otherwise modify code written by you. Don't overdo it, however.

If fewer lines of code make sense, go ahead and do it in that many lines. But do not make it so that typing fewer lines would make you have to add comments to explain your code. It should be as self-explanatory as possible.

What about

var result = methodOne(   methodTwo(    a, 
                                        methodThree(b)
                                   ),
                          c,
                          d
                      );

This will probably be even further improved by the help of a decend IDE, printing lines that match the braces.

In a nutshell, the human brain,for most common people, can hold 7 different things at best. This is for common people, and this is an average. (I'm afraid I haven't found the source of my statement)

As a consequence, if you want to write code that anyone can read, it should not use more than 7 differents variables/concepts/functions/operations.

for example:

rotate(translate(rotate(translate(circle,50,0),0,10,pi),-50,0),0,10,pi)

there it is really hard to understand what is going on, astonishingly this lines does absolutely nothing in the geometric sense:

circle = translate(circle,50,0)
circle = rotate(circle,0,10,pi)
circle = translate(circle,-50,0)
circle = rotate(circle,0,10,-pi)

this is also why it is easier to read functions that are less than 40 lines of code too.

Both examples are actually bad.

What happens if methodOne fails? What happens if methodTwo fails? What happens if methodThree fails? You obviously omitted the error checking for clarity here, but it is an important point - adding error checking will add more lines of code (which on it's own should answer your question) so extra lines of meaningful code that actually does something should never be viewed as a bad thing.

Aside from that, I would still favour the second example. The reason why is because it's more maintainable; you can more easily insert a call to methodTwoPointFive between the calls to methodTwo and methodThree, for example, should future requirements need it.

Licensed under: CC-BY-SA with attribution
scroll top