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?

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.

| improve this answer | |
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.

| improve this answer | |
  • But now you are duplicating get_result() which is just shifting the problem, not eliminating it. – DisgruntledGoat Aug 8 '17 at 0:06
  • Where is duplicated? – Laiv Aug 8 '17 at 9:12
  • What is a good, general practice, way to avoid this double if statement?. The problem is the doble if, no the two calls to the get_result() – Laiv Aug 8 '17 at 9:14
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.

| improve this answer | |
  • funcWithOpt does nothing to prevent opt from being none. – candied_orange Aug 7 '17 at 20:30
  • @CandiedOrange ah, blame my lack of Python skills. Is there any way of making it required? – Glorfindel Aug 7 '17 at 20:31
  • Python isn't statically typed. You'd have to test and throw an exception. – candied_orange Aug 7 '17 at 20:35
  • Yes, through if statements, and now you're back to the beginning – whatsisname Aug 7 '17 at 20:35

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