#### Topic: Calculating a Due Date

I am having some difficulty calculating a due date from a specified turnaround time.  The turnaround time is in business days and I've constructed a loop to try to calulate the final real days count.  The code is part of the Create method in my JobsController:

def create
begin
@job = Job.new(params[:job])
@job.company_id = @job.contact.company_id # Back-assign the company association
# New problem code
@turn = @job.turnaround # Int value for business days of turnaround
@daycount = 0 # Int value for number of real days
@count = 0 # An integer counter
while @count < @turnaround # Will loop until count = turnaround
if cwday(@accepted+(@daycount)) ==(6) # Commercial work date of the accepted date plus the current daycount falls on sat
@daycount += 1 # Increment the daycount
elsif cwday(@accepted+(@daycount)) ==(7) # Commercial work date of the accepted date plus the current daycount falls on sun
@daycount += 1
else # Otherwise...
@daycount += 1 # Increment the daycount
@count += 1 # Increment the counter
end
end
@job.duedate = @job.created_at +(@daycount) # Add the final tally of daycount to the creation date
# End of problem code
if @job.save
flash[:notice] = 'Job was successfully created.'
redirect_to :action => 'list'
else
@companies = Company.find_all
@contacts = Contact.find_all
@employees = Employee.find_all
render :action => 'new'
end
# NEW STUFF, the Rescue
rescue
flash[:notice] = 'Job could not be saved.'
redirect_to :action => 'new'
end
end

I'm not sure where things are going horribly wrong, but the job just doesn't save.  Leading suspects are my used of the cwdate() feature or the day additions.  Any help would be greatly appreciated.

Thanks!
-Jacob

#### Re: Calculating a Due Date

One word: Refactoring

I'm sorry this doesn't apply directly to your question or problem, but I think it will really help you. The above code could use some serious refactoring. The method is long and overly complicated. There are a slew of instance variables which should be either temporary variables or removed completely.

Refactoring the code may reveal the error. It is the art of improving code design and quality without changing the external behavior of it. I'd suggest getting this book on refactoring, it is still the best way to learn it:
http://www.amazon.com/gp/product/020148 … 70-6713438

As far as your problem, what do you mean by "the job just doesn't save"? Do you get an error message? Or does everything seem successful except nothing is in the database?

Railscasts - Free Ruby on Rails Screencasts

#### Re: Calculating a Due Date

Actually, this seems like a good spot to put a little refactoring exercise. I hope you don't mind, Muffin Man, if I clean up the code a little.

First off, some instance variables (they begin with an @ sign) are used where there should be temporary variables. I'll replace them with temporary variables and remove some unnecessary parenthesis and comments. I'll also remove the rescue statement as that seemed to be used for testing purposes.

def create
@job = Job.new(params[:job])
@job.company_id = @job.contact.company_id # Back-assign the company association
turn = @job.turnaround # in business days
daycount = 0 # in real days
count = 0
while count < turn
if cwday(@accepted+daycount) == 6 # Saturday
daycount += 1
elsif cwday(@accepted+daycount) == 7 # Sunday
daycount += 1
else
daycount += 1
count += 1
end
end
@job.duedate = @job.created_at + daycount

if @job.save
flash[:notice] = 'Job was successfully created.'
redirect_to :action => 'list'
else
@companies = Company.find_all
@contacts = Contact.find_all
@employees = Employee.find_all
render :action => 'new'
end
end

Take a look at the result of that calculation, it is being placed into the @job.duedate attribute. Why save this value at all? It can be calculated already, so let's move it to a "duedate" method in the Job model.

The main thing keeping us from moving this calculation out of the controller is the reference of the @accepted instance variable. I'm just going to use created_at instead because I think it's what you need. here's the calculation moved into a "duedate" method for the Job model.

class Job < ActiveRecord::Base
def duedate
daycount = 0 # in real days
count = 0
turn = turnaround
while count < turn # in business days
if cwday(created_at + daycount) == 6 # Saturday
daycount += 1
elsif cwday(created_at + daycount) == 7 # Sunday
daycount += 1
else
daycount += 1
count += 1
end
end
created_at + daycount
end
end

The "turn" temporary variable is unnecessary, so we can remove it and just use the "turnaround" method call.

class Job < ActiveRecord::Base
def duedate
daycount = 0 # in real days
count = 0
while count < turnaround # in business days
if cwday(created_at + daycount) == 6 # Saturday
daycount += 1
elsif cwday(created_at + daycount) == 7 # Sunday
daycount += 1
else
daycount += 1
count += 1
end
end
created_at + daycount
end
end

Notice we are incrementing the "daycount" variable on each condition of that logic? The only thing that changes is if the "count" variable is incremented or not. Simplify the logic like this:
class Job < ActiveRecord::Base
def duedate
daycount = 0 # in real days
count = 0
while count < turnaround # in business days
daycount += 1
count += 1 if cwday(created_at + daycount) <= 5
end
created_at + daycount
end
end

Now that I can understand the code I see a couple problems. I'm assuming "created_at" returns a Time object, so adding an integer to a Time results in adding seconds to the time, not days. Let's fix that by storing the seconds instead (using the Integer "day" method).
class Job < ActiveRecord::Base
def duedate
seconds = 0
count = 0
while count < turnaround # in business days
seconds += 1.day
count += 1 if cwday(created_at + seconds) <= 5
end
created_at + seconds
end
end

