7
\$\begingroup\$

I started reading Robert Martin's Clean Code. I'm trying to "translate" all his examples into Python, so I can understand them better since my knowledge of Python is greater than that of Java.

This class formats a count in into a natural English sentence:

Java original code of the book

public class GuessStatisticsMessage {
    private String number;
    private String verb;
    private String pluralModifier;

    public String make(char candidate, int count) {
        createPluralDependentMessageParts(count);
        return String.format(
            "There %s %s %s%s", 
            verb, number, candidate, pluralModifier);
    }

    private void createPluralDependentMessageParts(int count) {
        if (count == 0) {
            thereAreNoLetters();
        } else if (count == 1) {
            thereIsOneLetter();
        } else {
            thereAreManyLetters(count);
        }
    }

    private void thereAreManyLetters(int count) {
        number = Integer.toString(count);
        verb = "are";
        pluralModifier = "s";
    }

    private void thereIsOneLetter() {
        number = "1";
        verb = "is";
        pluralModifier = "";
    }

    private void thereAreNoLetters() {
        number = "no";
        verb = "are";
        pluralModifier = "s";
    }
}

My Python version

class GuessStatsMessage:
    def __init__(self, candidate, count):
        self.candidate = candidate
        self.count = count
        self.__number = self.__verb = self.__plural_modifier = ''

    def make_message(self):
        self.__create_pluraldependant_message_parts()
        guess_message = (f'There {self.__verb} '
                         f'{self.__number} '
                         f'{self.candidate}{self.__plural_modifier} ')
        print(guess_message)

    def __create_pluraldependant_message_parts(self):
        if self.count == 0:
            self.__there_are_no_letters()
        elif self.count == 1:
            self.__there_is_one_letter()
        else:
            self.__there_are_many_letters()

    def __there_are_no_letters(self):
        self.__number = 'no'
        self.__verb = 'are'
        self.__plural_modifier = 's'

    def __there_is_one_letter(self):
        self.__number = '1'
        self.__verb = 'is'
        self.__plural_modifier = ''

    def __there_are_many_letters(self):
        self.__number = str(self.count)
        self.__verb = 'are'
        self.__plural_modifier = 's'

It should be used as follows:

message = GuessStatsMessage('Foo', 10)
message.make_message()
# output: There are 10 Foos 
\$\endgroup\$
6
  • 2
    \$\begingroup\$ For those who don't know the book, it would help to know what is the goal of the original code. Any simplifying assumptions, etc. (no unusual plurals like oxen or mothers-in-law). It looks like a reasonable translation (except make_message prints and the original returned a string). But Python is not Java. For example, in Python one would would define a __str__() method and maybe a __format__() method instead of make_message(). \$\endgroup\$
    – RootTwo
    Commented Aug 8, 2020 at 0:18
  • \$\begingroup\$ Thanks for taking the time to look at it. I will update the question (I don't know if I should call it like that here, in Code Review) to show what is the goal of the original code of Uncle Bob. @RootTwo \$\endgroup\$
    – revliscano
    Commented Aug 8, 2020 at 0:35
  • 2
    \$\begingroup\$ I get it: You translated the Java private methods to __double_underscore in Python. But be careful: Any identifier of the form __spam (at least two leading underscores, at most one trailing underscore) is textually replaced with _classname__spam, where classname is the current class name with leading underscore(s) stripped. But __private names don't work the same way as in Java. Actually I don't think this is a good practice in Python at all. \$\endgroup\$ Commented Aug 8, 2020 at 10:11
  • \$\begingroup\$ Thanks for letting that clear, Peter. I appreciate it. I knew about it, and about name mangling, but I don't really know another way of actually hiding those attributes and methods for the users of GuessStatsMessage class. @PéterLeéh \$\endgroup\$
    – revliscano
    Commented Aug 8, 2020 at 15:19
  • \$\begingroup\$ So do you want to know if your Python translation is a correct translation or if the translation is pythonic? \$\endgroup\$ Commented Aug 9, 2020 at 16:35

1 Answer 1

5
\$\begingroup\$

IMO, the python implementation reads like Java, and isn't Pythonic. How can we improve it?

This isn't a class

It looks like one, and it has a class definition, but it's really two functions, one of which is __init__. The other "private" methods are just if statements.

Let's refactor this as a single function:

def make_message(candidate, count):
    if not count:
        number, verb, plural = 'no', 'are', 's'
    elif count == 1:
        number, verb, plural = count, 'is', ''
    else:
        number, verb, plural = count, 'are', 's'

    message = f'There {verb} {number} {candidate}{plural}'

    return message

Now you can simply do:

print(make_message('Foo', 2))
There are 2 Foos

Or

msg = make_message('Bar', 3)
print(msg)
There are 3 Bars

Now, the number, verb and plural names aren't able to be accessed by anything outside the function, not that we needed the privacy model anyways. There are also fewer methods to read and maintain, your eye isn't jumping between code blocks.

The naming is short and concise, and the code is pretty easy to reason about.

We've also used the if not count idiom, where a 0 behaves like False and nonzero values act like True.

Lastly, the tuple unpacking used for number, verb, plural is a very common technique for defining multiple variables inline.

Docstrings and Type Hints

We could further enhance the readability of this function by adding a docstring and type annotations for our variables:

def make_message(candidate: str, count: int) -> str:
    """Returns a message for a number of objects with the name candidate"""
    ...
\$\endgroup\$
2
  • \$\begingroup\$ very common technique for defining multiple static variables - OK, but those aren't statics. \$\endgroup\$
    – Reinderien
    Commented Jul 19, 2023 at 14:47
  • \$\begingroup\$ @Reinderien yeah, not the best choice of words, I'll make an edit \$\endgroup\$
    – C.Nivs
    Commented Jul 19, 2023 at 14:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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