I've been struggling alot recently with controllers and with code written by my colleagues. During this period I think I've been making quite a few mistakes. One of the rules I generally like to make about things particularly in software is that if its stupid or doesn't work its probably your fault. This is a really important rule which is real easy to apply to anyone but yourself. So its really easy for me to spot others making mistakes by not applying this rule. When I hear people say Selenium doesn't work, or SASS is rubbish I can comfortably be a wise old owl and think, "ah they should just take a little more time and they will realise that these judgements are made hastily.
Another rule I like to apply is that we (human beings) are mostly ignorant about everything. Socrates (who at the time was considered the wisest of the Greeks) used to tell people seeking his wisdom that he knew nothing. This wasn't some glib bit of rhetoric to reinforce his social status. All his years of study had made him fundamentally aware of how vast knowledge is, how much depth any subject can have, and how little he really did know and understand. This ignorance goes far deeper than just a lack of factual knowledge and goes from the global to the intimate. Using this rule is really dangerous, its so easy to apply it to others and to forget to apply it to yourself, doing this results in smug ignorance. In fact the only value of this rule is self application. It is perhaps a more dangerous expression of the buddhist concept of a "begginers mind"
So what does this have to do with controllers
Well I've been struggling with existing code in our application and in particular with trying to keep code out of controllers. I don't want code in controllers for a number of reasons
- They seem to be much harder to spec than models
- It seems real easy for various bits of business logic to end up in them - where it is really hard to test
- Its real easy for important things like error messages to end up in controllers, and again it feels that this is the wrong place for them
- I'm just not comfortable with them, they don't feel like good objects.
Now I've recently been dealing with alot of existing code in controllers that I've not been happy with. In our application we are now treating addresses as a polymorphic nested entity that other entities can have. In addition we have introduced the concept of NamedAddress, which are special in the fact that they have a name and also in the way they persist. Both our controller and views; and our features and steps have struggled to isolate the addressable bit from the relationship with the particular parent.
During this process I've been learning lots, improving the code and generally working really hard on improving quality. Trouble with working really hard is that its really easy to work really stupidly and I've been doing that too. I've also been assuming that existing code is stupid. Take the following code in the index method of the OrdersController
def index @pending_orders = @customer.pending_orders @completed_orders = @customer.completed_orders end
I broke this code (well actually the view it shows) by the following refactoring of routes
map.resources :customers do |customers| customers.resources :addresses customers.resources :orders end
map.resources :customers, :has_many :addresses, :shallow => true map.resources :customers, :has_many :orders, :shallow => true
This is a really stupid refactoring. Why?
So back to this index method which I'm now having to look at because my features are broken. Well its obviously wrong, because its an Order controller with customer code in it. I've just spent ages making sure that my Address controller doesn't have code that refers to a specific parent, and refactored lots of features and in particular steps that should have been Addressable not Customer/Address or Order/Address. And now on top of that there are all these Order features and in particular steps that have @customer in them. So now I'm peeved and am about to start to work even harder to get this stuff sorted when ... I have to go home.
Time passes ... and I realise that "its my fault" and that "I am stupid"
Remember "if its stupid and it doesn't work its probably your fault". Remember that stupid refactoring! and the fact that your added shallow routes to the customer order relationship at the same time as to the customer address relationship. Understand that applying shallow routing will change/break numerous named routes and url generation methods. Realise how stupid it is to do this in for two relationships at the same time. Realise that doing this refactoring for one relationship should be a seperate issue and commit - and here I am trying to do this for two relationships in the middle of doing something else!! Realise that I shouldn't even be looking at the OrdersController never mind making flawed judgements about the code in it. Understand that in the 'shop' part of this application that orders fundamentally belong to a customer and cannot exist without them. Realise that my colleagues implicitly get this possibly due to their greater experience with ecommerce. Realise that if the index method in the OrdersController is flawed its only very minor (perhaps just the customer needs to made available to the view). Realise that controllers are more contextual than models; and that the implied one to one relationship between controllers and models is a figment of my misunderstanding. Realise that a simple one line before filter can express a controllers context and allow a controller to fulfill a significantly different role.
There is lots more, but I'm really tired now.
So once again programming provides that wonderful opportunity to learn, and reinforces the need for my to keep my little rules and diligently apply them to myself.