0
\$\begingroup\$

I'm looking for a much simpler way of formatting a string in Python which will have a different number of replacement fields in different cases. Here's what I'm doing now, which is working fine:

if '{yes}' in reply and '{value}' in reply:
    reply = reply.format(yes=get_yes(), value=value)
elif '{no}' in reply and '{value}' in reply:
    reply = reply.format(no=get_no(), value=value)
elif '{yes}' in reply:
    reply = reply.format(yes=get_yes())
elif '{no}' in reply:
    reply = reply.format(no=get_no())
elif '{value}' in reply:
    reply = reply.format(value=value)

The only problem is that this code has a Cognitive Complexity of 11 on Code Climate, which is higher than the allowed value of 5, and so I'm trying to find out a way of reducing it.

Additional information about variables and methods

  • reply is a string with will have one of the following combinations of replacement fields:
    • {yes} and {value}
    • {no} and {value}
    • {yes} only
    • {no} only
    • {value} only
    • no replacement field
  • get_yes() randomly returns a string that has the same meaning as "yes" ("yeah", "yep" etc.)
  • get_no() randomly returns a string that has the same meaning as "no" ("nah", "nope" etc.)
  • value is a numeric value (integer or float)
\$\endgroup\$
  • \$\begingroup\$ Would str.format_map() be helpful in some way? \$\endgroup\$ – Faheel Ahmad Nov 29 '17 at 22:23
1
\$\begingroup\$
  • Build a kwargs dictionary, rather than using an if-else.
  • You don't have to treat yes and no as mutually exclusive.
  • You could use a 'key, function' list to create a dictionary comprehension that builds the dictionary for you.

This can lead to:

kwargs = {}
if '{value}' in reply:
    kwargs['value'] = value
if '{yes}' in reply:
    kwargs['yes'] = get_yes()
if '{no}' in reply:
    kwargs['no'] = get_no()
reply = reply.format(**kwargs)
params = [
    ('value', lambda:value),
    ('yes', get_yes),
    ('no', get_no)
]

def format_format(format, params):
    return format.format({
        key: fn()
        for key, fn in params
        if f'{{{key}}}' in format
    })

reply = format_format(reply, params)
\$\endgroup\$
  • \$\begingroup\$ The first method is really the best way to go in this case. Thanks! \$\endgroup\$ – Faheel Ahmad Nov 30 '17 at 2:49
0
\$\begingroup\$

If I understand the question correctly, there are 3 cases for yes/no/empty, and 2 cases for value/no value.
Given each of those have different outputs, yet the reply formatting is fairly straight forward, most of the complexity has to do with determining what to extract from the reply.
What if we separated both yes/no/empty into a function, and value/no value into another function? That would clarify the logic and implement a Single Responsibility Principal (SRP) for the functions. For instance:

def check_answer(reply):
    if "{yes}" in reply:
        return get_yes()
    if "{no}" in reply:
        return get_no()
    return ""

And something similar for the value:

def check_value(reply):
    if "{value}" in reply:
        return value
    return ""

Unfortunately this would break your reply formatting function, as you have yes= and no= whereas the yes/no/empty is now only a single output.
I'll leave that part up to you to fix (it should be fairly straight-forward). Good luck! (FYI all code is untested)

\$\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.