how to format long if conditions

[Arnaud Delobelle]

Hi all,

I'm wondering what advice you have about formatting if statements with
long conditions (I always format my code to <80 colums)

Here's an example taken from something I'm writing at the moment and
how I've formatted it:


if (isinstance(left, PyCompare) and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]):
py_and = PyCompare(left.complist + right.complist[1:])
else:
py_and = PyBooleanAnd(left, right)

What would you do?

 

[Steven D'Aprano]

Hi all,

I'm wondering what advice you have about formatting if statements with
long conditions (I always format my code to <80 colums)

Here's an example taken from something I'm writing at the moment and
how I've formatted it:


if (isinstance(left, PyCompare) and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]):
py_and = PyCompare(left.complist + right.complist[1:])
else:
py_and = PyBooleanAnd(left, right)

What would you do?

I believe that PEP 8 now suggests something like this:

if (
isinstance(left, PyCompare) and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]):
)
py_and = PyCompare(left.complist + right.complist[1:]
else:
py_and = PyBooleanAnd(left, right)


I consider that hideous and would prefer to write this:


if (isinstance(left, PyCompare) and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]):
py_and = PyCompare(left.complist + right.complist[1:]
else:
py_and = PyBooleanAnd(left, right)


Or even this:

tmp = (
isinstance(left, PyCompare) and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0])
)
if tmp:
py_and = PyCompare(left.complist + right.complist[1:]
else:
py_and = PyBooleanAnd(left, right)


But perhaps the best solution is to define a helper function:

def is_next(left, right):
"""Returns True if right is the next PyCompare to left."""
return (isinstance(left, PyCompare) and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0])
# PEP 8 version left as an exercise.


# later...
if is_next(left, right):
py_and = PyCompare(left.complist + right.complist[1:]
else:
py_and = PyBooleanAnd(left, right)

 

[Hans Mulder]

I'm wondering what advice you have about formatting if statements with
long conditions (I always format my code to<80 colums)

Here's an example taken from something I'm writing at the moment and
how I've formatted it:


if (isinstance(left, PyCompare) and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]):
py_and = PyCompare(left.complist + right.complist[1:])
else:
py_and = PyBooleanAnd(left, right)

What would you do?

I would break after the '(' and indent the condition once and
put the '):' bit on a separate line, aligned with the 'if':


if (
isinstance(left, PyCompare)
and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]
):
py_and = PyCompare(left.complist + right.complist[1:])
else:
py_and = PyBooleanAnd(left, right)

It may look ugly, but it's very clear where the condition part ends
and the 'then' part begins.

-- HansM

 

[Steven D'Aprano]

Hans Mulder wrote:

[...]

It may look ugly, but it's very clear where the condition part ends
and the 'then' part begins.

Immediately after the colon, surely?

 

[Arnaud Delobelle]

Hi all,

I'm wondering what advice you have about formatting if statements with
long conditions (I always format my code to <80 colums)

Here's an example taken from something I'm writing at the moment and
how I've formatted it:


        if (isinstance(left, PyCompare) and isinstance(right, PyCompare)
                and left.complist[-1] is right.complist[0]):
            py_and = PyCompare(left.complist + right.complist[1:])
        else:
            py_and = PyBooleanAnd(left, right)

What would you do?

I believe that PEP 8 now suggests something like this:

       if (
               isinstance(left, PyCompare) and isinstance(right, PyCompare)
               and left.complist[-1] is right.complist[0]):
           )
           py_and = PyCompare(left.complist + right.complist[1:]
       else:
           py_and = PyBooleanAnd(left, right)


I consider that hideous and would prefer to write this:


       if (isinstance(left, PyCompare) and isinstance(right, PyCompare)
           and left.complist[-1] is right.complist[0]):
           py_and = PyCompare(left.complist + right.complist[1:]
       else:
           py_and = PyBooleanAnd(left, right)


Or even this:

       tmp = (
           isinstance(left, PyCompare) and isinstance(right, PyCompare)
           and left.complist[-1] is right.complist[0])
           )
       if tmp:
           py_and = PyCompare(left.complist + right.complist[1:]
       else:
           py_and = PyBooleanAnd(left, right)


But perhaps the best solution is to define a helper function:

def is_next(left, right):
   """Returns True if right is the next PyCompare to left."""
   return (isinstance(left, PyCompare) and isinstance(right, PyCompare)
       and left.complist[-1] is right.complist[0])
   # PEP 8 version left as an exercise.


# later...
       if is_next(left, right):
           py_and = PyCompare(left.complist + right.complist[1:]
       else:
           py_and = PyBooleanAnd(left, right)

Thanks Steven and Hans for you suggestions. For this particular
instance I've decided to go for a hybrid approach:

* Add two methods to PyCompare:

class PyCompare(PyExpr):
...
def extends(self, other):
if not isinstance(other, PyCompare):
return False
else:
return self.complist[0] == other.complist[-1]
def chain(self, other):
return PyCompare(self.complist + other.complist[1:])

* Rewrite the if as:

if isinstance(right, PyCompare) and right.extends(left):
py_and = left.chain(right)
else:
py_and = PyBooleanAnd(left, right)


The additional benefit is to hide the implementation details of
PyCompare (I suppose this could illustrate the thread on when to
create functions).

 

[Hans Mulder]

Hans Mulder wrote:

[...]

It may look ugly, but it's very clear where the condition part ends
and the 'then' part begins.

Immediately after the colon, surely?

On the next line, actually

The point is, that this layout makes it very clear that the colon
isn't in its usual position (at the end of the line that starts
with 'if') and it is clearly visible.

With the layout Arnaud originally propose, finding the colon takes
longer. (Arnaud has since posted a better approach, in which the
colon is back in its usual position.)

-- HansM

 

[Colin J. Williams]

I'm wondering what advice you have about formatting if statements with
long conditions (I always format my code to<80 colums)

Here's an example taken from something I'm writing at the moment and
how I've formatted it:


if (isinstance(left, PyCompare) and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]):
py_and = PyCompare(left.complist + right.complist[1:])
else:
py_and = PyBooleanAnd(left, right)

What would you do?

I would break after the '(' and indent the condition once and
put the '):' bit on a separate line, aligned with the 'if':


if (
isinstance(left, PyCompare)
and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]
):
py_and = PyCompare(left.complist + right.complist[1:])
else:
py_and = PyBooleanAnd(left, right)

