Remix.run Logo
filoeleven 4 days ago

The bigger point I take from this is that the purpose of comments and good names are all attempts to help the developer grasp "the context of this code". The article uses "context switch" repeatedly, and in fact never uses the word "context" any other way. Since the author acknowledged they're starting a friendly flame war, I'll go ahead and add that the biggest problem with the example code is that it's object-oriented and mutable, which forces a sprawling context on the developers working with it.

When I read the replace() method, I was immediately confused because it has no arguments. stringToReplace and alreadyReplaced are properties of the class, so you must look elsewhere (meaning, outside of the function) for their definitions. You also don't know what other bits of the class are doing with those properties when you're not looking. Both of these facts inflate the context you have to carry around in your head.

Why is this a class? In the replace() method, there is a call to translate(symbolName). Why isn't there also a SymbolNameTranslator class with a translate() method? Who decided one was simple enough to use a function while the other warrants a class?

SymbolReplacer surely could also be done with a function. I understand that this is illustration code, so the usage and purpose is not clear (and the original Bob Martin article does not help). Is there a reason we want these half-replaced strings made available every time we call SymbolReplacer.replace()? If there is, we can get the same data by using a reduce function and returning all of the iterations in a list.

A plain, immutable function necessarily contains within it the entire scope of its behavior. It accepts and returns plain data that has no baggage attached to it about the data's purpose. It does one thing only.

commandersaki 3 days ago | parent [-]

The translation from function to class Martin Bob style makes it become literal spaghetti code.