The other problem is that "cwday" is a method of the Date class and should be called on an instance of that class, the method doesn't accept any arguments. We can use the Time "to_date" method to convert a time to a date
class Job < ActiveRecord::Base
def duedate
seconds = 0 # in real days
count = 0
while count < turnaround # in business days
seconds += 1.day
count += 1 if (created_at + seconds).to_date.cwday <= 5
end
created_at + seconds
end
end

There is another refactoring we can do. That "created_at + seconds" calculation is duplicated. We can remove that by setting seconds to the "created_at" time to begin with. Let's rename the seconds variable to "time" while we do that.

class Job < ActiveRecord::Base
def duedate
time = created_at
count = 0
while count < turnaround # in business days
time += 1.day
count += 1 if time.to_date.cwday <= 5
end
time
end
end

That "time.to_date.cwday <= 5" area isn't very self descriptive. We can refactor this part out into a "business_day?" Time method. That way, to see if the time falls on a business day, we just call it's "business_day?" method. I can see that method being useful elsewhere as well.

class Job < ActiveRecord::Base
def duedate
time = created_at
count = 0
while count < turnaround # in business days
time += 1.day
end
time
end
end

class Time
self.to_date.cwday <= 5
end
end

That looks really good. We can even refactor this a bit further by making a "business_days_since" method for Integer.

class Job < ActiveRecord::Base
def duedate
end
end

class Integer
time += 1.day
end
time
end
end

class Time
self.to_date.cwday <= 5
end
end

That's it for the refactoring exercise. Pretty cool eh?

Have you noticed there are no comments which pertain to that code? Comments are often a sign that the code itself isn't clear. Next time, instead of adding a comment, try to make the code itself a little clearer through refactoring.

I see a couple of other refactorings you can do in the controller "create" method. I'll leave those as an exercise for the reader.

Last edited by ryanb (2006-07-11 02:30:38)

Railscasts - Free Ruby on Rails Screencasts

#### Re: Calculating a Due Date

I think that's a really good exercise in refactoring but you did lose me slightly with the last change and the introduction of business_days_since.

Of course we're not allowing for public holidays in this algorithm.  You'd then need to work through all the public holidays you observe, adding one to your end date for each one that falls between the start date and the end date.  However, I'm not aware of a Rails function that correctly returns all the public holidays in England.

Allen

#### Re: Calculating a Due Date

allen wrote:

I think that's a really good exercise in refactoring but you did lose me slightly with the last change and the introduction of business_days_since.

Yeah, that was kind of a final "throw in" if you really want to go all out. What I was trying to achieve is the readability you get from some of the rails date helper calls (i.e. 5.days.since(time)).

allen wrote:

Actually, I considered that at first but I don't think it will work. For example, let's say the turnaround is 8 business days and the job is created on a Thursday, the job would span across two weekends (four extra days). If the job is created on a Monday it would span only one weekend. Seeing if the creation falls on a weekend is not enough. Adding any further logic would be more complicated than the loop, so I think the loop is the best method.

allen wrote:

Of course we're not allowing for public holidays in this algorithm.  You'd then need to work through all the public holidays you observe, adding one to your end date for each one that falls between the start date and the end date.  However, I'm not aware of a Rails function that correctly returns all the public holidays in England.

Yeah, I probably wouldn't have gone to the extent of moving the logic into the Time and Integer classes if we want holidays because they are so specific to the circumstances.

Railscasts - Free Ruby on Rails Screencasts

#### Re: Calculating a Due Date

ryanb wrote:
allen wrote:

Actually, I considered that at first but I don't think it will work. For example, let's say the turnaround is 8 business days and the job is created on a Thursday, the job would span across two weekends (four extra days). If the job is created on a Monday it would span only one weekend. Seeing if the creation falls on a weekend is not enough. Adding any further logic would be more complicated than the loop, so I think the loop is the best method.

OK, let's try that.

Case 1: I'd break the eight days into a week and three days.  Adding the week takes me to the following Thursday, adding the three days takes me to the Sunday.  Then I apply the check about it being a Saturday or a Sunday and advance it to the Monday.

Case 2: I'd again break the eight days into a week and three days.  Adding the week takes me to the following Monday, adding the three days takes me to the Thursday.  The check about it being a Saturday or a Sunday fails so it stays at the Thursday.

QED!

#### Re: Calculating a Due Date

Ohh, I guess that will work, I was looking at it incorrectly. Thanks allen.

I don't know if the resulting logic would be less complicated than a loop, but it would definitely be more efficient.

Edit:
On second thought, what if the creation date is on a Thursday and the turnaround is 4 days? Adding 4 days will land you on a Monday which will not trigger the exception, but what you want is Wednesday. Am I still misunderstanding your solution?

Last edited by ryanb (2006-07-12 11:21:27)

Railscasts - Free Ruby on Rails Screencasts

#### Re: Calculating a Due Date

Thanks for the replies, guys.  I got the loop working and decided to leave it within create.  That way, I'll be able to easily add the logic later to account for a table of statutory holidays (user generated).

#### Re: Calculating a Due Date

ryanb wrote:

Edit:
On second thought, what if the creation date is on a Thursday and the turnaround is 4 days? Adding 4 days will land you on a Monday which will not trigger the exception, but what you want is Wednesday. Am I still misunderstanding your solution?

No you're not.  Back to the drawing board...