13

Is it OK to create a variable which only purpose is to increase readability?

Example:

public class Example
{
    private final HowLongCanARepositoryNameReallyBeRepository howLongCanARepositoryNameReallyBeRepository;

    public boolean method( final HowLongCanAMethodArgumentNameReallyBeArgument howLongCanAMethodArgumentNameReallyBeArgument )
    {
        final HowLongCanARepositoryNameReallyBeRepository repository = howLongCanARepositoryNameReallyBeRepository;
        final HowLongCanAMethodArgumentNameReallyBeArgument argument = howLongCanAMethodArgumentNameReallyBeArgument;

        return repository.isThisReallyThatLongOfMethodToConsiderARefactoringLikeThis() && argument.isThisReallyThatLongOfMethodToConsiderARefactoringLikeThis();
    }
}

Is there any good reason to not do things like this?

From what I can see, this does not look too bad. You can easily trace the original variables and get the intention of the original variables by their names, and while reading the code to see what it is actually doing, you can have a shorter and easier way to read variable names.

Is this reasonable?

5
  • 15
    Be careful not to conflate readability with verbose. If repository is a sufficient name in that context, can you explain why the original name must be that long? Nov 22, 2021 at 15:56
  • The code base that I'm working on, there are more that then one repository being used. So so having different names is useful. And on the code base we usually use the name of the class as the variable name as well, for the most part. Nov 22, 2021 at 16:00
  • 3
    It sounds like a way to get around an issue you are not supposed to address. Including class names in member names does not make sense, context should do. There is a double layer of submissiveness in the question: you can/may not fix existing code and then you come here to ask for permission to work around it. You may have a cultural problem more than a technical one. Apparently there is an undisputed owner of the code, which is good! But it seems to me you should be discussing this with him/her (probably him) instead. Nov 23, 2021 at 7:09
  • @MartinMaat I'm not actually restricted and conversion about this stuff is totally ok. I was just wondering to myself more then anything. I didn't create those variables in code in fact. Thanks for your feedback :) Nov 23, 2021 at 11:17
  • Of course, you want to make sure you don't assign to your local alias instead of the variable you originally intended to...
    – jcaron
    Nov 23, 2021 at 18:05

6 Answers 6

30

In general, creating local variables for readability is a good thing. A local variable gives a locally relevant name to something; that same thing might have a different name in a different context.

But in the example you give, the parameter name is already a local variable. If that particular method only uses one repository, then the local name for that parameter can just be "repository".

The only exception I can think of to this is when the method is defined in an interface or base class, particularly if the language allows parameters to be passed by name. In that case, a local alias like you have in the example might make sense - but if possible, the interface should use a less long-winded name anyway.

In a comment, you say that the reason for the long name is:

we usually use the name of the class as the variable name as well, for the most part

This is definitely a bad idea. A variable name should convey purpose, not just type.

For instance, in a back office system, you might have a class User, an instance of which represents the logged in user. In some contexts, that might be the only user that's relevant, so user seems a reasonable name; but you'll quickly run into situations where it's ambiguous.

