# Turned down for Rails job - seeking code review of test

**URL:** <https://www.ruby-forum.com/t/turned-down-for-rails-job-seeking-code-review-of-test/179823>\
**Category:** Rails\
**Created:** [December 4, 2009, 5:51pm UTC](https://www.ruby-forum.com/t/turned-down-for-rails-job-seeking-code-review-of-test/179823 "2009-12-04T17:51:44Z")\
**Posts on this page:** 3\
**Page:** 1

<div class="post-metadata">

**Author:** ![curious\_about\_rails](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/c/8797f3/32.png) [@curious\_about\_rails](https://www.ruby-forum.com/u/curious_about_rails)\
**Post date:** [December 4, 2009, 5:51pm UTC](https://www.ruby-forum.com/t/turned-down-for-rails-job-seeking-code-review-of-test/179823/1 "2009-12-04T17:51:44Z")

</div>

I was recently turned down for a Rails developer position (I’m not  
really a Rails developer, but it’s something I’ve been playing with in  
my spare time, so I gave it a go).

Part of the technical screen was to take an old, junky Rails app, with  
invalid views, old controllers etc. and smarten it up so be ‘better’  
(the time allocated was less than an hour or so).

Unfortunately I just heard they rejected my submission, saying they were  
‘unhappy with the standard of the code’.

I agree it’s not beautiful, but given the time limit, I thought I had  
done an ok job. Clearly I was wrong. However they didn’t provide any  
further feedback, and so I’m currently at a loss as to what I had done  
wrong.

Their few comments mentioned that I had written unnecessary code and had  
not stuck to Rails principles (which I focussed on above CSS, etc.).

I’m not sure what the app was really for, but I kept the intention much  
the same, and tried to improve the implementation.

Because I’d like to learn from my mistakes - please take a look at the  
attached Rails app. Please pick it apart. Please be as critical as you  
can - anywhere where I’ve used Rails in a non-standard way, anywhere  
I’ve written unnecessary code, anywhere I’ve been anything other than  
the most exemplary Rails developer, please, please, let me know.

Currently I’m ignorant!

N.B. For size, I’ve only included the app folder.

Thanks.

---

<div class="post-metadata">

**Author:** ![curious\_about\_rails](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/c/8797f3/32.png) [@curious\_about\_rails](https://www.ruby-forum.com/u/curious_about_rails)\
**Post date:** [December 5, 2009, 3:21am UTC](https://www.ruby-forum.com/t/turned-down-for-rails-job-seeking-code-review-of-test/179823/2 "2009-12-05T03:21:24Z")

</div>

Im not sure what state the app was in when you received it, but here’s a  
few pointers.

Controller Code –

There’s a lot of code to rescuing from ActiveRecord::RecordNotFound  
errors. There is an easier way to do this on a global scale in  
application\_controller. Check out a method called  
rescue\_action\_in\_public here →

> **[ActionController::Rescue](https://api.rubyonrails.org/classes/ActionController/Rescue.html#M000415)**
>
> This module is responsible for providing rescue\_from to controllers, wrapping actions to handle configured errors, and configuring when detailed exceptions must be shown.

In the index action Page.all was used. This is almost NEVER what you  
want. ActiveRecord is amazing for finding and manipulating database  
records, but really really bad with large amounts of data. I  
accidentally used it once on an xml feed and rendered almost 400K  
records… needless to say, that was a bad day.

Model Code –

The models are pretty thin, so not much to say. The email regex is the  
only that stuck out. It’s not nearly enough for production. Rails has  
a regex for email validation built in (i think). Check that out.

Views and Helpers –

I noticed both helper files were empty. When I first started with rails  
I was super confused about how to use the helpers. The views for this  
app seem pretty simple, but here’s a good rule of thumb. When you write  
an if statement in your view, ask yourself, “could I write this in a  
helper method?”. Or when you write a link that will change based on the  
user’s state (login/logout, user role etc…), put it in a helper method.  
They are life savers.

I think that in a perfect world, ANY view logic would be in a helper  
method, but that’s a war that will wage for a long time.

The only other thing in the views that I noticed was the flash partial.  
Rails’ flash idiom is global and really should be in the layouts folder,  
or better yet, in application\_helper. 😉

In conclusion, what I see is not that bad, but there’s definitely a few  
things that show some unfamiliarity with Rails. Hope that helps.

Jon

---

<div class="post-metadata">

**Author:** ![curious\_about\_rails](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/c/8797f3/32.png) [@curious\_about\_rails](https://www.ruby-forum.com/u/curious_about_rails)\
**Post date:** [December 5, 2009, 10:11am UTC](https://www.ruby-forum.com/t/turned-down-for-rails-job-seeking-code-review-of-test/179823/3 "2009-12-05T10:11:29Z")

</div>

> Views and Helpers –
> 
> I noticed both helper files were empty. When I first started with rails  
> I was super confused about how to use the helpers. The views for this  
> app seem pretty simple, but here’s a good rule of thumb. When you write  
> an if statement in your view, ask yourself, “could I write this in a  
> helper method?”. Or when you write a link that will change based on the  
> user’s state (login/logout, user role etc…), put it in a helper method.  
> They are life savers.
> 
> I think that in a perfect world, ANY view logic would be in a helper  
> method, but that’s a war that will wage for a long time.
> 
> The only other thing in the views that I noticed was the flash partial.  
> Rails’ flash idiom is global and really should be in the layouts folder,  
> or better yet, in application\_helper. 😉
> 
> In conclusion, what I see is not that bad, but there’s definitely a few  
> things that show some unfamiliarity with Rails. Hope that helps.
> 
> Jon

It’s not the only thing I agree with but Jon made a good point regarding  
helper files. It’s too often that people do not take advantage of them.

Views should always have as little work to do as possible in an ideal  
world. An example of using helper files is on my test site  
(unfortunately on my local server). However, in it contains several  
methods to assist the forms on the site. I constructed a CMS for the  
site which has forms for manipulating the records in each table of  
course. Placing actions in the form helper made it simple to integrate  
code for my TinyMCE Editor, as well as giving my forms global access to  
things like this editor, collection selects, document up-loaders and  
image up-loaders.

See below for my example helper method for my TinyMCE Editor:

```
def tinymce_text_area(attribute)
  @template.text_area(@object_name, attribute, :cols => nil, :rows 

```

=\> nil, :class =\> “mceEditor”)

Helpers are especially useful when you find yourself generating a lot of  
applications that follow the same framework and you can simply emulate  
the data from the helpers.  
end