CODEWITHSUNDEEP
pythonif-statement
f0body
f0body

Reputation: 33

Pythonic way to do if statement comparisons only if the variable is not None

Is there a more pythonic way to do this if block?

def is_notification_needed(request: RiverPreference, gauge: Gauge) -> bool:
    """
    Decides whether an email should be sent to the user or not based on their provided preferences.
    :param request: RiverPreference dataclass
    :param gauge: Gauge dataclass
    :returns: boolean
    """
    for preference in request:
        if preference.level and preference.trend and preference.level >= gauge.level and preference.trend == gauge.trend:
            return True
        elif preference.level and not preference.trend and preference.level >= gauge.level:
            return True
        elif preference.trend and not preference.level and preference.trend == gauge.level:
            return True
        return False

preference.trend and preference.level default to None when the user doesn't want to set it. I'd like to be able to check these values against the gauge values only if they are set.

Upvotes: 1

Views: 109

Answers (4)

Cireo
Cireo

Reputation: 4427

First off, the pythonic way to compare would be is not None, which is important if your values can be falsy (0).

Second off, your indentation looks off, and there is a typo in logic, but I think your goal is to return True if all conditions for any preference are met, and False otherwise, this would be simpler by checking negatives.

    for preference in request:
        if preference.level is not None and preference.level < gauge.level:
            continue
        if preference.trend is not None and preference.trend != gauge.trend:
            continue
        return True  # NB: treats all-None as a match
    return False

Upvotes: 1

Jasmijn
Jasmijn

Reputation: 10477

There are many logically equivalent ways to do this, but my preference in this case would be to invert the logic and try to figure out when you need to return False, because we now have no more than one and or or per condition, which makes a large difference to how easy to maintain code is in my experience.

(I assume RiverPreference works as a sequence of preference objects?)

def is_notification_needed(request: RiverPreference, gauge: Gauge) -> bool:
    """
    Decides whether an email should be sent to the user or not based on their provided preferences.
    :param request: RiverPreference dataclass
    :param gauge: Gauge dataclass
    :returns: boolean
    """
    preference = request[0]
    if preference.level and preference.level < gauge.level:
         return False
    if preference.trend and preference.trend != gauge.level:
         return False
    return preference.level or preference.trend

(You may need to add is None to the checks for preference.level and preference.trend if 0 or 0.0 can be a valid value for the gauge levels.)

Upvotes: 0

tripleee
tripleee

Reputation: 189936

I don't know if this is more Pythonic, but I certainly find it easier to understand at a glance.


def is_notification_needed(request: RiverPreference, gauge: Gauge) -> bool:
    """
    Decides whether an email should be sent to the user or not based on their provided preferences.
    :param request: RiverPreference dataclass
    :param gauge: Gauge dataclass
    :returns: boolean
    """
    preference = request[0]
    if preference.trend is None:
        if preference.level is None:
            return False
        return preference.trend >= gauge.level
    elif preference.level is None:
        return preference.trend == gauge.level
    return preference.level >= gauge.level and preference.trend == gauge.trend

The main principle here is to get rid of the None cases as early as possible, and then rely in the subsequent code on the fact that we know these values are defined.

If the values could be falsey for other reasons too, probably change the is None to something less specific.

Upvotes: 1

inof
inof

Reputation: 495

First, it’s better to explicitly compare with None, instead of “abusing” the boolean value of None. Remember “explicit is better than implicit” from the Zen of Python (PEP-20). That is,

if preference.level is not None and ...

is better than just

if preference.level and ...

On the other hand, you can make use of the fact that None has a boolean False value by doing this:

if (preference.level or 0) >= gauge.level:

instead of:

if preference.level and preference.level >= gauge.level:

Of course, you should specify an appropriate value instead of 0 (depending on what values gauge.level can have); this is just an example.

Using the or operator to provide a default value for a variable that can be None (or False, or any other boolean False equivalent) is actually used quite often in Python programs, and I would call this a “Pythonic” way to express this.

By the way, your for loop is meaningless, because your loop body always returns on the first pass through the loop.

Upvotes: 1

Related Questions