How to resolve code smells in my code?

Ruby on Rails
1

Hi,

I have an application with some code smells. I know there is code smells but I don’t know how to resolve them. I will be very happy if someone can look at my code and check for problems and solutions. In exchange, I will do it too.

You can find the repo of my application here : https://github.com/GCorbel/lescollectionneursassocies

This is an example :

I have a controller which is responsible to show a form with order information (credit card an product information). As you can see, there is a two form objects, one for services and one for paintings. Also, there is an ordre creator which is responsible to save the form, do the payment and send a notification. I made a creator because I think there is too much work for the controller.

There is some elements I did not like. First, I must to create forms in the controller because I need it in views. I need to pass it in argument to the creator. I think it’s a bad spaghetti code.

Also, I have some depedent objects which are responsible to do only one thing, like this one. It’s good to have classes with single responsabilities but I think there is too much classes with no apparent structures.

When I look screencasts of the thoughtbot’s team, they do awsome thinks very easily. I think I am 10 times slower and I produce a code 10 times worse. I hope I will be a ruby ninja one day.

Thanks for your help!

2

Hi, @Guirec_Corbel… I took a quick look at your application: pretty nice stuff, IMO, with lots of functionaltiy nicely broken up into separate controllers and models.

I took a look at your controllers and had the following comments, below. I can’t say that I’m highly experienced in this kind of review, so take them as gentle observations.

I also installed and ran the rails_best_practices gem; you can find the output in this pastie.

orders_controller.rb
--------------------
#create
  nested ifs could be simplified?

messages_controller.rb
----------------------
#message_for_painting
  method too long; separate first three lines into another method?
#message
  method too long?

paintings_controller.rb  
-------------------------
#build_resource
  method too long? complex if?

provider_services_controller.rb
-------------------------------
#update
  form.save without if clause to ensure actually saved? use save! version

users/registrations_controller.rb
---------------------------------
#create & #update
  form.save without if clause to ensure actually saved? use save! version
#form & #profile
  type checking? method too long? too many parameters?
3

Hi and thanks for comments. Tips given by the gem seems to be useful. I will check it and correct my code. Take a look at sandi_meter too.

4

Thanks for that gem; looks very interesting.