def check_appt_conflicts()
appointments.each do |appointment|
if appointment.frequency == 'weekly'
if date_conflict(self.date, appointment.date, 7) == true
return true
end
elsif self.frequency == 'weekly'
if date_conflict(self.date, appointment.date, 7) == true
return true
end
else appointment.frequency == 'bi_monthly'
if date_conflict(self.date, appointment.date, 14) == true
return true
end
end
end
return false
end
#what i would like to do is if this returns true, break out of the iteration above and return true as the check_appt_conflicts() (first function) #value
def date_conflict(this_date, later_date, mod_value)
if this_date.strftime("%j").to_i % mod_value == later_date.strftime("%j").to_i % mod_value
return true
end
end
Refactorings
No refactoring yet !
bob
April 23, 2009, April 23, 2009 00:25, permalink
But what if date_conflict is called from some other function, would it be able to return from that? So basically you want it to have the ability to "return from" the function that called it, no matter what it is? I don't think there is something like that.
I guess the closest thing would be exception handling. Raise an exception and then handle it in the calling function. I am not sure if that is much simpler than just checking the return value of date_conflict though.
Tj Holowaychuk
April 23, 2009, April 23, 2009 01:35, permalink
Learn ruby first.. :( railers make me sad!! lol. you can ditch all those return keywords for starters, and the == true's .. and well the if statements all together since its an expression anyways
Marc-Andre
April 23, 2009, April 23, 2009 15:21, permalink
Can't agree more with Tj. Read a good book on Ruby (say the new edition of Pragmatic Programmers') and learn Ruby.
I don't understand what's up with the self.frequency test, so apart from that, here's a refactor of your code. I didn't change the date_conflict test because I don't want to understand what it's doing, but there's a better way. Check the api to get days, months, etc... directly, not going through strings.
Bob is right, the way to break out of many functions is to raise an exception. Clearly you don't need it here, you just need more Ruby-fu :-)
FREQUENCY = { 'weekly' => 7, 'bi_monthly' => 14}.freeze
def check_appt_conflicts
appointments.any? do |appointment|
date_conflict(date, appointment.date, FREQUENCY[appointment.frequency])
end
end
def date_conflict(this_date, later_date, mod_value)
this_date.strftime("%j").to_i % mod_value == later_date.strftime("%j").to_i % mod_value
end
Adam
April 23, 2009, April 23, 2009 17:21, permalink
Expanding on MA's work:
class Appointment
FREQUENCY = { 'weekly' => 7, 'bi_monthly' => 14 }.freeze
def conflicts?
appointments.any? { |appointment| conflict?(appointment) }
end
def conflict?(appointment)
date.yday % FREQUENCY[frequency] == appointment.date.yday % FREQUENCY[frequency]
end
end
Carlton
April 23, 2009, April 23, 2009 18:53, permalink
The night of the fight, you may feel a slight sting. That's pride f*cking with you. F*ck pride. Pride only hurts, it never helps.
cb8d865fbe44164ad01142de2103e8ba
The night of the fight, you may feel a slight sting. That's pride f*cking with you. F*ck pride. Pride only hurts, it never helps. cb8d865fbe44164ad01142de2103e8ba
ttdavett.myopenid.com
April 24, 2009, April 24, 2009 17:01, permalink
Thanks for the help everyone. There is a problem though, I do need the self.frequency calls b/c I am comparing two model objects. If either of the objects frequencies are 'weekly', then I know I have to call the date_conflict function with 7. If neither frequencies are 'weekly' and one of them is 'bi_monthly', then I have to call date_conflict with 14. I would also like to have check_appt_return return true if both date_conflict and time_conflict are true. The only way I have gotten this to work is to use: return true if time_conflict(self.start_int, self.end_int, event.start_int, event.end_int), but that seems verbose...This is what I have so far.
def check_appt_conflicts()
events_blocked.any? do |event|
if event.frequency == 'weekly' || self.frequency == 'weekly'
if date_conflict(self.date, event.date, 7)
return true if time_conflict(self.start_int, self.end_int, event.start_int, event.end_int)
end
elsif event.frequency == 'bi_monthly' || self.frequency == 'bi_monthly'
if date_conflict(self.date, event.date, 14)
return true if time_conflict(self.start_int, self.end_int, event.start_int, event.end_int)
end
else #this case should not happen but just in case
if date_conflict(self.date, event.date, 28)
return true if time_conflict(self.start_int, self.end_int, event.start_int, event.end_int)
end
end
end
return false
end
def date_conflict(this_date, later_date, mod_value)
this_date.strftime("%j").to_i % mod_value == later_date.strftime("%j").to_i % mod_value
end
#check overlap of time (using integer representation b/c ruby date class is too slow)
def time_conflict(start_int, end_int, later_start, later_end)
if start_int == later_start && end_int == later_end
return true
elsif time_conflict_range(start_int+1, end_int-1, later_start, later_end)
return true
elsif time_conflict_range(later_start+1, later_end-1, start_int, end_int)
return true
end
end
Tj Holowaychuk
April 24, 2009, April 24, 2009 22:56, permalink
I take it you dont realize that these are the same...
return true if time_conflict(self.start_int, self.end_int, event.start_int, event.end_int) time_conflict(start_int, end_int, event.start_int, event.end_int) # Learn ruby ;)
ttdavett.myopenid.com
April 27, 2009, April 27, 2009 19:29, permalink
Yea, I tried removing "return true if ..." before and just tried it again and it doesn't end up bubbling up the true so that it is returned by date_conflict. Are you sure there is not a difference between having a function evaluate as true and having it return true to a higher function?
ttdavett.myopenid.com
April 29, 2009, April 29, 2009 06:16, permalink
I've taken your advice to learn ruby and I must say that it has really improved my productivity. I mean before I was going out of my way to not learn the language b/c it was new and I figured I could just get by, but let me tell you, thats just a metaphor for swimming against the tide. I can't say enough about the positive effects of learning ruby. I know I sound like an infomercial for "sham wow" right now, but wow, leaning ruby is not a sham! I don't know why everyone doesn't just do this.
Tj Holowaychuk
May 5, 2009, May 05, 2009 04:29, permalink
hahaha :) good to hear. I was in the same boat originally, I loved ruby EVEN when I was writing terrible ruby! haha, but now that I have read several books / libraries it opens your mind to programming in general. Same with haskell, learning all kinds of new techniques in vastly different languages is quite interesting. Just be sure to brush up once and a while, I dont and then I forget about simple things like the #grep method
Is there a way to break/escape from a function within a function and return true as the value of the first function? I am using an if statement to see if the second function is true or not and if it is, then return true, but I was thinking there must be a way to break out of the whole function.