Is defining a variable to name a method argument a good practice

coding-style

For the sake of readability I often find myself defining temporary variables while calling functions, such as the following code

var preventUndo = true;
doSomething(preventUndo);

The shorter version of this to this would be,

doSomething(true);

But when I come back to the code I often wonder what true refers to. Is there a convention for this kind of conundrum?

Best Answer

Explaining Variables

Your case is an example of the introduce explaining variable / extract variable refactoring. In short, an explaining variable is one which is not strictly necessary, but allows you to give a clear name to something, with the aim of increasing readability.

Good quality code communicates intent to the reader; and as a professional developer readability and maintainability are your #1 goals.

As such, the rule of thumb I would recommend is this: if your parameter's purpose is not immediately obvious, feel free to use a variable to give it a good name. I think this is a good practice in general (unless abused). Here's a quick, contrived example - consider:

editButton.Enabled = (_grid.SelectedRow != null && ((Person)_grid.SelectedRow).Status == PersonStatus.Active);

versus the slightly longer, but arguably clearer:

bool personIsSelected = (_grid.SelectedRow != null);
bool selectedPersonIsEditable = (personIsSelected && ((Person)_grid.SelectedRow).Status == PersonStatus.Active)
editButton.Enabled = (personIsSelected && selectedPersonIsEditable);

Boolean Parameters

Your example actually highlights why booleans in APIs are often a bad idea - on the calling side, they do nothing to explain what's happening. Consider:

ParseFolder(true, false);

You'd have to look up what those parameters mean; if they were enums, it'd be a lot more clear:

ParseFolder(ParseBehaviour.Recursive, CompatibilityOption.Strict);

Edit:

Added headings and swapped the order of the two main paragraphs, because too many people were focusing on the boolean parameters part (to be fair, it was the first paragraph originally). Also added an example to the first part.

Related Solutions

Old Coding Styles – Is It Normal for Programmers to Use Old Coding Styles?

Ok, I'll be blunt. That is a bad place to work... I've been in such situations, and usually it ends with you being swallowed by this code. After a year or so, you'll get used to it, and you'll loose the grip on how modern alternatives can be used to achieve the same task easier, in a more maintainable way, and also, faster at run-time in most cases.

I left a workplace like that, because, after just a month, I felt I'm being dragged into an old skool code. I tried to give it a go, but decided not to. I couldn't use clean code and started to loose skills because of missing everyday practice. Any modern approach had to be approved by 3 layers of developers, which never happened, because the idea was that things might break when modern approaches are used. And fragile nature of the code that comes out when you don't use modern approaches is quite scary.

Don't get me wrong, there are cases where people over-engineer solutions, and I'm all against it. But being dragged into '80 coding conventions and style for extensive amount of time will stop your progress, and also, I think, career opportunities.

Then again, you have to earn money, so sometimes you have to do what you don't exactly like. Keep an eye on burnout symptoms and turning coding into a mundane task in those cases though.

A HashMap be used(in functions) to determine which value to return(for a key) when an if else construct can do the job in better time

The point is to move the creation of the hashmap outside the function and do it once (or just less times than otherwise).

private static final Map<String, Integer> map;
static{
    Map<String, Integer> temp = new HashMap<String, Integer>();
    temp.put("A", 12);
    temp.put("B", 21);
    temp.put("C", 45);
    map = Collections.unmodifiableMap(temp);//make immutable
}

int foo(String s){
    if(!map.containsKey(s))
        throw new RuntimeException("Invalid input to function foo");

    return map.get(s);
}

However java has since java7 been able to have (final) strings in switches:

int foo(String s){
    switch(s){
    case "A":
        return 12;
    case "B": 
        return 21;
    case "C": 
        return 45;
    default: throw new RuntimeException("Invalid input to function foo");
}