Use whichever one you think is more readable and maintainable. Depending on the language, variables and function calls involved, either one might be preferable. If I had to evaluate your two examples in isolation, I would have a slight preference for the second version because it makes it a little more obvious that do_something() only gets called once.

But in the context of the rest of your codebase, some “litmus test” questions like these may help with evaluating those criteria:

• Is it more intuitive to say “if [condition] is true, [variable] should be 1” or “if [condition] is true, we should do_something(1)”?
• Does the value of [variable] have a meaning of its own, or is it only meaningful as an argument to do_something()?
• When this block of code has to change in the future, is it more likely that the branches will end up assigning different values to the variable(s), or that the branches will end up calling different functions?
• What are the odds we’ll ever want to call do_something() more than once in this block of code?
• Will it ever make sense to move this branching logic inside of do_something(), rendering this entire question pointless?
• How complicated could [condition] and [other_condition] potentially become?

Imagine a concrete example of a code which, given a price of an item, should call a method while specifying if the item is a rebate (price inferior to zero), a paid product (price superior to zero) or a free product (price equal to zero).

Your two pieces of code become:

Solution 1:

if (price < 0)
{
    this.DoSomething(PriceType.Rebate);
}
else if (price > 0)
{
    this.DoSomething(PriceType.Product);
}
else // price == 0
{
    this.DoSomething(PriceType.Free);
}

Solution 2:

priceType = PriceType.Free;

if (price < 0)
{
    priceType = PriceType.Rebate;
}
else if (price > 0)
{
    priceType = PriceType.Product;
}

this.DoSomething(priceType);

The second solution, as is, has less lines (10 versus 12), despite your assertion that “it takes up more lines of code”, but the first one looks more readable. On the other hand, the second solution can be refactored to become much more readable:

Refactoring of solution 2:

private PriceType FindPriceType(price)
{
    if (price < 0)
    {
        return PriceType.Rebate;
    }
    else if (price > 0)
    {
        return PriceType.Product;
    }

    return PriceType.Free;
}

...

this.DoSomething(this.FindPriceType(price));

By separating the original method into two methods, the code becomes more readable through the introduction of an explicit name of the method.

There are also two specific cases where the second solution allows even more benefits:

Specific cases

The first case is the one where all conditions have the form: if (something == value). For instance:

if (priceType == 1)
{
    return PriceType.Rebate;
}

if (priceType == 2)
{
    return PriceType.Product;
}

if (priceType == 3)
{
    return PriceType.Free;
}

throw new NotImplementedException();

can be transformed into a much shorter:

var map = {
    1: PriceType.Rebate,
    2: PriceType.Product,
    3: PriceType.Free
}[priceType];

A second specific case is where you have not three branches, but only one if/else. Example:

Solution 1:

if (price < 0)
{
    this.DoSomething(PriceType.Rebate);
}
else
{
    this.DoSomething(PriceType.Product);
}

Solution 2:

priceType = PriceType.Product;

if (price < 0)
{
    return PriceType.Rebate;
}

this.DoSomething(priceType);

Here, the second solution can (in many languages) be transformed into a one-liner:

this.DoSomething(price < 0 ? PriceType.Rebate : PriceType.Product);

I recommend separating the concerns of the do_something and the conditions. There is some business logic that should be well named and encapsulated to determine what the do_something will operate on (ConditionType in my example below). Use the resulting ConditionType as a parameter to do_something.

I read a lot of code (more than I write) and I’m not a big fan of putting as much code into one line as possible. It’s too easy to miss some crucial step when scanning the code.

Also, use constants, enums or your favorite stuc to avoid magic numers (0,1,2 in this case).

Here’s a code sample:

   const int ConditionTypeZero = 0;
   const int ConditionTypeOne = 1;
   const int ConditionTypeDefault = 2;

…

        var conditionType = GetConditionType(condition, otherCondition);
        do_something(conditionType);

…

    private int GetConditionType(bool condition, bool otherCondition )
    {
        if (condition)
        {
            return ConditionTypeZero;
        }
        if (otherCondition)
        {
            return ConditionTypeOne;
        }
        return ConditionTypeDefault;
    }