def increment_clicks!(hash_string)
if hash = AdvertisementHash.find_by_hash_string(hash_string)
self.update_attribute("clicks", self.clicks + 1)
self.advertisement_hashes.delete(hash)
end
end
Refactorings
No refactoring yet !
Martin Plöger
September 30, 2009, September 30, 2009 09:56, permalink
No rating.
Login to rate!
To simply increment an attribute you could use #increment or #increment! (uses the setter).
The submitted code was not so ugly to me, but you could shorten it a bit by using the if-modifier.
You should access the AdvertisementHashes through the scope of the advertisement_hashes-association
def increment_clicks!(hash_string) self.increment :clicks if (hash = self.advertisement_hashes.find_by_hash_string(hash_string)) && hash.destroy # If you want to destroy the object # self.increment :clicks if self.advertisement_hashes.delete self.advertisement_hashes.find_by_hash_string(hash_string) # Otherwise end
Ben Marini
October 1, 2009, October 01, 2009 17:21, permalink
No rating.
Login to rate!
def destroy_hash_and_increment_clicks(hash_string)
if hash = advertisement_hashes.find_by_hash_string(hash_string)
increment!(:clicks) # Need the ! if you want to save to the db immediately
hash.destroy
end
end
Again a small piece of code that could be probably written in a nicer way