It may look ugly, but it's very clear where the condition part ends
and the 'then' part begins.

-- HansM

What about:
cond= isinstance(left, PyCompare)
and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]
py_and= PyCompare(left.complist + right.complist[1:])if cond
else: py_and = PyBooleanAnd(left, right)
Colin W.

 

[Hans Mulder]

What about:
cond= isinstance(left, PyCompare)
and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]
py_and= PyCompare(left.complist + right.complist[1:])if cond
else: py_and = PyBooleanAnd(left, right)
Colin W.

That's a syntax error. You need to add parenthesis.

How about:

cond = (
isinstance(left, PyCompare)
and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]
}
py_and = (
PyCompare(left.complist + right.complist[1:])
if cond
else PyBooleanAnd(left, right)
)

-- HansM

 

[Roy Smith]

Hi all,

I'm wondering what advice you have about formatting if statements with
long conditions (I always format my code to <80 colums)
[...]
if (isinstance(left, PyCompare) and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]):
py_and = PyCompare(left.complist + right.complist[1:])
else:
py_and = PyBooleanAnd(left, right)

To tie this into the ongoing, "When should I write a new function?"
discussion, maybe the right thing here is to refactor all of that mess
into its own function, so the code looks like:

if _needs_compare(left, right):
py_and = PyCompare(left.complist + right.complist[1:])
else:
py_and = PyBooleanAnd(left, right)

and then

def _needs_compare(left, right):
"Decide if we need to call PyCompare"
return isinstance(left, PyCompare) and \
isinstance(right, PyCompare) and \
left.complist[-1] is right.complist[0]

This seems easier to read/understand than what you've got now. It's an
even bigger win if this will get called from multiple places.

 

[Colin J. Williams]

What about:
cond= isinstance(left, PyCompare)
and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]
py_and= PyCompare(left.complist + right.complist[1:])if cond
else: py_and = PyBooleanAnd(left, right)
Colin W.

That's a syntax error. You need to add parenthesis.

How about:

cond = (
isinstance(left, PyCompare)
and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]
}
py_and = (
PyCompare(left.complist + right.complist[1:])
if cond
else PyBooleanAnd(left, right)
)

-- HansM

I like your 11:53 message but suggest indenting the if cond as below to
make it clearer that it, with the preceding line, is all one statement.

Colin W.

#!/usr/bin/env python
z= 1
class PyCompare:
complist = [True, False]
def __init__(self):
pass
left= PyCompare
right= PyCompare
def isinstance(a, b):
return True
def PyBooleanAnd(a, b):
return True
def PyCompare(a):
return False
z=2

def try1():

''' Hans Mulder suggestion 03:50 '''
if (
isinstance(left, PyCompare)
and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]
):
py_and = PyCompare(left.complist + right.complist[1:])
else:
py_and = PyBooleanAnd(left, right)

def try2():
''' cjw response - corrected 11:56 '''
cond= (isinstance(left, PyCompare)
and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0])
py_and= (PyCompare(left.complist + right.complist[1:]) if cond
else PyBooleanAnd(left, right))

def try3():
''' Hans Mulder 11:53 '''
cond = (
isinstance(left, PyCompare)
and isinstance(right, PyCompare)
and left.complist[-1] is right.complist[0]
) # not }
py_and = (
PyCompare(left.complist + right.complist[1:])
if cond
else PyBooleanAnd(left, right)
)
def main():
try1()
try2()
try3()
if __name__ == '__main__':
main()
pass