Why I have to add "None" argument in if statement?

hello :slight_smile:
After few hours of thinking why my code won’t work in exercice: Most Frequent and Least Frequent. I checked the answer. I don’t understand why I have to add the “None” argument in the if statement?
my buggy code:

# provided input
values = [10, 20, 30, 10, 30, 10] 
# answer to this input: 3, 1

def most_least_frequent(values):
    freq = {}
    for n in values:
        if n in freq:
            freq[n] += 1
        else:
            freq[n] = 1
            
#    for key in freq:
#        print(key, ":" , freq[key])
        
    min_freq = None
    max_freq = None

    for key in freq:
        if freq[key] > max_freq:
            max_freq = freq[key]
        if freq[key] < min_freq:
            min_freq = freq[key]
    return max_freq, min_freq
    

print(most_least_frequent(values))

error:

proper answer:

# provided input
values = [10, 20, 30, 10, 30, 10] 
# answer to this input: 3, 1

def most_least_frequent(values):
    freq = {}
    for n in values:
        if n in freq:
            freq[n] += 1
        else:
            freq[n] = 1
            
#    for key in freq:
#        print(key, ":" , freq[key])
        
    min_freq = None
    max_freq = None

    for key in freq:
        if max_freq is None or freq[key] > max_freq:
            max_freq = freq[key]
        if min_freq is None or freq[key] < min_freq:
            min_freq = freq[key]
    return max_freq, min_freq
    

print(most_least_frequent(values))

What happened behind the curtain that made my code failed? Why do I have to add this statement?
I thought it should work anyway ( even if there is no max or min, or both frequencies, it should print “None” without adding it into "if ‘’ statement.

Hi @drill_n_bass,

Your starting values of min_freq and max_freq are both None, i.e. not numbers (not float or int). In your for-loop you compare each value of the dictionary with these values. Since you cannot compare numbers and None values, being different data types (exactly as it is written in your TypeError), you have to check first if they are None (which is actually the case, because it’s their initial condition). Already after the first iteration, they both will not be None anymore, and only the second part of both if-conditions will be checked. But for the very first iteration, we need to check if they are None to re-write them, because they actually are.

You can use an alternative approach here: to assign the initial min_freq deliberately very high and the initial max_freq - artificially very low, like in the piece of code below

min_freq = 100000000000000
max_freq = 0

for key in freq:
    if freq[key] > max_freq:
        max_freq = freq[key]
    if freq[key] < min_freq:
        min_freq = freq[key]
return max_freq, min_freq

The result will be the same, and you will not have to check if the values are None.

2 Likes

I was thinking about this approach, but the best max should be infinity ( I don’t know how to put it into code). Also, when I was trying to do this, I put min_freq = 0 , and max_freq = 1000000000. Because of that, I had a problem creating proper if statement for min_freq. ( I missed this idea of opposite values that you mentioned above :smiley: )

1 Like

There are several ways of how to express infinity and -infinity in Python, here is a good GeeksForGeeks’ article about it. I personally have never used any them, being afraid of potential errors in the further code lines (discrepancies in the types of data, for example) and choosing more conservative approaches instead. But I think indeed all these methods are good, it’s only my irrational fears :blush:

What about initiate min_freq and max_freq with 0 ? Sounds more elegant to me than with None. And you can shorter the code:

    for key in freq:
       if freq[key] > max_freq:
            max_freq = freq[key]
        if freq[key] < min_freq:
            min_freq = freq[key]

Edit: Oh wait it will not work with min_freq of course ! Ignore this post :slight_smile:

Well you can set min_freq to len(values) + 1 since by definition max_freq<=len(values)

1 Like

Hi all.

Good discussion so far. However, I feel the manual iteration approach for figuring out the highest and lowest frequency makes things actually too compliacted. You have built-in methods, which can take care of this scenario with ease. dictionary.values() gives you in essence a list with all values (here:frequencies) contained in the dictionary. And then you can use the max() function to retrieve the largest frequency from this object. Full code:

max_freq = max(frequencies.values())

Best
htw

1 Like

I like this approach! thank you :slight_smile:

May you show me what would full code looks like?

1 Like

Thanks a lot guys @htw @WilfriedF, very good suggestions! I will also take them in consideration for further tasks.

1 Like

Sure.

def most_least_frequent(values):
    frequencies = {}
    for v in values:
        if v in frequencies:
            frequencies[v] += 1
        else:
            frequencies[v] = 1
    
    max_freq = max(frequencies.values())
    min_freq = min(frequencies.values())

    return max_freq, min_freq

Just for clarification: There is definitive some educational value when solving this with for-loops on your own. For learning purposes this is perfectly fine. However, I am a strong advocate for using functions/methods that are already built-in, because these are well tested and robust. It is a good practice to check the documentation from time to time to see what is there/possible.

Best
htw