Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I work with someone who, every time they call a function they put the arguments on a new line e.g.

aFunction(
    byte1,
    short1,
    int1, 
    int2,
    int3,
    int4,
    int5
) ;

I find this very annoying as it means the code isn't very compact, so I have to scan up and down more to actually make any sense of the logic. I'm interested to know whether this is actually bad practice and if so, how can I persuade them not to do it?

share|improve this question
19  
+1, I don't know enough to answer your question, but I hate this as well. If you have that many arguments in a function however then your function is probably doing too much. –  maple_shaft Jun 30 '11 at 14:56
21  
More to the point, why do we still have to see each others preferences about this? why can't our IDEs just automatically present it in whatever style we want? –  Alex Feinman Jun 30 '11 at 15:08
    
You say it means the code isn't very compact. But then I see people using high-res screens so they can see a lot of code, but then they puff it up with whitespace, like putting each { on its own line, that pushes other code off-screen. I assume these are just matters of what people are used to, nothing more. –  Mike Dunlavey Jun 30 '11 at 16:06
4  
Mike, I like having code take up minimal screen real estate. But it's a compromise. I put { on a separate line because it makes it easier to match with the closing } and correctly understand the scope of the block. It's worth the tradeoff of losing a line of screen real estate. –  Brian Schroth Jun 30 '11 at 17:58
1  
@Alex: You're totally right. I think the right thing to do would be to have a language where the parse tree of the code is stored on disk, and displayed according to user preferences. –  Neil G Jun 30 '11 at 18:59
show 6 more comments

14 Answers

It makes it easier to:

  • Reorder arguments.
  • Comment out or disable arguments.
  • See exactly which argument has changed when you view diffs in your version control system
  • Avoid re-indenting and word-wrapping everything when you add or remove an argument, or change the function name. (Even if your editor automatically indents, you are still creating lots of unhelpful whitespace changes which make it harder to follow changes in your version control system.)
share|improve this answer
add comment

Well, here's some downvote-bait. I've never been accused of doing the popular thing. Clearly, if things fit on one line, then fine, fit them on one line.

But my main concern is not whether the code is "ugly" or "pretty". My main concern is how easy it is to understand and make changes to without making mistakes.

If the arguments are long and there are a lot of them, why not put them on separate lines? To my mind that makes it easier to see what they are, and easier to change them if necessary. It also gives me room to attach a comment to each argument if I want to.

I also want to minimize the chance of making a mistake if I add or remove an argument to a function, which is more likely to happen at the end of an argument list than at the beginning. For that reason, I prefer to put the comma (,) at the beginning of a line, than at the end. Then, if for example, I want to remove or add an argument at the end of the list, it's a one-line edit. I don't have to go fiddle with the comma that needs to go at the end of all lines but the last, where the last has to end with a parenthesis.

So (boy, I'm gonna get flamed for this) I write it like this:

nameOfFunction(firstArgument
    , secondArgument // optional comment
       ...
    , lastArgument   // optional comment
    );

When there's a function with from five to twenty arguments, the function didn't get that way all at once. It grew over time, meaning there were lots of edits. Any edit not completed is a syntax error or a bug. So I don't claim this is pretty. I claim it helps in getting the edits right.

(And for those who say I should pass a structure instead, all that does is displace the issue, because you need a bunch of lines of code to fill in the structure, not to mention the extra code to declare and allocate it.)

share|improve this answer
    
I personally think this is a great answer because you explained your reasoning very well. Good job Mike. –  Jordan Dec 4 '11 at 22:58
add comment

"I'm interested to know whether this is actually bad practice..."

Yes, it's bad practice, except when the list of variables is abnormally long. But in that case, the problem is likely due to the function's design. Why not pass an object that encapsulates many of the parameters?

"...and if so, how can I persuade them not to do it?"

Tie them down and keep tickling them until they agree to stop that crap.

share|improve this answer
add comment

Why are you wasting cycles on such a trivial concern? Just fire up your trusty IDE, open the file, and reformat. Voila! It will be in whatever form you want.

Now let's move on to the really important issue--vi or emacs, LOL.

share|improve this answer
    
And then when you come to check that into source-control? –  pdr Dec 4 '11 at 18:16
add comment

You'll rarely get an honest answer out of a programmer for something like this. Everyone will just answer with what they do or do not prefer. The truth is that, as much we all struggle with it at times, the only "bad practice" here is your own inflexibility.

You have to be brutally honest with yourself to be able to distinguish between things that are actually bad and things that just annoy you. The truth is that in C/C++ and similar languages you will rarely find an indentation practice that has a tangible affect on the understandability of the code. Most discussions about these sorts of things just have both sides stacked with people making ridiculous, disingenuous arguments to try to justify their own personal preference.

Which to my reading... is exactly what you're requesting in this question: a ridiculous, disingenuous argument to justify your personal preference.

share|improve this answer
add comment

To be honest it depends on the person.. I would say for complex functions as demonstrated by FrustratedWithForms first example, then yes; otherwise a big NO. Then again this is why I prefer to apply the IDE functionality of code arbitrarily.

share|improve this answer
add comment

It may look funky to you but it makes working on code easier. While refactoring you can comment out individual arguments very easily and check your refactor before actually deleting things. You can also comment out and temporarily replace types quite easily.

It's also easier to read than:

int f(int, int, int,
      char, double, int
      X const&, Y)
{}

I have not gone as extreme as you're showing (since there's no names on the parameters it is not much use), but I have gotten into the habit of either splitting each parameter on its own line or not doing splitting at all.

