4 ms·

Agreed, comments are generally a way to compensate failure to express ourselves in the code (in this case bad naming).

thaumasiotes 13y ago
Actually, there's a difficulty here that naming can't solve; I don't see a better method than the comment. The goal is to enumerate the powers of 3, 5, and 7, once each. Since power sequences all overlap at x^0 = 1, but we specifically don't want to enumerate 1 three times, we have to give one (or, from an alternative viewpoint, two) of the variables special treatment. Whether you name the variables "three", "five", and "seven", or "next_power_of_three", "next_power_of_five", and "next_power_of_seven", you're doing something strange by starting one of them at 1 and the other two past 1, and that should be commented on. The naming-only solution "powers_of_three_initialized_starting_at_three_to_the_zeroeth", "powers_of_five_initialized_starting_at_five_to_the_first", and "powers_of_seven_initialized_starting_at_seven_to_the_first", is hilariously awful, and still requires a comment to explain why the threes variable is more (or less) special than the other two.
- stephencanon 13y ago
  The goal is not to enumerate the powers of 3, 5, and 7. The goal is to iterate through the groups which hold BACKUP superblock/GDT copies. “three”, “five” and “seven” are not especially good names for iterator state, nor is there an obvious good reason for exposing the inner details of iterator state.
  - chengiz 13y ago
    Yes thank you! People seem to be missing this point. The variables should be simply named something like group_counter_a, group_counter_b, group_counter_c, with an explanation of why their default values are what they are.
- mseebach 13y ago
  I think the problem is the mixing of concerns: The generation of the sequence (which apparently is a function of whether the filesystem is sparse or not) and whatever it is trying to verify.
- userbinator 13y ago
  Or how about just combining all those variables into an array named "iterator_powers" or similar? There could be another one, "iterator_multipliers" containing 3, 5, and 7. I don't know if this is actually a "best practice" or rule, but I've found that if I want to name variables after numbers, usually what I'm trying to do should be using an array.

Actually, there's a difficulty here that naming can't solve; I don't see a better method than the comment. The goal is to enumerate the powers of 3, 5, and 7, once each. Since power sequences all overlap at x^0 = 1, but we specifically don't want to enumerate 1 three times, we have to give one (or, from an alternative viewpoint, two) of the variables special treatment. Whether you name the variables "three", "five", and "seven", or "next_power_of_three", "next_power_of_five", and "next_power_of_seven", you're doing something strange by starting one of them at 1 and the other two past 1, and that should be commented on. The naming-only solution "powers_of_three_initialized_starting_at_three_to_the_zeroeth", "powers_of_five_initialized_starting_at_five_to_the_first", and "powers_of_seven_initialized_starting_at_seven_to_the_first", is hilariously awful, and still requires a comment to explain why the threes variable is more (or less) special than the other two.

stephencanon 13y ago
The goal is not to enumerate the powers of 3, 5, and 7. The goal is to iterate through the groups which hold BACKUP superblock/GDT copies. “three”, “five” and “seven” are not especially good names for iterator state, nor is there an obvious good reason for exposing the inner details of iterator state.
- chengiz 13y ago
  Yes thank you! People seem to be missing this point. The variables should be simply named something like group_counter_a, group_counter_b, group_counter_c, with an explanation of why their default values are what they are.
mseebach 13y ago
I think the problem is the mixing of concerns: The generation of the sequence (which apparently is a function of whether the filesystem is sparse or not) and whatever it is trying to verify.
userbinator 13y ago
Or how about just combining all those variables into an array named "iterator_powers" or similar? There could be another one, "iterator_multipliers" containing 3, 5, and 7. I don't know if this is actually a "best practice" or rule, but I've found that if I want to name variables after numbers, usually what I'm trying to do should be using an array.