Boolean Operators are not working consistently. Prints "a" correctly, but prints "b" incorrectly

Question

So, I want to write this program where I define a class, and the method within it takes a list of colours as an argument. The colours "red", "Red, "green" and "Green" should be replaced with "black", "Black", "white" and "White" (see the dictionary "self.colour_replacement" in the code-text).

If only one of the two colors red/green regardsless of capital letter, is in the list, the program should only return the list without changing it.

Example_1:
print(c.make_readable(['Green', 'Green', 'pink', 'green', 'yellow', 'green', 
'green']))

should return: 
['Green', 'Green', 'pink', 'green', 'yellow', 'green', 'green']


Example_2: 
print(c.make_readable(['green', 'Green']))

should return:
['green', 'Green']

I think the problem has to do with my "or" and "and" statements in the line:

if ('red' and 'green') in colours or ('Red' and 'green') in colours or 
('red' and 'Green') in colours or ('Red' and 'Green') in colours:

but I am not entirely sure.

class ColourChanger:
    def __init__(self):
    """A dictionary that shows the 
    replacement colours."""
        self.colour_replacement = {'Green' : 'White', 'red': 'black', 
    'green': 'white', 'Red' : 'Black'}

    def make_readable(self, colours):
        result = []    
        if ('red' and 'green') in colours or ('Red' and 'green') in colours or 
        ('red' and 'Green') in colours or ('Red' and 'Green') in colours:
            for col in colours:
                if col in self.colour_replacement:
                    result.append(self.colour_replacement[col]) """Appends the 
                   "result"-list with the replacement color instead of the color (col)."""
                else:
                    result.append(col)

        else:
            return colours

        return result


c = ColourChanger()
print(c.make_readable(['green', 'Green']))
print(c.make_readable(['Red', 'red']))
print(c.make_readable(['Red', 'Red', 'pink', 'red', 'yellow', 'red', 'red', 
'Green']))
print(c.make_readable(['Green', 'Green', 'pink', 'green', 'yellow', 'green', 
'green']))


Expected output:
['green', 'Green']
['Red', 'red']
['Black', 'Black', 'pink', 'black', 'yellow', 'black', 'black', 'White']
['Green', 'Green', 'pink', 'green', 'yellow', 'green', 'green']

Actual Output: 
['white', 'White']
['Red', 'red']
['Black', 'Black', 'pink', 'black', 'yellow', 'black', 'black', 'White']
['White', 'White', 'pink', 'white', 'yellow', 'white', 'white']

Make your colors all uppercase or all lowercase first, and you'll avoid all of those comparison combinations that differ only in case. — Robert Harvey, Nov 11 '18 at 15:52
Also, for what it's worth, I think you can just skip that `if` altogether. — Robert Harvey, Nov 11 '18 at 15:53
I looked up the duplicate and it also solved my problem. Guys, I thank you for the help. — Chris Banniq, Nov 11 '18 at 17:16

Patrick Artner · Answer 1 · 2018-11-11T17:29:30.653

Your condition is wrong, to fix it use

if ('red' in colours and 'green' in colours) or ( ...)  .... :

but its not needed - as much is shared in the dupe.

You can avoid most of your ifs alltogether. There is very few benefit in checking first if something is in the list and then conditionally build the replacement to return it. You might sometimes omit building the replacement list which can be a benefit if your lists are huge but you will still have to iterate each list once to its full length to "see" this. You will also need to do this for all lists that would need to get a substitute list to be built - thouse would be traversed more then once - eating up all the "time" you saved for those that do not need replacements. For short(ish) lists its easier to simply build the replacement every time:

It is easier to build the "replace" on the first run through your data:

class ColourChanger:
    def __init__(self):
        # A dictionary that shows the replacement colours - only lowercase needed
        self.colour_replacement = {'green' : 'white', 'red': 'black'}

    def make_readable(self, colours):
        result = []

        # iterate all col's, append them if not in dict, else append the replacement
        for col in colours:
            replace = self.colour_replacement.get(col.lower(), col)

            # adjust case-ness by string-splicing with uppercase where needed
            if col[0].isupper() and replace[0].islower(): 
                replace = replace[0].upper()+replace[1:]

            result.append(replace)

        return result



c = ColourChanger()
print(c.make_readable(['green', 'Green']))
print(c.make_readable(['Red', 'red']))
print(c.make_readable(['Red', 'Red', 'pink', 'red', 'yellow', 'red', 'red', 
'Green']))
print(c.make_readable(['Green', 'Green', 'pink', 'green', 'yellow', 'green', 
'green']))

Output:

['white', 'White']
['Black', 'black']
['Black', 'Black', 'pink', 'black', 'yellow', 'black', 'black', 'White']
['White', 'White', 'pink', 'white', 'yellow', 'white', 'white']

See Why dict.get(key) instead of dict[key]? for why using .get(key, default).

Thank you so much for answer. That is a really good tip. I have really been thinking about this one. But it works now. Thank you! — Chris Banniq, Nov 11 '18 at 17:15

Boolean Operators are not working consistently. Prints "a" correctly, but prints "b" incorrectly

1 Answers1