The important part is that your code can be printed or displayed on standard, 80col displays and still be legible.

share|improve this answer
add comment

It's a matter of preference. For complicated function calls where you want to document each parameter, or where the variables are rather long and there are many of them, this can be good.

For example:

do_complex_op(
      0, //Starting state, always 0, ask Joe why
      X, //X-coord of thingy
      y, //Y-coord of thingy
      73, //in this case, we don't want to use Z but want constant 
      dlogMessageTitle, //message for dialogue title
      dlogMessageText, //message for dialogue contents, don't care about this.
      SomethingIP, //IP of something-or-other server, can be NULL, won't crash.
      someObject.childObject.getValue(key1).HVAL, //very long path to HVAL
      someObject.childObject.getValue(key1).LVAL, //very long path to LVAL
      this.parentWindow.owner.mainTextBox.text.value.trim, //get the trimmed text, untrimmed text causes weird output
      pvrMainSettingForLongBlahs.getObjectByPath(somePath),
      pvrMainSettingForLongBlahs.K_TCA_UPPER_LIMIT,
      pvrMainSettingForLongBlahs.K_ENDPOINT_COMPLIANCE_LEVEL,
 );

With languages that allow named parameters this is more common if you use the parameter names (example is in PL/SQL):

PKG_SOME_TEST_CODE.FN_DO_SOMETHING( in_text => 'test text',
                                    in_id => v_id,
                                    in_ref_id => v_ref_id,
                                    out_array_for_storage => v_bArray); 

But I agree with you that if the function call is simple and not too many parameters, this could get annoying, such as:

setColour (
    r,
    g,
    b
 );

I find much easier to read as

 setColour(r,g,b);

For @ammoQ:

rc=a(b,c(d,e(f)))

rc=a(
     b,
     c(
       d,
       e(
         f
        )
      )
    )
share|improve this answer
9  
The first example is a wrong answer to a real problem. Why not use explicit variable anmes in the first place ? –  deadalnix Jun 30 '11 at 15:00
    
@deadalnix: Good point, cleaned it up a bit. –  FrustratedWithFormsDesigner Jun 30 '11 at 15:07
1  
True. However, it's not always an issue with variable names. It's more to do with long variable names, arguments with default values, etc –  inspectorG4dget Jun 30 '11 at 15:09
3  
I would argue the better solution to the problem is to refactor do_complex_op() so it takes a structure as a parameter. Then you can do do_complex_op(new paramStruct { StartingState = 0, XCoord = xcoord }), then it becomes self documenting and much easier to read –  KallDrexx Jun 30 '11 at 15:12
1  
@KallDrexx: I agree, but sometimes changing the code is not an option, such as when it's a function in someone else's library. Sure, I could make a wrapper to it, but I'll still have to call their original function at some point. –  FrustratedWithFormsDesigner Jun 30 '11 at 15:16
show 10 more comments

I've frequently seen that style on function declarations or definitions, but never on a call (until now). There it makes sense sometimes as it lets you add a comment for individual parameters more clearly. It seems like he copied that style to calls without really knowing the underlying reasons behind it. You have a good argument against and he doesn't seem to have a good one for, so you have my vote, but I'm not the one you have to convince.

share|improve this answer
    
I see both on calls. If the parameter list is too long, it should be broken across multiple lines. If the parameters don't group normally within width limits, I expect them to be on separate lines. If the function and parameter names fit well on one line, then I often see them arranged that way. –  BillThor Jul 1 '11 at 2:00
add comment

