2

This is a problem I run into often, and am looking for the best solution. I will have code like this (python):

def func(var, opt):
    if opt:
        var = var.set_opt(opt)

    result = var.get_result()

    if opt:
        return [r[0] for r in result] # arbitrary
    else:
        return result

What is a good, general practice, way to avoid this double if statement?

Robert Harvey
  • 198,589
  • 55
  • 464
  • 673
shane
  • 137
  • 1
  • 3
  • 1
    Possible duplicate of [How to avoid repeating a condition in methods that use it differently?](https://softwareengineering.stackexchange.com/questions/333361/how-to-avoid-repeating-a-condition-in-methods-that-use-it-differently) – gnat Aug 07 '17 at 19:41
  • Does `var.set_opt(opt)` change what `var.get_result()` returns? – Bryan Oakley Aug 08 '17 at 16:36

3 Answers3

3

For this case, I think the following makes the most sense. Your need for the two ifs is based on trying to reuse the same unnecessary variable in two distinct execution paths.

def func(var, opt):
    if opt:
        return [r[0] for r in var.set_opt(opt).get_result()]
    else:
        return var.get_result()

I don't know if there is a general rule but this tends to be a smell. I'd rather see to independent blocks of code with some repetition than code where separate paths are weaved together like this.

JimmyJames
  • 24,682
  • 2
  • 50
  • 92
3

We can resolve the else first, which in my opinion, is the problem. I didn't see any reason to leave it for the end.

def func(var, opt):
    if opt is None:
        return var.get_result()

    result = var.set_opt(opt).get_result()
    return [r[0] for r in result] # arbitrary

I'm afraid I can't give you a more sophisticated answer because it would depends on the real code and whether the order of the if/else can be altered for a quick resolution of the function.

I'm not familiar with Python. I have searched a bit and I came to the conclusion that if opt is None is the opposite to if opt. Feel free of correct me if I'm wrong.

Laiv
  • 14,283
  • 1
  • 31
  • 69
0

If this is your entire function, you can just break it into two pieces:

def func(var):
    return result = var.get_result()

and

def funcWithOpt(var, opt):
    var = var.set_opt(opt)
    result = var.get_result()
    return [r[0] for r in result]

If it's longer, you can use subfunctions for the different parts.

However, you should begin with asking yourself if this is a problem at all. Performance is not an issue, checking if opt exists is inexpensive. If you have a boolean function you need to check, you can just call that function once and 'cache' it in a variable. The function might be slightly longer with this double if statement, but it's readable and that definitely counts for something.

Glorfindel
  • 3,137
  • 6
  • 25
  • 33