For instance, you might have a function fetchLogs(User user) (using a parameter name because it's easy to illustrate, but the same applies to purely local variables). There is only one user in scope, but why is it in scope? Is it the user who's logged in, to check permissions? Or is it a filter to find only log entries made by, or about, that user? If you aim to name variables based on their purpose, it might instead be fetchLogs(User currentUser), or fetchLogs(User loggedBy), or fetchLogs(User subject). That's immediately clearer, and makes it easy to create contexts where all three users are in play: fetchLogs(User currentUser, User loggedBy, User subject)

Note that the local names in the example above are longer than the type-based ones, partly because "user" is such a short word; but they lead to more readable code, because they use that extra length to convey useful information.

2
  • 8
    This is definitely a bad idea. A variable name should convey purpose, not just type. I see your point. Nov 22, 2021 at 16:17
  • To go further, the types are there to tell you the type, not the variable name.
    – OrangeDog
    Nov 23, 2021 at 16:51
11

Creating aliases for the sole purpose of having a shorter variable name could be seen as a code smell.

In general, variable names should not be long enough to require aliases. They should be concise and precise. Just long enough to convey its purpose. Long variable names could mean that your naming needs improvement or that your code is too complex, thus requiring longer descriptions.

Also, two variables in the same scope should usually indicate separate things, which is the opposite of what an alias does.

That being said... It is a smell, not a rule. The real world is messy and there could be external reasons that your naming needs to be long.

  • Maybe it’s a company naming convention you must follow
  • Maybe the language conventions you are using are verbose in themselves
  • Maybe you are describing some real world behaviour that is in itself complex or has a large name (imagine a method that plots something for, let’s say, the Barban–Davenport–Halberstam theorem. BarbanDavenportHalberstamTheorem is already quite large!)

It’s up to you to evaluate your case!

Check if any of the following could improve your design:


Are your variable names too detailed?

Are you repeating the type of the variable in the name?

    Repository sourceRepository = Foo(); // worse
    Repository source = Foo(); // better

    // Keep in mind that a good IDE will help
    // when inspecting the variables to find which type they are

Are you repeating the context in the variable name?

    public class HTMLParser
    {
         private HTMLBody htmlBody; // worse
    }

    public class HTMLParser
    {
         private Body body; // better
    }

Could you use a more specific term?

    ListThatAddsToTheFrontAndPopsFromBack listThatAddsToTheFrontAndPopsFromBack = Foo(); // worse
    Queue queue = Foo(); // better

Is there any redundant wording in the variables?

    int moneyAmount = Foo(); // worse
    int money = Foo(); // better

Short and precise names is the goal

...and don't forget good naming is an art in itself


Is your module (or class, or function) doing way too many things?

If your code is doing way too many things, you may have to use too many words to describe what it is doing.

If you require too many words to define its context, this may be a sign that you should break down the context into smaller parts.

    public class FooBarForecaster
    {
        private FooBarForecasterAsyncDataController source;
    }

    // maybe could be refactored into more classes, packages, namespaces...

    public namespace FooBar
    {
        public class Forecaster
        {
            private AsyncDataController source;
        }
    }

Can you reestructure your operations to better convey context and meaning?

A more precise context and meaning can lead to shorter variable names.

    private Connection connection;

    public bool ValidateRemoteRepositoriesAndCopy()
    {
        List<Repository> localRepositories = this.connection.GetLocalRepositories();
        List<Repository> remoteRepositories = this.connection.GetRemoteRepositories();

        for(int i=0; i<remoteRepositories.Length; i++)
        {
           bool isPassingFoo = remoteRepositories[i].IsPassingFoo();
           bool isPassingBar remoteRepositories[i].IsPassingBar();
           if(!isPassingFoo || !isPassingBar)
              return false;
        }

        if(remoteRepositories.Length != localRepositories.Length)
            return false;

        for(int i=0; i<remoteRepositories.Length; i++)
        {
           remoteRepositories[i].CopyTo(localRepositories[i]);
        }
        return true;
    }
    private Connection connection;

    public bool ValidateRemoteRepositoriesAndCopy()
    {
        List<Repository> local = this.connection.GetLocalRepositories();
        List<Repository> remote = this.connection.GetRemoteRepositories();
        this.ValidateAndCopy(remote, local);
    }

    private bool ValidateAndCopy(List<Repository> source, List<Repository> destination)
    {
        if(!this.IsValid(this.source))
            return false;
        if(source.Length != destination.Length)
            return false;
        this.Copy(source, destination);
        return true;
    }

    public void Copy(List<Repository> source, List<Repository> destination)
    {
        for(int i=0; i<source.Length; i++)
           source[i].CopyTo(destination[i]);
    }

    private bool IsValid(List<Repository> repositories)
    {
        for(int i=0; i<repositories.Length; i++)
            if(!this.IsValid(repositories[i]))
                return false;
        return true;
    }

    private bool IsValid(Repository repository)
    {
        if(!repository.IsPassingFoo())
            return false;
        if(!repository.IsPassingBar())
            return false;
        return true;
    }
2
  • I find you answer very insightful. Thanks :) Nov 23, 2021 at 11:15
  • The "better" vs "worse" instances in your post are highly questionable. For instance, you suggest changing a HTMLBody type to just Body: that's fine for the property name, but not for the type itself. For now, i think this answer provides bad suggestions.
    – julealgon
    Dec 29, 2021 at 15:46
6

I realize that your example is a bit contrived, but having an argument with the name, or suffix, of "argument" is poor style.

One case where going to shorter names makes sense is if you are coding a mathematical equation, and, within the specific context of that equation, the shorter names are idiomatic and therefore more clear. For example (also slightly contrived, but imagine a much more complicated equation)

plotLine(slopeOfLine, yIntercept) {
  float xLocation, yLocation;
  ... 
  lots of code using yLocation, xLocation, slopeOfLine, yIntercept
)

might be coded as

plotLine(slopeOfLine, yIntercept) {
  float x, y;
  float m = slopeOfLine;
  float b = yIntercept;
  ...
  lots of code using the familiar y = mx + b
)

p.s. For other possibilities, I'm less familiar with military/government spec or encryption code, but I could see similar examples where incoming variables get renamed to alice, bob, etc...

1
  • 1
    The math context makes total sense, since the short names is how they operate in the field anyway. The problem is when we are trying to express more "real world" complicated concepts that are not as exact as math I guess. Thanks for the insight. Nov 23, 2021 at 11:20
4

It is not inherently a problem. For example:

private void CleanName(PersonDetails details)
{
    var name = details.Person.Identity.Name;

    // Lots of name-handling logic
}

This is perfectly fine, to cut down on an otherwise obtrusively long dot chain in your code.

However, while I'm aware your example is intentionally contrived, long names are generally undesirable in clean code, specifically because it's not nice to have to juggle them.

Your question correctly identified that the names (both class and field) are clumsily long; but your proposed solution is to hide it by using a local variable. The better solution here is to fix the long names, so that the problem is solved for everyone, not just your current method.

0

In a way, you're touching on the subject of Roles here, which is an essential concept of the DCI (Data, Context and Interaction) programming paradigm.

In DCI, a Role is an identifier, like the local variables you're creating, but at the class-level instead, having objects passed to an instance of the class (called the Context) playing the Role of this identifier. For example, if the Context is called Writing, an object can act as one of the Roles if its type satisfies the interface defined for that Role:

  • An object Pen plays the Role instrument
  • An object Human plays the Role writer
  • An object Paper plays the Role material

This connects to how the users reason about the system, using mental models with certain terminology that should be reflected in the code, instead of programmer abstractions and engineering concepts.

So to answer your question, it's definitely OK if it helps convey a relevant mental model for that part of the system.

-3

It would depend on the language, its semantics, the compiler's optimizer, and programmer idioms.

In C++, you can declare an alias that can be used that way with no penalty in the generated code.

The language you're showing seems to just assign/initialize one name with another. Does the language have reference semantics for variables and parameters? Does your use of final mean something like const? In that case, will the compiler and optimizer track that this is in fact an exact alias, or will it generate worse code because you have two "pointers" of the same type now that may or may not refer to the same place?

As for globals, you might want to make a local copy, not a local alias name. That way the compiler knows nothing else is affecting it and it can keep the value in a register or optimize across function calls because it knows everything that's reading and writing it.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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