TPP Topic 3: Software Entropy

See the first post in The Pragmatic Programmer 20th Anniversary Edition series for an introduction.

Challenge 1

Help strengthen your team by surveying your project neighborhood. Choose two or three broken windows and discuss with your colleagues what the problems are and what could be done to fix them.

I’ll discuss one example from work recently - exact details removed of course. Our current main project is the next version (3) of our main API. Since it’s a new version we’re able to make breaking changes, however there is still a fair amount backwards compatibility required. The new API had to translate between the domain model of existing users in the previous API version and the newer model. The previous model had a number of user types, each with different permissions.Here’s a few examples (there were more):

• Read Only
• Standard
• Super

These were represented using a user_type field with values such as:

READONLY = "readonly"
STANDARD = "standard"
SUPER = "super"

The new Django User model also has a user_type field which maps to this field. For reasons I won’t get into however, the new user_types have a _user suffix. The implementation of this using choices was along the lines of:

from django.db import models

class User(models.Model):

    # user_type values used in API V2
    READONLY = "readonly"
    STANDARD = "standard"
    SUPER = "super"

    # mapping V2 user_type to Django group name
    USER_TYPES = [
        (READONLY, "readonly_user"),
        (STANDARD, "standard_user"),
        (SUPER, "super_user"),
    ]

    user_type = models.CharField(choices=USER_TYPES)

Ignoring the many missing details from this example; the code works. The previous user_types are mapped to the new ones and the values for the new user_type are constrained as desired using Django choices. It was also implemented in a short period of time - great right? Well, there are some broken windows:

• The string values for the V2 API user_type are class attributes on the new User model. This clutters the interface of the User model and their purpose can only be known by reading the code and accompanying comments.
• The USER_TYPES list used for the choices argument in the user_type field is very un-DRY (wet?).

I brought these up during code review and we fixed the broken windows before the pull request was accepted:

from enum import Enum

from django.db import models


class V2UserTypes(Enum):
    READONLY = "readonly"
    STANDARD = "standard"
    SUPER = "super"


class User(models.Model):

    user_type = models.CharField(
        choices=[
            (t, t.value + "_user") for t in V2UserTypes
        ]
    )

The interface clutter for the User model was removed by moving the version 2 string constants out of the class and removing the USER_TYPES list (it’s only purpose was to support the choices parameter of the user_type field). The string constants are now encapsulated within an Enum. This has two main benefits:

1. It is very clear what the values of V2UserTypes represent
2. The Enum ensures unique, constant values. Helping to protect them against the double edged sword of a high level of dynamicism offered by Python.

A list comprehension was introduced to add the _user suffix the the version 2 user types to keep the code DRY.

Since this is a new project, fixing this broken window before it got into the codebase was important to ensure that the codebase remained high quality.

Challenge 2

Can you tell when a window first gets broken? What is your reaction? If it was the result of someone else’s decision, or a management edict, what can you do about it?

Challenge 1 was an example of being able to spot when a window first gets broken. My reaction in this case was to bring it up during code review; explaining why I thought it was a broken window, the issues it could cause in the future and providing some potential solutions. This was a result of someone else’s decision, however I am fortunate to be on a team with which encourages code review and feedback/learning from others in general.

A management edict, on the other hand, would be more difficult to address. Of course it depends on exactly what the edict was however my general approach would be:

• Extract the broken window from the edict
• Gather evidence from previous projects, open source, research papers etc where a similar broken window was introduced
• Explain why the broken window is an issue and (using the evidence) what the consequences could be
• Provide a solution, most likely something along the lines of “I would like x days to implement x solution. This additional time will save y amount of issues in the future <reference evidence here>”