search

Intrepidd / working_hours

⏰ A modern ruby gem allowing to do time calculation with business / working hours.
https://github.com/Intrepidd/working_hours
MIT License
533 stars 30 forks source link

refactored long method #17

Closed dfucci closed 3 years ago

dfucci commented 9 years ago

Hello, I have noticed that one of the method to compute the time (Computation#add_seconds) was 15 lines long. I have refactored it by extracting 2 private methods

jarthod commented 9 years ago

Thanks for your contribution but I have several remarks:

  1. You commited scope_settings.xml I don't want to know why ^^
  2. You broke indentation, we're using two spaces
  3. Your methods are not correctly scoped, "rolling to next period" is what advance_to_working_time does, I think you should leave this line in the loop and only extract the # look at working ranges part. These extracted methods could be named something like: work_during_day, use_daily_periods or something else describing what it does.
Intrepidd commented 9 years ago

@dfucci are you willing to take @jarthod 's comments into consideration and continue with this PR or do you want me to take over ?

dfucci commented 9 years ago

@Intrepidd please go ahead.