| ▲ | dijksterhuis 4 days ago |
| i personally prefer this kind of version — if i want to do the maths to work out tweaks i can, but i’m not forced to do maths in my head to know/tweak the end value // a total of
// - backup interval = 24
// - approx backup duration = 2
// - “wiggle room” = 2
ageAlertThresholdHours = 28
yes lazy devs are lazy and won’t want to or just won’t update the comments (be pedantic in review :shrug:). it’s all trading one thing off with another at the end of the day.edit — also your version forces me to read horizontal rather than vertical, which takes longer ime. sorry, i’ve basically done an unprompted code review. i feel like a bit of a dick now. |
|
| ▲ | Joker_vD 4 days ago | parent | next [-] |
| const int backupIntervalHours = 24
const int approxBackupDurationHours = 2
const int wiggleRoomHours = 2
const int ageAlertThresholdHours = backupIntervalHours + approxBackupDurationHours + wiggleRoomHours;
static_assert(28 == ageAlertThresholdHours);
It's a shame more languages don't have static asserts... faking it with mismatched dimensions of array literal/duplicate keys in map literals is way too ugly and distracting from the intent. |
| |
| ▲ | pwdisswordfishy 3 days ago | parent [-] | | Mmm... ageAlertThresholdHours = 24 + // backup interval
2 + // approx backup duration
2; // "wiggle room"
No static assert needed, no need to pre-compute the total the first time, and no need to use identifiers like `approxBackupDurationHours`, the cognitive override about the possibility of colliding with other stuff that's in scope, or the superfluous/verbose variable declaration preamble. | | |
| ▲ | feffe 3 days ago | parent [-] | | I'm a believer in restricting the scope of definitions as much as possible, and like programming languages that allows creating local bindings for creating another. For example: local
val backupIntervalHours = 24
val approxBackupDurationHours = 2
val wiggleRoomHours = 2
in
val ageAlertThresholdHours = backupIntervalHours + approxBackupDurationHours + wiggleRoomHours
end
Then it's easier to document what components a constant is composed of using code without introducing unnecessary bindings in the scope of the relevant variable. Sure constants are just data, but the first questions that pops into my head when seeing something in unfamiliar code is "What is the purpose of this?", and the smaller the scope, the faster it can be discarded. | | |
| ▲ | zephen 3 days ago | parent | next [-] | | Mentally discarding a name still takes some amount of effort, even if local. I often write things the way you have done it, for the simple reason that, when writing the code, maybe I feel that I might have more than one use for the constant, and I'm used to thinking algebraically. Except, that I might make them global, at the top of a module. Why? Because they encode assumptions that might be useful to know at a glance. And I probably wouldn't go back and remove the constants once they were named. But I also have no problem with unnamed but commented constants like the ones in the comment you responded to. | |
| ▲ | a day ago | parent | prev [-] | | [deleted] |
|
|
|
|
| ▲ | tetha 4 days ago | parent | prev [-] |
| > sorry, i’ve basically done an unprompted code review. i feel like a bit of a dick now. That's all fine. Just note that this was one of the easiest examples I could find. For example, for reasons out of my control, the individual network configuration on a linux host is positively nuts. The decision whether to deploy routes, static DNS servers and such depends on 3-5 facts about the datacenter and the provider it's on. In such a case, it is more maintainable to separate the facts about the provider, or the thing we are looking at (e.g. "Does this provider allow us to configure routes in their DHCP server?", from the computation/decision making ("Can the system rely on the routes from the DHCP servers?"), and all of that from the actual action based off of the decision ("Deploy routes statically if DHCP provided routes are not correct"). |