Software Engineering Stack Exchange is a question and answer site for professionals, academics, and students working within the systems development life cycle. It only takes a minute to sign up.
Sign up to join this community
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?
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.
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
)
)
)
do_complex_op(new paramStruct { StartingState = 0, XCoord = xcoord }), then it becomes self documenting and much easier to read
Jun 30, 2011 at 15:12
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.)
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.
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.
It makes it easier to:
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.
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.
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.
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.
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.
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.
"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.
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.
I would say, if the arguments fit on one line, do so. If not, then one argument per line makes for great readability.
foo(arg1, arg2, arg3, arg4, arg5)
vs.
foo(
arg1=arg1,
arg2=arg2,
arg3=arg3,
arg4=arg4,
arg5=arg5,
arg6=arg6,
arg7=arg7
)
