5 ms·
I think the problem is not that "not all tokens are valid" Rather it is that we often repeat the same token sequences, and we should seek to abstract those pred
by ColonelPhantom 4y ago
I think the problem is not that "not all tokens are valid" Rather it is that we often repeat the same token sequences, and we should seek to abstract those predictable sequences into more unique tokens, e.g. by turning it into a library function.
Of course, the downside is that now you have way more tokens you need to know to understand some code, similarly to how Haskell code tends to have tons of mega-abstract function combinators or whatever whereas Go code is very simple. Which one is more readable depends on the reader, because a language like Go requires the reader to sift through more details, whereas Haskell is much terser but also requires much more pre-existing knowledge to understand.
I'm wondering if we can use these code-generation models to find "low entropy code" that is a prime target for turning into libraries.
- newaccount74 4y agoI think the Dont-Repeat-Yourself mantra is a bit overused. For example, assume you have a bunch of functions that all have the following code sequence A() B() C() So someone thinks, hey, lets extract that bit, and create a function that does all three things def ABC(): A() B() C() Now the calls site is much simpler and there is no more repetition: ABC() Except you realise that in some call sites the B() call was missing, so to fix that you add an argument: def ABC(doB): A() if(doB): B() C() The call site now looks a bit less clean, but still no repetition! ABC(True) or ABC(False) Then you realise in some cases the C() call is not necessary, so just add another argument: def ABC(doB, doC): A() if (doB): B() if (doC): C() And your call sites look like this: ABC(True, False) etc. At that point the repetition in the original is definitely preferrable...
- lmm 4y agoA codebase is a living thing. Inlining a function or splitting it into multiple cases should always be an option, and boolean flags are generally a code smell. I don't see this as an argument against DRY; when the facts change, your code structure needs to change too, but that doesn't mean your original structure was wrong.
- hbrn 4y agoI used to share the same example against obsession with DRY. But it's a bit more nuanced. Both options can be valid. DRY is a tangential topic, the main goal should be to keep your code as close to your mental model as possible (also keeping mental models in sync between team members, which is quite hard). ABC being a sequence could be a pattern, or it could be a coincidence. You can't know which one it is just by looking at the code. The knowledge whether it's a sequence or not is in the business domain and in your mental model of that domain. You might think you're disagreeing on DRY with another team member, but in reality you two have different mental models, and one of you is using DRY to justify a his.
- another-dave 4y agoIf you put names to those functions it instantly feels a lot less like 'the original is definitely preferrable', for example something like: def handleNewEmail(isInboxDisplayed, areNotificationsEnabled): AddToInbox(); if (isInboxDisplayed): RefreshInboxView(); if (areNotificationsEnabled): SendSystemNotification(); Not defending needless refactoring in all cases, it's definitely a judgement call. (Edit: formatting)
- newaccount74 4y agoIn your example the if statements would be needed in any case, since they depend on external state. In that case the refactoring would make sense. I'm talking about the case where the conditionals are only needed because of the refactoring.
- another-dave 4y agoAh I presumed from your example that conditionals were external to the doABC() logic since they were subsequently passed in as params? To be honest, I find it hard to discuss readibility without a real-world example. The algebraic placeholders feel too terse.
- newaccount74 4y agoI'm talking about the case where you have these two functions: def func1(): ... A() B() C() ... def func2(): ... A() B() C() ... Which is then refactored to def func1(): ... ABC() ... def func2(): ... ABC() ... def ABC(): A() B() C() Which is sensible, but then other functions show up: def func3() ... A() C() ... And you refactor to: def func1(): ... ABC(True) ... def func2(): ... ABC(True) ... def func3() ... ABC(False) ... def ABC(doB): A() if (doB): B() C() I'll leave the final example to your imagination, since I've already used so much screen space. I'm currently in the process of undoing a lot of such refactorings in my codebase because it has become unmanageable. I really think repetitive code is much easier to work with than overabstracted code.
- mejutoco 4y agoVery well put. I run into this all the time in the name of "reducing complexity", when it is really hiding it under the rug. My personal approach to combat this is better data structures that model the problem (and are checked by the compiler). Once this is in place I try to "flatten" the calls, so that things are mostly at the top level, or few levels deep, which usually comes up naturally once the data structure is consciously defined. I try to have as much code as possible be "structure in - structure out" (pure functions) and to concentrate stateful code to work on the structure's fields/values. This is surprisingly easy once the data structures match the problem, instead of only growing organically.
- anilakar 4y agoCan't wait for the next code review. "You seem to be using ifs and fors very much. These should be abstracted into a function."
- ColonelPhantom 4y agoYou missed the point; an if or for is a single token. The problem is with predictable token sequences, and if you write the same for loop (or extremely similar ones) in multiple places then yes, it should be turned into a function.