After results been send we were asked whats wrong with code we pointed out:
v = params[:vintage].to_i if v < 100
if v < (Date.today.year - 2000)
v = 2000 + v
else
v = 1900 + v
end
end
One developer failed to explain, because he wasn't the guy who did this review and he didn't know the context.
The context was the it was inside one of the actions instead being separate method in controller or even in helper.
The tendency in Rails (accordind to its creators, and common sense) says that action code should be as simple as possible - in perfect world just calls to some methods and rendering page
In this case it should be something like
v = get_vine_vintage(params[:vintage])or something, with method defined somewhere else. We always should keep controller actions code clean and readable.
No comments:
Post a Comment