How to split a statement into multiple lines?

Question

What would be the proper Pythonic way to break the first line of the below expression (to appear on multiple lines) so it can be more readable:

if props.getProperty("app.auth.idp.strategy") == '' or props.getProperty("app.auth.idp.strategy") == 'saml/simpleSAMLphp' and PROXYING_MECHANISM == "ngrok":
    IDP_STRATEGY = "saml/simpleSAMLphp"
elif props.getProperty("app.auth.idp.strategy") == 'saml/gsuite':
    IDP_STRATEGY = "saml/gsuite"
elif props.getProperty("app.auth.idp.strategy") == 'saml/remote-simpleSAMLphp':
    IDP_STRATEGY = "saml/remote-simpleSAMLphp"
else:
     IDP_STRATEGY = "saml"

add `\\` (escape character) at every point before the new line. — W Stokvis, Jun 07 '18 at 17:02
I struggle sometimes too making stuff look readable, so I think this will be useful as reference once completely solved, for that reason please fix the indentation under else for future reference to others. I agree to other comments that assigning a variable for the reused property will enhance this code too. — d parolin, Jun 07 '18 at 17:31
I just figured that what your example code does is assigning the value of the "...idp.strategy" property as-is for most of the cases, are there more potential exceptions to being a value of "" or completely different values that you default to "saml" ? What do you want as a result if you have 'saml/simpleSAMLphp' but PROXYING_MECHANISM is not 'ngrok', because that would result in 'saml' — d parolin, Jun 07 '18 at 17:39
@dparolin: that's correct, I'm not doing the exhaustive check for all possible permutations (for which, I guess, I'd need to use some parenthesis as well) but `saml` should be correct as a default in that case. — Simeon Leyzerzon, Jun 07 '18 at 17:52

score 3 · Answer 1 · answered Jun 07 '18 at 17:34

Refactor

I would start by not calling props.getProperty("app.auth.idp.strategy") repeatedly. Call it once, and you immediately have less reason to split any lines.

strategy = props.getProperty("app.auth.idp.strategy")
if not strategy or strategy == 'saml/simpleSAMLphp' and PROXYING_MECHANISM == "ngrok":
    IDP_STRATEGY = "saml/simpleSAMLphp"
elif strategy == 'saml/gsuite':
    IDP_STRATEGY = "saml/gsuite"
elif strategy == 'saml/remote-simpleSAMLphp':
    IDP_STRATEGY = "saml/remote-simpleSAMLphp"
else:
     IDP_STRATEGY = "saml"

Line continuation

For the first long line, your options are line continuation, either explicit:

if not strategy or \
   strategy == 'saml/simpleSAMLphp' and \
   PROXYING_MECHANISM == "ngrok":

or implicit, inside parentheses:

if (not strategy or
    strategy == 'saml/simpleSAMLphp' and
    PROXYING_MECHANISM == "ngrok"):

Use lookup, not repeated comparisons

An even better option is to replace a long string of comparisons with a dict lookup:

strategies = {
    "saml/gsuite": "saml/gsuite",
    "saml/remote-simpleSAMLphp": "saml/remote-simpleSAMLphp",
}
if PROXYING_MECHANISM == "ngrok":
    strategies['saml/simpleSAMLphp'] = 'saml/simpleSAMLphp'

IDP_STRATEGY = strategies.get(props.getProperty("app.auth.idp.strategy"), "saml")

And because each key of the dict is just mapped to itself, you can replace that with a simple set lookup.

strategies = {
    "saml/gsuite",
    "saml/remote-simpleSAMLphp",
}
if PROXYING_MECHANISM == "ngrok":
    strategies.add('saml/simpleSAMLphp')

IDP_STRATEGY = "saml"

strategy = props.getProperty("app.auth.idp.strategy")
if strategy in strategies:
    IDP_STRATEGY = strategy

Take your pick which of the last two you find more readable. The dict is more redundant in its definition, but allows a single assignment to IDP_STRATEGY.

Great answer to the use-case the OP is trying to solve, not necessarily a perfect answer for other cases of line continuation that could arise, but those should be covered in other questions I presume, and my feeling is no one really comes here to solve theoretical problems when better practical solutions make them go away. — d parolin, Jun 07 '18 at 17:50

score 1 · Answer 2 · answered Jun 07 '18 at 17:05

prop_var = props.getProperty("app.auth.idp.strategy")    
if prop_var == '' or prop_var == 'saml/simpleSAMLphp' and PROXYING_MECHANISM == "ngrok":
        IDP_STRATEGY = "saml/simpleSAMLphp"
    elif prop_var == 'saml/gsuite':
        IDP_STRATEGY = "saml/gsuite"
    elif prop_var == 'saml/remote-simpleSAMLphp':
        IDP_STRATEGY = "saml/remote-simpleSAMLphp"
    else:
         IDP_STRATEGY = "saml"

You add a \ at the end of each line as stated in the comment. You can also replace each getProperty("app.auth.idp.strategy") with a variable so it's called once.

score 1 · Accepted Answer · answered Jun 07 '18 at 17:05

1

Probably something like this as per PEP8

if props.getProperty("app.auth.idp.strategy") == '' \
        or props.getProperty("app.auth.idp.strategy") == 'saml/simpleSAMLphp' \
        and PROXYING_MECHANISM == "ngrok":
    IDP_STRATEGY = "saml/simpleSAMLphp"
elif props.getProperty("app.auth.idp.strategy") == 'saml/gsuite':
    IDP_STRATEGY = "saml/gsuite"
elif props.getProperty("app.auth.idp.strategy") == 'saml/remote-simpleSAMLphp':
    IDP_STRATEGY = "saml/remote-simpleSAMLphp"
else:
     IDP_STRATEGY = "saml"

answered Jun 07 '18 at 17:05

D Dhaliwal

552
5
23

Combining this with assigning a variable first for `props.getProperty("app.auth.idp.strategy")` looks best to be, however there will always be arguably a disrupting look to it at the double indentation, but nothing really easy to fix. – d parolin Jun 07 '18 at 17:33
You are showing PEP-8's second choice; implicit line continuation is preferred over explicit when it is available. – chepner Jun 07 '18 at 17:37

score 0 · Answer 4 · answered Jun 07 '18 at 17:08

0

You can either break it explicitly with '\' or break it by putting the condition inside a parentheses

if (a
    ==b
    ==c):

answered Jun 07 '18 at 17:08

Ernie Yang

114
5

1

you can but don't. please follow D.S. Dhaliwal's answer based on pep8: break on `or`/`and`, don't split the tests whether you're using parentheses or not – bobrobbob Jun 07 '18 at 17:23

How to split a statement into multiple lines?

4 Answers4

Refactor

Line continuation

Use lookup, not repeated comparisons