4 ms·

Everyone goes through this phase where we laugh at non-optimal solution to a problem, and proudly cut a line or ten, or hundred from someone's solution. But af

Everyone goes through this phase where we laugh at non-optimal solution to a problem, and proudly cut a line or ten, or hundred from someone's solution. But after 20 years in the industry, I find this trivial and expected even in code by people who are quite smart. Sometimes a solution evolves in a way where the initial variant wasn't subject to simplification, while the current version is. Sometimes there's an edge case that's non-obvious and solved by a complicated solution (comments help here). Sometimes the coder had their mind elsewhere so they approached the problem in a seemingly odd manner. But none of this matters in the big picture. What matters is all this code is wrapper in a function that goes like "getFirst42Bits(int)" and it returns that. And so the only thing that matters is if this function has the right input, output for the bigger problem to solve. The implementation is a detail. Of course, I'm excluding vulnerabilities and vast performance problems in hotspots, this matters. But a specific instance of inefficient solution doesn't matter. A pattern of inefficient solutions may be ground for being demoted or laid off. But any instance is irrelevant. WE ALL DO THIS from time to time, but we don't realize it. Heck, the primary reason why I hate most "web frameworks" is that they do in 2000 lines of boilerplate what I can do with my own APIs in 100.

mdip 5y ago
> Sometimes a solution evolves in a way Yeah, forgot all about that detail -- reminded me of one the other day. I had finished up a really painful feature add, involving altering code in like 7 different classes due to a data model change only to find out that four of the seven classes were unused, and the remaining provided small bits of unrelated data needed for previous versions. This feature had undergone six changes of the back-end that was responsible for providing the service it needed. Each time a new pipeline was added, the old pipeline was removed from the container but the code was left behind and I'd just added a 7th.
codesections 5y ago
> But none of this matters in the big picture. What matters is all this code is wrapper in a function that goes like "getFirst42Bits(int)" I disagree. Imo, `getFirst42Bits(snowflake)` is worse than `snowflake >> 22` — not only is it longer, but it also hides away the implementation in a way that could obscure bugs/require `jumpToDefinition` debugging. Abstraction is often useful, but this seems like a textbook case of over-abstraction.
- BulgarianIdiot 5y ago
  You realize I used this placeholder name just as an example, since I have no idea what function this code was a part of. That said, a one-liner function that's semantically specific to an app (where 42 bits is important for some reason) is absolutely harmless. The compiler will inline it, while for you it represents an opportunity to refactor this in one place. Because the assumption is you call a function at least twice, or it wouldn't be a function in the first place (some exceptions but mostly it's the rule).
  - gnubison 5y ago
    The compiler may inline it, but my brain can’t.
- rudian 5y ago
  I disagree with your disagreement. I don't know what 22 is and how that is related to the initial requirements: get the first 42 bits. (Non-)ironically the requirement matches the suggested function name. What I see is just a rare operator and a magic number.
  - bruce343434 5y ago
    64-42 = 22 This is quite obvious if you have spent some time using bitshift operators, just like inheritance and traits and all that are quite obvious if you have spent some time with OOP. If you have never programmed in C or C++ or the likes then you probably don't have a strong intuition about 8,16,32,64 and the bit operators. You probably don't know how to express a couple of common numbers in hexadecimal either. Different programming. Not better or worse. For the record, I had no fucking idea what that code snippet in the article was supposed to be doing had I not know the objective or function name beforehand.
    - jolmg 5y ago
      > 64-42 = 22 I think they knew that. Their point was probably that it's better for code to directly express the requirements rather than meet the requirements through the expression of other data that isn't immediately obvious to be related at a glance. In this case, it isn't obvious that the 22 is related to the 42 in the requirements until you notice that the int size is 64. `snowflake >> ((sizeof snowflake) * 8 - 42)` is better. Even better would be putting the 42 in a descriptive constant.

mdip 5y ago

> Sometimes a solution evolves in a way Yeah, forgot all about that detail -- reminded me of one the other day. I had finished up a really painful feature add, involving altering code in like 7 different classes due to a data model change only to find out that four of the seven classes were unused, and the remaining provided small bits of unrelated data needed for previous versions. This feature had undergone six changes of the back-end that was responsible for providing the service it needed. Each time a new pipeline was added, the old pipeline was removed from the container but the code was left behind and I'd just added a 7th.

codesections 5y ago

> But none of this matters in the big picture. What matters is all this code is wrapper in a function that goes like "getFirst42Bits(int)" I disagree. Imo, `getFirst42Bits(snowflake)` is worse than `snowflake >> 22` — not only is it longer, but it also hides away the implementation in a way that could obscure bugs/require `jumpToDefinition` debugging. Abstraction is often useful, but this seems like a textbook case of over-abstraction.

BulgarianIdiot 5y ago
You realize I used this placeholder name just as an example, since I have no idea what function this code was a part of. That said, a one-liner function that's semantically specific to an app (where 42 bits is important for some reason) is absolutely harmless. The compiler will inline it, while for you it represents an opportunity to refactor this in one place. Because the assumption is you call a function at least twice, or it wouldn't be a function in the first place (some exceptions but mostly it's the rule).
- gnubison 5y ago
  The compiler may inline it, but my brain can’t.
rudian 5y ago
I disagree with your disagreement. I don't know what 22 is and how that is related to the initial requirements: get the first 42 bits. (Non-)ironically the requirement matches the suggested function name. What I see is just a rare operator and a magic number.
- bruce343434 5y ago
  64-42 = 22 This is quite obvious if you have spent some time using bitshift operators, just like inheritance and traits and all that are quite obvious if you have spent some time with OOP. If you have never programmed in C or C++ or the likes then you probably don't have a strong intuition about 8,16,32,64 and the bit operators. You probably don't know how to express a couple of common numbers in hexadecimal either. Different programming. Not better or worse. For the record, I had no fucking idea what that code snippet in the article was supposed to be doing had I not know the objective or function name beforehand.
  - jolmg 5y ago
    > 64-42 = 22 I think they knew that. Their point was probably that it's better for code to directly express the requirements rather than meet the requirements through the expression of other data that isn't immediately obvious to be related at a glance. In this case, it isn't obvious that the 22 is related to the 42 in the requirements until you notice that the int size is 64. `snowflake >> ((sizeof snowflake) * 8 - 42)` is better. Even better would be putting the 42 in a descriptive constant.