Judge's feedback

[ghost]

Placeholder issue for judges to leave feedback.

[ghost]

General:

• Your model code is among the cleanest I've seen (at least so far).
• Your controllers are simple. No custom actions, and you handle the 404 statuses inline, which while redundant, is easy to see, and not a negative in my opinion.
• Your routes file is clean, standard, and you're not trying to handle 404s, here. Rather you do that in your controller, which I personally think is a cleaner way of dealing with it overall. But then again, this is the opinion of just one judge.

Single-table inheritance:

• You explicitly avoid using single-table inheritance. I've seen this on a couple of other submissions as well. I'm neither for or against this approach at the moment, but I wonder if you could provide some background on what made you decide to go this direction.

Migrations:

• You don't need this column on the responders table. It just duplicates this column on the emergencies table, and you want to remove any duplication in your data model that can lead to out-of-sync data.

That's bad news in your data model, and it's easily cleaned up.

Otherwise migrations look okay. Since this is a one-off app you could probably consolidate all those migrations into a single one, but I'd say that's not really a big deal here.

Regarding this line, and its close cousin on line 28 in the same controller, are you familiar with this: http://api.rubyonrails.org/classes/ActiveSupport/Rescuable/ClassMethods.html

ActiveRecord callbacks in Emergency model:

• I usually avoid using callbacks, mostly because they can become very complicated. However, your use of them here actually makes sense, and seems well thought out. If the specification for this model started to become more complex (which it won't since this is just a competition), but in a real-world app this might start to become unwieldy.
• This is more general feedback in this particular case, not a negative. Just something that stuck out to me.

Overall This is one of the better initial solutions I've seen so far. I would wait for at least one more judge to provide their feedback.

[maxschram]

Thank you so much for the feedback.

Single-table inheritance:

• I disabled this so I could have a column named 'type' for the responder type. I could have named it something else, but given that the tests were passing in a hash key of 'type' and the fact that I didn't think STI would be needed it seemed cleaner. Seemed like the lesser of two evils. Do you think keeping STI would have been better practice?

Migrations:

• I intended to make that column on the responders table be a reference, but by mistake just created data duplication. Thanks for pointing that out.

I was not familiar with rescue_from. I was looking for something like that, but couldn't find it. Perfect.

Thanks again for your feedback!

[ghost]

Verified score of 114 points on the automated scoring, based on the current master branch.

[ghost]

Your score breakdown (you placed 11th, just 2.5 points away from tying for 10th - so close!):

221.5 points total:

• 114 points for passing tests
• 107.5 points for judge's section