Is it against the company coding standards?

If not, then initiate a discussion about the standards and what people would like to see changed. Make sure you bring this up as one of the things you'd like to change.

Have a full discussion about why you don't think this is useful and hopefully you'll win the day. You never know your colleague might convince you that his way is the best after all ;)

Once you've got an updated standard then it's documented what everyone should be coding to, so if your colleague persists in doing this you can raise it in his code reviews.

share|improve this answer
add comment

IMO everything that is uncommon is bad practice unless it's superiority over the usual style can be positively proven. “Matter of taste“ is a poor excuse for writing code that is harder to read than necessary for the majority of programmers, because one day, a poor soul, not used to that style, will have to maintain that code.

Proving that it's uncommon is relatively easy, show the source of examples in MSDN or similar sites, show large open-source code bases etc. Show the output of code beautifiers. Ultimately, show how everbody else in your team is doing it. Don't accept a bad style just because someone is too stubborn.

share|improve this answer
    
Hm... with that approach, how could we ever introduce a new best practice (or, gramatically correct: a better practice)? –  Treb Jun 30 '11 at 15:24
1  
Treb: Sure, just show that the better practice is in fact better, not just different. –  user281377 Jun 30 '11 at 15:27
3  
But "harder to read" is, itself, subjective and a matter of opinion. For me, one argument per line is easier to parse visually than two, three, or four arguments per line. And I always break a call into multiple lines if it extends beyond approx the 100 character mark in the editor. –  Toby Jun 30 '11 at 15:57
    
Toby: Everthing that is not formatted the way you are used to is, at least in the first moment, harder to read. –  user281377 Jun 30 '11 at 16:11
2  
Meh. "Harder to read" can be measured objectively. It just tends not to be. Arguing about it is more fun. –  JasonTrue Jun 30 '11 at 23:00
show 1 more comment

I would say that function calls should be all on one line unless they significantly exceed whatever your standard code width is (often 80 characters, often a cause of arguments :-).

I don't see any advantage to this style. It looks subjectively ugly, and I find it a pain when searching code. For instance you might want to quickly search and see whether the function is ever called with a certain parameter passed as NULL. This is easy visually when all parameters are on one line, and harder when they are split like this.

share|improve this answer
    
This 80 character thing has to go, there just aren't any technically-valid reasons for it anymore. We now live in an era of 19xx X 16xx monitors and adjustable fonts AND font sizes. –  anon Jun 30 '11 at 23:24
1  
@anon: Valid reasons for it? Why do you think text is printed in columns and books are narrower than they could be? Because the human eye loses track when reading across very long lines. –  Zan Lynx Jul 1 '11 at 7:29
1  
@anon: Also I like to use my widescreen to have two or three files open in a horizontal split which gets back to 80-100 characters in a line. –  Zan Lynx Jul 1 '11 at 7:30
1  
@anon: Technically no, practically: hell YES. Zan Lynx is completely right, plus there are additional reasons: merging, diff, using command line utilities... Oh and good luck focusing on 8p font as you'll get older :o) –  MaR Dec 5 '11 at 12:32
add comment

It's just a coding guideline which you may or may not like. The important thing is that you and your colleagues agrees to use it or not.

Obviously, the best way to increase readability is to limit the number of arguments.

share|improve this answer
16  
Too many people reduce arguments by dumping them in an array. I'd rather see an ugly mess than cleaner-looking code with hidden complexity. –  Satanicpuppy Jun 30 '11 at 15:20
12  
Arrays aren't the way to go. There might be a structure hiding in the args, or maybe the function is doing too much and should be split up. –  Michael K Jun 30 '11 at 15:30
4  
I find that using multiple lines help to make the code readable IF parameters are long expression or a few too many. Otherwise it's just annoying. –  PedroC88 Jun 30 '11 at 15:58
1  
Putting them in a data transfer object just moves the problem. If all the arguments are required, then they all have to be mandatory arguments of the DTO's constructor, which means you still have that many arguments. –  Scott Whitlock Dec 4 '11 at 17:29
add comment

I wouldn't call it either. Best practice where I've worked has typically been to have function calls be all on one line, unless you would have to scroll horizontally any significant amount to see the entire call. Its a judgement call, but I would definitely say that to put all functions like this is out of line unless that is the standard established by your organization.

This is why it is good practice for an organization to establish a set of guides to which all programmers should adhere. Its all about consistency and readability.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.