Is it appropiate to break down a long line into two operations?

Question

Tidying up some code that has what I consider to be a confusing line break structure:

return
       CommonContext.HttpWebService.DownloadXml(configuration.MethodUrl(APIMethods.CharacterSheet),
                                                         postData);

If it were on one line it would clearly to be to long to be readable. As it stands it is not clear to me at a cursory glance how the "return" and "postData" are related to the long line. CommonContext and APIMethods are static classs, configuration is a local variable.

Thinking about this I would probably write the same in two lines as follows:

string methodUrl = configuration.MethodUrl(APIMethods.CharacterSheet);
return CommonContext.HttpWebService.DownloadXml(methodUrl, postData);

Is this an effective way of spiting the code up or is there a better way? In this instance I am using C# 2.0.

Answer 1

Yes. It's a good thing usually. It makes code more self-documenting (with a good variable name) and also makes debugging easier (allows putting a breakpoint on the first line, and when an exception is thrown from the first line, you can immediately distinguish it as opposed to the single line situation).

Answer 2

Readability above all. No program ever failed its users for being too slow due to too many lines.

Answer 3

Yes, that's absolutely an effective way of refactoring the code. It can also be helpful if you want to use the debugger or a logger to look at the value of methodUrl before it's passed to DownloadXml.

Another additional benefit is that you're giving the value a name. That can be a big help by explaining its meaning at the same time. This can be particularly important if there are lots of parameters - it really helps to be able to quickly glance at an argument list at the call site and a parameter list at the method declaration and take a good guess what's going on just based on the names.