Software with any complexity requires careful design to ensure readability and maintainability of the code. The two extremes to achieve a good design are upfront design done on a whiteboard before writing a single line of code or evolve it by iterating over the code by adding functionality one at a time. Which begs the question, which approach leads to better design or if there is a middle ground.
Like most of the questions of this sort, the answer is 'it depends'. What really matters is the state of the codebase when you call it 'done'. As a rule of thumb you should ensure that your code adheres to simple rules of design (Xp Simplicity Rules) before it's complete.
There are various factors which influence the choice of methodology used to solve the problem. To list a few:
Understanding of the problem space - it includes domain and non-functional requirements such as scale, usability and so on
Familiarity with the solution space - how well you know the toolchain used for solving the problem such as language, libraries, framework, third party apis
If you have intimate understanding of both the problem space and the solution space, you can jump right in and start designing your system by defining various layers of abstractions and how they communicate with each other.
Once the basic skeleton is in place, pick a part of the system to flesh out in greater detail. You can mock/stub most of the other abstractions at this time and work on a single piece of the puzzle. This allows you to focus on a part of the problem and solve it well while knowing how it fits into the bigger picture.
It is important to have a good understanding of the problem and the solution space to create a scalable upfront design. Building abstractions without understanding the problem and solution space can lead you to suboptimal solution as it locks you down to a particular implementation detail - Premature Generalization Is Evil.
On the other hand if you are in an exploratory phase, trying to create a design upfront can distract you from the actual problem at hand. You will end up spending a lot of time thinking about what are the right abstractions for a given problem space or how to best utilise the language features to create a terse implementation.
To avoid such diversions, it's better to pick one simple flow and codify it the best you can and build on top of it till you have enough understanding of what the solution should look like. This piece of code acts as a Tracer Bullet indicating how close you are to the target. Once you have sufficient understanding of the problem and the solution space you can refactor the code to improve the code quality.
Another nifty tool often employed to evolve the system design over time is TDD/BDD. Test driven development helps you split the problem domain into smaller chunks while defining the interactions between various objects by defining a concise API. It allows you to focus on a small subsection of the problem while providing a safety net to ensure that system as a whole is working as expected.
Which is Better?
Unfortunately there is no panacea when it comes to software development methodologies. Every situation is unique so is every team. Depending on the problem at hand and expertise available the team needs to set appropriate pace. More often than not, a project will oscillate between these two extremities during it's lifecycle.
These are the practices you should follow for robust and well maintained code.
Single Object Responsibility
In object-oriented programming, the single responsibility principle states that every class should have a single responsibility, and that responsibility should be entirely encapsulated by the class. All its services should be narrowly aligned with that responsibility. The reason it is important to keep a class focused on a single concern is that it makes the class more robust.
"There are only two hard problems in Computer Science: cache invalidation and naming things.” - Phil Karlton.
The importance of well-named classes and methods can't be emphasized enough. When another developer (or you, a couple of weeks down the line) is reading the code, the names should make it obvious what the class / method is supposed to to. At the very least, keep away from cryptic abbreviations, as illustrated by Steven Deobald here.
Single assertion per test
Tests are easier to read if there is a single assertion, that tests a particular aspect of the functionality.
The above test can be split up into the following
Rails Best Practices
Skinny controller, fat model
Push all logic to the model.
In fact, there are standard patterns available for each of the 7 RESTful actions, so try to fit into one of them.
Use ! as a name for any method that saves in the database
By convention, ! is used for any ruby method is dangerous in some way. One dangerous effect that we often care about is that modifying an entity that will be persisted.
When to use this also fits in to transaction management, and these are decisions that should be made on a case by case basis.
But, for the most part, it's usually a good idea to name methods that do a save in the db as a side effect with a !, and use either exceptions or the errors collection to handle error scenarios.
Do not use RJS
This is bad for multiple reasons:
Security implications of eval() on the client side
Debugging requires you to keep a lot of context about what state the dom is in. In the above example, it is important to know that the .story div is hidden
The API that we have written is pretty much tied to a particular page. If i'm calling my API from another screen, I will need to create another div with the same class (story)
Use handlebars to render AJAX calls
Maturity for AJAX calls is as follows:
Worst: Use RJS (see above)
Bad: Render HTML in the output of the method, and just attach that HTML to the DOM
Good: Let the API return some JSON. Use Mustache or underscore to render
Have a very RESTful API
REST is based on a PHd paper by Roy Fieldings.
Borrowing from REST, the rails world has evolved it's own set of best practices around how to build a URL. One thought is to have every URL be one of seven standard actions. Every modification of a resource can be modeled as a CRUD operation on another resource.
can be remodeled as follows
In the latter case, we are creating a cancellation resource within an order. Internally, the service will cancel the order as the cancellation object is created. The cancellation object may or may not need to be actually stored in the database, or even modeled. The cancellation object is a facade to the external world, to help any HTTP aware client understand your interface.
Remember that once you expose an API, you will have to honor it. Be very conservative, especially when exposing nested resources, and belongs_to relationships.
For most apps, you probably want to handle transactions at the controller level
For most CRUD operations, you probably want the entire operation to happen, or not happen. Transactions are something that should be studied on a case by case basis. However, for most cases, the easiest place to start and finish the transaction is at the controller level, so that your model is not littered with it.
Use settings logic to push configuration into files
Settings logic is a great way to have configuration outside of the application, and is a fantastic alternative to using global variables.
Especially when you are doing a feature toggle, or something else of the sort.
Not every model has to be database backed
One common anti pattern that is often observed is to have every single class inherit from ActiveRecord::Base.
This is clearly a no-no. Like with every other programming language, in ruby you will need classes to encapsulate various behavior that does not need to be persisted to the database.
You can include ActiveModel into any class to get some of the nice things you get from ActiveRecord, like to_json, etc...
Controller tests should include an assert on the status code
This will provide you with a helpful failure if your test fails due to a permission error / internal server error. Thus you readily know that the failure is due to something outside your actual assertion, rather than have to figure that out working backwards from a failure message related to your actual assertion.
The failure here doesn't provide a clue that the problem lies in the test setup. But when you assert on the expected status code:
The formal definition for the law of demeter can be found on Wikipedia, but we will summarise to say that method calls in the following form are a really bad idea.
Instead, create a delegator in the parent class
This allows Dog to control how it walks. If someday, we invent a robot dog that walks using a hover device, then we need only to change the implementation of walk!, that is internal to dog, and the external world need not do
Every method M on an object O should only access these types of methods:
methods on O itself
methods on M's parameters
methods on any objects created/instantiated within M
methods on O's component objects
Tell Don't Ask
When you want an object to perform an action, never ask the object for information about it's state, make a decision based on the information, and then tell the object to perform the appropriate action.
The responsibility of how to perform the action should reside in that object, not the caller. In short, tell the object what you want it to do, and let that object worry about how to do it.
If a method is over 7 lines of code, more often than not the method would be doing more than what it should, and a candidate for refactoring. At the very least, pull out a private method to improve readability of the large method.
Having a single level of indentation makes it obvious what every method does at a glance.
can be replaced with
Polymorphism over if conditions and switch / case
Avoid if conditions & switch / cases whenever possible, instead preferring polymorphism over them.
Whenever possible, try to isolate the place where switch/cases are used to a single method where appropriate strategy (Strategy Pattern) is created to care of the behaviour, and allow polymorphism to take over.
If you must use if conditions, then use it in one of the forms:
With the growing awareness of the need to integrate user experience design into agile delivery, there is now a better recognition of the role played by design facilitators in enabling good design faster. I wrote a blog post to share my insights on what should be a design facilitator's playbook:
We receive a lot of love from our users about RubyMonk's visual design. And while we agree -- it's super cute! -- the site's visuals are hardly its interface. The "user interface" for RubyMonk is really the boundary between the student and the mentor.
How do we evaluate your code? How do we decide whether it is correct or not? These are the interesting questions in the user interaction. And, as emerges in the design of any product, these questions highlight the necessary unification of form and function, engineering and design, purpose and path. When you boil it down, the purpose of RubyMonk is to automate the teacher. On some level, this goal might seem akin to beating the Turing Test. In its current incarnation things are much simpler, of course.
I've explored this idea a bit in my post "defining the interface for rubymonk". Take a look and check back soon as I take this idea forward to discuss the yin and yang of tests and the boundaries they exercise.