Is using spacing effectively equivalent to the long method code smell?

by Jake   Last Updated April 15, 2019 17:05 PM

There's a common code smell involving long methods with the most common answer being that methods should be really small, less than 50 lines per say (or 20). I understand why this is because it enhances readability, reduces repeated code, etc. However, I was wonder if this is at a statement level or at a file line level.

For example, let say I had a method that was being called and it had a lot of parameters passed (ignore the LPL smell for this example), too much that it would span across the width of a common editing window size in an IDE.

method(A, B, C, D, ...);

EDIT Envision the lettered parameters to be some typed variable, ellipses indicates further parameters.

However, somebody may write the method call like this

method(
A,
B,
C,
...);

Now this spans multiple lines, but in my personal opinion, its far easier to read scrolling down than across. The actions on the mouse are also different since scrolling side to side often requires a mouse select of the (left to right) scroll bar (I'm aware a button could be programmed to do this and some mouses have a joystick built in, but it isn't common in business).

I find this to be relevant for SQL queries. For example

SELECT 
 A.A
,A.B
,B.C
,B.D
FROM 
table A
LEFT JOIN
table B
ON
A.E = B.E
AND 
A.F = B.F
WHERE 
A.A IS NOT NULL
AND 
B.C > 50

Now in this scenario, when I'm testing, its very easy to comment out where clauses or change logic operators by commenting them out instead of performing deletes if it was all on one line and had to re-type.

I guess I was wondering what long line method smell really means and whether its long lines in terms of many statements or long lines in terms of lines per file?



Answers 2


Your SQL example is terrible. It breaks things into individual lines unnecessarily, hurting readability without providing significant benefit.

A better arrangement is to line break on major keywords and provide sensible indentation, like this:

SELECT A.A, A.B, B.C, B.D
  FROM table A
  LEFT JOIN table B 
    ON A.E = B.E
    AND A.F = B.F
  WHERE A.A IS NOT NULL
    AND B.C > 50

Which doesn't hurt your commenting out and testing ability at all.

There are better ways of making long method calls more readable. For example:

var a = [long expression];
var b = [another long expression];
var c = ...

method(a, b, c, d, ...);
Robert Harvey
Robert Harvey
March 22, 2018 17:31 PM

Is using spacing effectively equivalent to the long method code smell?

No.

Never let line number concerns eliminate whitespace. If anything, this shows that line counting is not a good quality metric.

Methods and functions should be short. Very short. If you can break them down into anything smaller it's worth considering doing just that.

But line counting is really silly. Especially in languages that ignore whitespace. I can define an entire program in one line. That doesn't make it good.

Now personally I find this example very readable:

method(A, B, C, D);

I don't find this very readable:

public String method(final String Alessandro , final String Bartholomew, final String Cleveland, final String Dashiell) { return ""; }

Even though they have the same number of parameters, this one is terrible. It would be worth adding whitespace to make it readable.

public String method(
        final String Alessandro, 
        final String Bartholomew, 
        final String Cleveland, 
        final String Dashiell
) {
    return "";
}

Never shorten a name because of width or length considerations. Use the best name you can and find a better way to lay out your code.

However, I find

method(
A,
B,
C,
...);

to be pointlessly wasteful of space. At the very least use some indentation.

method(
    A,
    B,
    C
);

With such short names and no other adornments this still seems silly. But at least it emphasises the structure. If you had a good reason to do this I wouldn't deny you the white space, but it's annoying if I can't see the reason.

SELECT 
 A.A
,A.B
,B.C
,B.D
FROM 
table A
LEFT JOIN
table B
ON
A.E = B.E
AND 
A.F = B.F
WHERE 
A.A IS NOT NULL
AND 
B.C > 50

This is just plain hard to look at. All you've done is allow for your commenting scheme. That doesn't make it read much better than

SELECT A.A, A.B, B.C, B.D FROM table A LEFT JOIN table B ON A.E = B.E AND A.F = B.F WHERE A.A IS NOT NULL AND B.C > 50

You still have the flexibility for other considerations. You can add some visual separation between keyword and parameters

SELECT 
     A.A
    ,A.B
    ,B.C
    ,B.D

FROM 
    table A

LEFT JOIN 
    table B

ON 
    A.E = B.E

AND 
    A.F = B.F

WHERE 
    A.A IS NOT NULL

AND 
    B.C > 50

You can emphasize the relationship between the keywords

SELECT 
,A.A 
,A.B 
,B.C 
,B.D

    FROM 
    table A

    LEFT JOIN 
    table B 

        ON 
        A.E = B.E

        AND 
        A.F = B.F

    WHERE 
    A.A IS NOT NULL

        AND 
        B.C > 50

And it may be worth remembering that comments don't have to take up an entire line:

SELECT A.A, /*A.B,*/ B.C, B.D
    FROM table A
    LEFT JOIN table B 
        ON A.E = B.E
        AND A.F = B.F
    WHERE A.A IS NOT NULL
        AND B.C > 50

Whitespace is powerful. It has a serious impact on readability. Use it wisely.

Counting lines is the worst lesson to take from learning that functions should be short. They shouldn't be short because you squished all the whitespace and descriptive names out of them. They should be short because you gave everything it's own little function to live in.

candied_orange
candied_orange
March 22, 2018 17:34 PM

Related Questions


Updated June 21, 2017 19:05 PM

Updated June 03, 2015 23:02 PM

Updated April 12, 2018 16:05 PM

Updated May 22, 2019 15:05 PM