Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ForDuration calculation with 0 raises error #59

Closed
mwean opened this issue Apr 9, 2016 · 1 comment
Closed

ForDuration calculation with 0 raises error #59

mwean opened this issue Apr 9, 2016 · 1 comment

Comments

@mwean
Copy link

mwean commented Apr 9, 2016

I just ran into an issue where I tried to calculate 0 days after a given time, and got an error.

schedule = Biz::Schedule.new do |config|
  config.hours = {
    mon: { '07:00' => '18:00' },
    tue: { '07:00' => '18:00' },
    wed: { '07:00' => '18:00' },
    thu: { '07:00' => '18:00' },
    fri: { '07:00' => '18:00' }
  }
end

Biz::Calculation::ForDuration.days(schedule, 0).after(Time.now)
NoMethodError: undefined method `year' for nil:NilClass
from /Users/Matt/.rbenv/versions/2.3.0/lib/ruby/gems/2.3.0/gems/biz-1.5.1/lib/biz/time.rb:80:in `on_date'

It looks like the issue is here:

def after(date)
  schedule.after(date).take(number_of_days).to_a.last
end

Where it's taking 0 from the array. Is this a case you would like to handle? If not, it might be good to add a descriptive error, because it took a little while to hunt down the problem.

craiglittle added a commit that referenced this issue Apr 11, 2016
Zero or a negative number does not make sense in the context of moving
backward or forward through time, so we'll catch those uses before the
calculations are even performed.

Fixes #59.
craiglittle added a commit that referenced this issue Apr 11, 2016
Zero or a negative number does not make sense in the context of moving
backward or forward through time, so we'll catch those uses before the
calculations are even performed.

Fixes #59.
@craiglittle
Copy link
Collaborator

Thanks for the report!

Descriptive error on the way: #60

craiglittle added a commit that referenced this issue Apr 11, 2016
A negative scalar does not make sense in the context of moving backward
or forward through time, so we'll catch those uses before the
calculations are even performed.

Fixes #59.
craiglittle added a commit that referenced this issue Apr 11, 2016
Negative scalars are not something we want to explicitly support at
this time, so we'll raise a helpful error message instead of randomly
blowing up when someone tries to do it.

Fixes #59.
craiglittle added a commit that referenced this issue Apr 12, 2016
The first active period in the chosen direction is found and the time at
the beginning of that period returned.

If the specified origin is in the middle of an active period, that time
is returned.

The same logic is applied looking forward or backward with the only
difference being the direction.

Fixes #59.
craiglittle added a commit that referenced this issue Apr 12, 2016
The first active period in the chosen direction is found and the time at
the beginning of that period returned.

If the specified origin is in the middle of an active period, that time
is returned.

The same logic is applied looking forward or backward with the only
difference being the direction.

Fixes #59.
craiglittle added a commit that referenced this issue Apr 12, 2016
The first active period in the chosen direction is found and the time at
the beginning of that period returned.

If the specified origin is in the middle of an active period, that time
is returned.

The same logic is applied looking forward or backward with the only
difference being the direction.

Fixes #59.
craiglittle added a commit that referenced this issue Apr 12, 2016
The first active period in the chosen direction is found and the time at
the beginning of that period returned.

If the specified origin is in the middle of an active period, that time
is returned.

The same logic is applied looking forward or backward with the only
difference being the direction.

Fixes #59.
craiglittle added a commit that referenced this issue Apr 12, 2016
The first active period in the chosen direction is found and the time at
the beginning of that period returned.

If the specified origin is in the middle of an active period, that time
is returned.

The same logic is applied looking forward or backward with the only
difference being the direction.

Fixes #59.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants