4
\$\begingroup\$

Given this code

if account_id:
    form = form.replace("<<ACCOUNT_ID>>", "<accountId>"+account_id+"</accountId>")
else:
    form = form.replace("<<ACCOUNT_ID>>", "")

I was wondering if I could remove the else tag to make the code simpler:

account_id_tag = ""
if account_id:
    account_id_tag = "<accountId>" + account_id + "</accountId>"
form = form.replace("<<ACCOUNT_ID>>", account_id_tag)

Or even using a ternary operator:

account_id_tag = "<accountId>" + account_id + "</accountId>" if account_id else ""
form = form.replace("<<ACCOUNT_ID>>", account_id_tag)

But even the last one uses less lines, I have the sensation that the original option (with else) is easy to read and to understand, and this is very important for my point of view.

\$\endgroup\$
  • 4
    \$\begingroup\$ This could be a much better question if you showed us the code with more context. What does the entire form template look like, and what other replacements are you performing? (I sense that this entire thing is a hack that should be rewritten altogether.) \$\endgroup\$ – 200_success Nov 17 '17 at 12:23
4
\$\begingroup\$

Honestly, it seems like you need a function. One thing they all have in common is form = form.replace("<<ACCOUNT_ID>>", ...). And so you can't simplify that any more. And so I'd just focus on the turnery. Leaving:

if account_id:
    account_id_tag = "<accountId>"+account_id+"</accountId>"
else:
    account_id_tag = ""
account_id_tag = ""
if account_id:
    account_id_tag = "<accountId>"+account_id+"</accountId>"
account_id_tag = "<accountId>"+account_id+"</accountId>" if account_id else ""

And so I think the last is the simplest.

If you use this a lot, then it'd be best to make this a function. And so you will have to make the tag a variable. And so the simplest would be to use str.format, or f-strings (Python 3.6+):

def make_tag(value, tag):
    return ('<{tag}>{value}</{tag}>' if value else '').format(value=value, tag=tag)

def make_tag(value, tag):
    return f'<{tag}>{value}</{tag}>' if value else ''

And so I'd personally use:

def make_tag(value, tag):
    return f'<{tag}>{value}</{tag}>' if value else ''

form = form.replace('<<ACCOUNT_ID>>', make_tag(account_id, 'accountId'))
\$\endgroup\$
4
\$\begingroup\$

There is not "you choose this" rule in this context. But to be able to decide, you should take few elements in consideration:

  1. Are you working on an existing code (which is not yours)? If so, it would be better to follow the philosophy of the existing code, or ask the project leader in case of doubt.
  2. If this is a new project, then you could opt for the third option which was first proposed on PEP 0308 because, in general, typing less makes you less error prone.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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