-
Notifications
You must be signed in to change notification settings - Fork 24
Open
Description
First of all, thanks for pulling in my changes indirectly in the recent pull request.
I am a little worried about seconds_between_years(). It only does the "> 2400" check etc. on 'left_year', and not on the 'right_year'. Variable 'increment' is assigned before the left year is adjusted in that way. It may all be OK, but then I would add a comment about why it is so.
Variable 'increment' is not really necessary in that routine. I would split the 2 conditions below into 3 conditions:
- if left_year == right_year, then there is apparently nothing to do.
- if left_year > right_year
- if left_year < right_year (actually the final 'else')
I would then clean up the trailing whitespace all over the file. It shows
in read when checking in with Git.
I am getting a warning about static function check_tm() not being used in release builds. Maybe you could surround its declaration with "#ifndef NDEBUG".
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels