Clean code and refactoring. Extra local variable

Question

We had big discussion about small peace of code in our team.

Example 1 developer added extra local variable to make code more readable. In local variable he saved value from constant.

Example 2 same code without extra variable, but less readable from our developer point of view.

What do you think is more readable? Better from refactoring point of view?

1.

var tolerance = Constants.DateTypeGeneratorTolerance;

var dayType = DateTypeGenerator.GenerateDateType(
                courseTripValidity,
                tolerance,
                symbols,
                startDate,
                endDate);

2.

var dayType = DateTypeGenerator.GenerateDateType(
                courseTripValidity,
                Constants.DateTypeGeneratorTolerance,
                symbols,
                startDate,
                endDate);

This question doesn't have a single correct answer, so it may be closed as being too subjective (but for what it's worth, I prefer the version with a local variable). — Chris Taylor
– Chris Taylor, Commented Jun 22, 2012 at 12:16
I'd remove the vars...I'm joking. At least the original developer considered readability which is a plus in my view — Chris S
– Chris S, Commented Jun 22, 2012 at 12:38
Pretty unhealthy to have big arguments about something so inconsequential as this. You all go stand in a circle now, hold hands and sing Kumbaya. — Hans Passant
– Hans Passant, Commented Jun 22, 2012 at 13:12

Péter Török · Accepted Answer · 2012-06-22 12:35:00Z

There is no definitive answer to this, but I personally prefer the latter version. IMHO Constants.DateTypeGeneratorTolerance is not unbearably long (although not very far away from that either). And if its name is well chosen, it tells precisely what it means. Whereas introducing a local variable clutters the space and makes it more difficult for me to follow what's going on - not only when directly reading the code, but it is also more cumbersome to search for the usages of this constant.

Moreover, the name itself is long largely because it contains scoping information. By putting all global constants into a single class, you feel the need to differentiate between them using naming prefixes (like DateTypeGenerator). Whereas if you moved the constant to where it logically belongs to - either to DateTypeGenerator itself, or to a separate DateTypeGeneratorConstants class - the constant name itself would become short and concise.

Note that several languages offer special features to reduce the need for qualifiers, thus shortening names. Such as static imports in Java, or using directives in C#.

Combining the above with C# using directives you get:

using Tolerance = DateTypeGenerator.Tolerance;

...

DateType dayType = DateTypeGenerator.GenerateDateType(
                courseTripValidity,
                Tolerance,
                symbols,
                startDate,
                endDate);

Collectives™ on Stack Overflow

Clean code and refactoring. Extra local variable

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related