Hacker News Reader

Comment by frognumber | Hacker News Reader

In this case, the problem is with a bug creeping in:

            crust: "thyn",

DRY is about avoiding this class of cut-and-paste bugs too. Or with changing a string to a token, as it should have been:

            crust: THIN

The code isn't even correct. It's mixing JavaScript and Python. I'm also not sure why you'd declare functions for each type of pizza; that's data. I'm not sure about the context, but the right way is:

    def make_pizza(crust=THIN, toppings=[], cheese=REGULAR, sauce=TOMATO)

and then in each call, to override.

    make_pepperoni_pizza()

is bad code compared to

    make_pizza(toppings=[PEPPERONI])

All of the code in this post is horrible, and has easy solutions.

I feel dumber for having read this post, and even dumber for having responded.

View on HN

Subtle bug in that toppings has a mutable default argument [1].

[1] https://docs.python-guide.org/writing/gotchas/

There is no bug... yet. Unless you modify the default argument.

Sometimes I just want to monkeypatch the list to be immutable in my app.

I'd consider this a bug and not expected behavior.

  def make_pizza(crust=THIN, toppings=[], cheese=REGULAR, sauce=TOMATO):
    // first call: toppings=["peperoni"]
    // second call: toppings=["peperoni", "sausage"]
    // third call: toppings=["peperoni", "sausage"]

  make_pizza(toppings=["peperoni"])
  make_pizza(toppings=["sausage"])
  make_pizza()

Speaking about bugs, can you spot how he introduced a bug when going DRY? :)

In case it gets fixed: https://i.imgur.com/ZR2XKA7.png

Or deliberate, as everybody knows that pineapple doesn't belong on pizzas :-)

> Copying and pasting a few lines of code takes almost zero thought and no time.

;)

> I'm also not sure why you'd declare functions for each type of pizza; that's data.

Yep, had the same thoughts reading the code. What you suggest even seems a purer implementation of the DRY principle, rather than what is proposed in the article which would result in copy and pasting the make_pepperoni_pizza() function as soon as you decide to sell a third type of pizza.

Of course, the DRY principle used without considering other factors could produce bad results, but all the code in the article is bad for reasons unrelated to the principle it attempts to criticize.

    make_pepperoni_pizza()
     is bad code compared to
     make_pizza(toppings=[PEPPERONI])

How would you make Hawaiian pizza? I forget, does it include Ham? or just pineapple? you're forced to the remember that nuance in your suggested implementation, but not with "make_hawaiian_pizza()"

It's data.

   make_pizza(toppings=HAWAIIAN_TOPPINGS)

   make_pizza(HAWAIIAN)

or similar. Data should generally not be hard-coded, both because it changes and because it wants to be validated. Starting with:

   HAWAIIAN = { TOPPINGS: [ PINEAPPLE  ...

is okay. That can later be loaded from a config file, a database, or otherwise, as the system expands.

There's an interesting architectural decision here: what form of the pizza recipes database strikes the right balance between too hardcoded and too complex. I'd use some kind of configuration file or RDBMS, constants are more readable but still out of place as part of code.

The nice thing about constants is that you can't make typos. A string like "peperoni" isn't caught, but toppings.PEPERONI will fail immediately.

I use Python. It's easy enough to, for example, make an `enum` from entries in a config file or even a database:

https://docs.python.org/3/library/enum.html

Scroll down to the functional API. There are many similar design patterns. The nice thing about these is that you get automatic type checking. If your config file is:

    toppings: ['pepperoni', 'ham', 'pineapple'],
    pizzas: {
       'Hawaiian': ['pinapple']
    }

If you load this in as strings, it will load and later silently fail. If you add validation code, you'll only validate what you remember. If you make an enum or similar custom type, it necessarily will fail-on-load, and probably with a reasonable error.

The major downside -- which is really incidental (due to poor library design) -- is that most JSON/YAML libraries won't reasonably serialize/deserialize non-Python types. So there's a bit of (unnecessary) overhead there.

Reply on news.ycombinator.com