6 ms·

Wait, what was wrong with the #define solution? Just the name of the macro? Didn't he do what you wanted?

sokoloff 6y ago
The 17 was a scaling factor for a graphics transformation (and sometimes was doubled to 34). Let's imagine that we later decide we want to make a change and have it be a little bigger. Do we change it to: #define SEVENTEEN 23 Instead, if it was: #define MENU_SCALING_FACTOR 17 then such a change would be sensible (which was the intention). I admit to having some sliver of culpability here (though the verbal explanation of why we'd prefer to pull the hard-coded numbers out of being sprinkled all over the codebase I assumed would have sufficed). This seemed to border on /r/MaliciousCompliance, but I know he was actually trying his best (and was pre-reddit, of course).
- stagger87 6y ago
  Thanks for the clarification. I guess the tools you had available at that time would affect the severity of this. Nowadays, with the ability to refactor a macro name in a few seconds, I wouldn't necessarily agree that a developer that used a silly name would "never make it to a senior developer". Fuck, I'm "senior" and I definitely use worse variable names on a regular basis.
  - sokoloff 6y ago
    I think I disagree strongly on the first point. If SEVENTEEN is used instead of MENU_SCALING_FACTOR and ARBITRARY_OTHER_THING (two semantically different things both happening to be 17), no refactoring tool (that I’m aware of) is going to be able to tease apart just the former use and amend it to 23, leaving ARBITRARY_OTHER_THING as 17. const int declarationAdoptedYear = SEVENTEEN * HUNDRED + THIRTY_EIGHT * TWO;
    - stagger87 6y ago
      I use QtCreator and it would be able to do this. If I had #define SEVENTEEN 17 and another macro #define OTHER_THING 17 I could simply refactor SEVENTEEN to #define MY_NEW_NAME 17 which would change all occurrences of SEVENTEEN to MY_NEW_NAME, and then I would just change the value of that macro. #define MY_NEW_NAME 23
      - sokoloff 6y ago
        But the devs who would define SEVENTEEN to be 17 are unlikely to define OTHER_THING at all, since there’s a “perfectly good” macro that evaluates to 17 and avoids hard-coded numbers already. (They might even cite DRY: Don't Repeat Yourself as a misapplied best-practices shibboleth.) See the (contrived) Declaration of Independence line above. Your editor would dutifully change the date to be off by six hundred years.
        stagger87 6y ago
        Sure, I understand what you mean. To be honest, that sounds like a different issue, one that would arise after additional circumstances took place. I still stand by my initial comment, that if you had the quick ability to refactor SEVENTEEN at the time this developer committed this, it wouldn't be a very big deal, and this developer could be told that good variable names are important and he wouldn't be stuck as a junior developer forever, lol.
  - deleted 6y ago
    [deleted]
- rspoerri 6y ago
  You could improve your feedback in the same way as he could have improved his change. Create meaningful context. You told him what to do (replace 17), not why to do it (the value can change).
zild3d 6y ago
i think he meant you should #define whatever this 17 value represents e.g. #define width 17
deleted 6y ago
[deleted]

The 17 was a scaling factor for a graphics transformation (and sometimes was doubled to 34). Let's imagine that we later decide we want to make a change and have it be a little bigger. Do we change it to: #define SEVENTEEN 23 Instead, if it was: #define MENU_SCALING_FACTOR 17 then such a change would be sensible (which was the intention). I admit to having some sliver of culpability here (though the verbal explanation of why we'd prefer to pull the hard-coded numbers out of being sprinkled all over the codebase I assumed would have sufficed). This seemed to border on /r/MaliciousCompliance, but I know he was actually trying his best (and was pre-reddit, of course).

stagger87 6y ago
Thanks for the clarification. I guess the tools you had available at that time would affect the severity of this. Nowadays, with the ability to refactor a macro name in a few seconds, I wouldn't necessarily agree that a developer that used a silly name would "never make it to a senior developer". Fuck, I'm "senior" and I definitely use worse variable names on a regular basis.
- sokoloff 6y ago
  I think I disagree strongly on the first point. If SEVENTEEN is used instead of MENU_SCALING_FACTOR and ARBITRARY_OTHER_THING (two semantically different things both happening to be 17), no refactoring tool (that I’m aware of) is going to be able to tease apart just the former use and amend it to 23, leaving ARBITRARY_OTHER_THING as 17. const int declarationAdoptedYear = SEVENTEEN * HUNDRED + THIRTY_EIGHT * TWO;
  - stagger87 6y ago
    I use QtCreator and it would be able to do this. If I had #define SEVENTEEN 17 and another macro #define OTHER_THING 17 I could simply refactor SEVENTEEN to #define MY_NEW_NAME 17 which would change all occurrences of SEVENTEEN to MY_NEW_NAME, and then I would just change the value of that macro. #define MY_NEW_NAME 23
    - sokoloff 6y ago
      But the devs who would define SEVENTEEN to be 17 are unlikely to define OTHER_THING at all, since there’s a “perfectly good” macro that evaluates to 17 and avoids hard-coded numbers already. (They might even cite DRY: Don't Repeat Yourself as a misapplied best-practices shibboleth.) See the (contrived) Declaration of Independence line above. Your editor would dutifully change the date to be off by six hundred years.
      - stagger87 6y ago
        Sure, I understand what you mean. To be honest, that sounds like a different issue, one that would arise after additional circumstances took place. I still stand by my initial comment, that if you had the quick ability to refactor SEVENTEEN at the time this developer committed this, it wouldn't be a very big deal, and this developer could be told that good variable names are important and he wouldn't be stuck as a junior developer forever, lol.
- deleted 6y ago
  [deleted]
rspoerri 6y ago
You could improve your feedback in the same way as he could have improved his change. Create meaningful context. You told him what to do (replace 17), not why to do it (the value can change).

zild3d 6y ago

i think he meant you should #define whatever this 17 value represents e.g. #define width 17

deleted 6y ago

[deleted]