Wishful Thinking & Test-Driven-Development

In the past few months I’ve been doing something a little bit different from the approach that I usually take while programming/developing software. If you’ve read the GREAT book SICP, you should be familiar with the term “Wishful Thinking”! (If you haven’t, I HIGHLY recommend reading it and also watching the talks by Professor Sussman and Abelson on the topics which you can find here)!

There are tons of places that you can see/hear that term but that book is probably one of the first places that talk about this idea. And it’s basically about not thinking about too many levels of abstraction at the same time and not jumping around with your brain for solving a problem. That means lay down the steps that you need to take to accomplish something and don’t think about HOW to do each of those steps at that very moment. It’s like a 1000 feet view of that specific task at hand. When you have the map for the steps, when you know WHAT are those steps, then try to figure out HOW to attack each of them. Then you focus in! Think of your whole application as a map and what you do here is basically taking a piece out and focus in in few steps! First laying down the steps of work needs to be done in order to accomplish that task and then figuring out HOW to do each of those steps! Hopefully following picture will give you some ideas.

map_focus_bubble_better

This sounds a lot like up-front design and frankly, if the level of abstraction that you are dealing with at one point is a whole application or even something really big in your application, you ARE doing an up-front design. And anybody knows that up-front design is not a good idea in most cases and I’m not going to talk about that, thousands of people said that way better than I ever can. But at the same time if we use this technique (Wishful Thinking) at a correct level of abstraction or at the right point, it can be really helpful and powerful. Imagine we’re writing a twitter client* application and the feature we’re trying to implement (the task at hand) is getting the last tweet of all the people you are following (what a useful feature BTW)! The naïve (for blog purposes) implementation steps for this feature is going to be something like the following:

  • Get the list of people who the current user is following.
  • Iterate through each of those people.
  • Get the last tweet for them.
  • Pack those tweets and Tada!

That’s like a 1000 feet view of this specific feature. And if you look at them, those can be exactly the methods that we can have in our code for this feature:

 def following_last_tweets
   following = get_following_of(current_user)
   last_tweets = []
   following.each do |person|
   last_tweets << get_last_tweet_for(person)
   end
   last_tweets
 end
 

And again this is not an up-front design or anything like that. We’re completely focused at a granular feature of the application at this point and laid down the steps needed to implement that. So the level of granularity in doing this matters and you should be careful of not flying so high! So now it’s the time for the question HOW we’re going to attack each of those steps? If you try to answer that question completely right away, the result of your code probably won’t be very elegant. What do we do then? The thing that we usually do! We answer that question for each of those steps via TDD. Actually we even comment out that code that we just wrote, because that code does not even work, it’s just a map for the steps that we need to take to accomplish that feature. Think of it as a TODO list in your code instead of having it in a separate text file or so!

The nice thing about this approach which is basically a combination of “Wishful Thinking” and “Test-Driven-Development”, is that we get some guidelines from our wishful thinking and then try to come up with a nice solution for each of those steps in a bottom-up and incremental fashion using our unit-tests and letting them DRIVE us toward that solution and a good design.

We don’t do any over-design or over-engineering and this is not against YAGNI either. If we are going through one of the steps and then realize: “This is not the way to go for it” it’s just a matter of reverting couple minutes of work (and thank God for having version controls to make it as easy as it can get), even if we realize that our map/guideline has problems, we haven’t gone too far with this approach and we can easily think of new steps and start over again. Because it’s just a matter of few minutes and our map wasn’t even functional, it was just few lines of comments for directing our overall approach for that specific piece of functionality.

That’s why it’s VERY important to use this technique or this combination of techniques at an appropriate level of abstraction and granularity in the application! Doing it at a VERY high level is just an up-front design and can make you do hours of effort and reverting all that back because of over-engineering/design and not considering some aspects which you’ll find later on and you KNOW that HAPPENS! Doing it at a VERY low-level is not going to provide any value (almost) and you might as well do the complete bottom-up approach without having any map or guideline cause your steps will become too much primitive. (e.g print a username or the like!!!)

It’s similar to Kent Beck’s rule of Single Level of Abstraction which we can only realize what that really means and how to get it done correctly by practicing A LOT and watching the code and thinking about it deeply for a while to see what’s going on at each point and is there a nice harmony between those lines of code at each method or module. For instance if all you’re doing in a method is calling some other methods and all of a sudden there’s a very primitive line of code like a = 2; in there, there’s a good chance that line does NOT belong there!

I’ve been trying this approach for a while and it’s been serving me well. I ended up writing more interesting and cleaner and better-designed code because of that. Maybe a lot of people write their code exactly like this or maybe we’ve been always doing this but we just lay down the steps in our mind unconsciously instead of writing them down IN THE CODE and attacking them one by one!

Anyway, I think it’s a good time for me to stop preaching, I found this approach interesting and useful and I thought maybe it is useful for someone else as well!

Hope you find it interesting as well and happy hacking ☺

* The reason that I recently use this kind of application as an example, is because I’m writing one just for fun and it’s full of interesting points and examples. You can find the code for it here!

Nested Stubbing => Shouts for Refactoring

A lot of programmers write unit tests during the development and also a lot of programmers do Test Driven Development. One thing that we usually forget while programming is Listening to our Tests. If we listen carefully to our tests they will give us a lot of interesting hints and information and frankly that’s why a lot of people call it Test-Driven-Design because we can find useful points in our tests that will help us to have a better design.

I’m going to talk about one of those points that we can find out very easily by listening to our tests and will help us to have a better design and having a piece of information in its right place in our program. That certain point is Nested Stubbing which BTW happens a lot in our tests.

Lets say we are writing a twitter client app and all the communication through the network, OAuth related parts, calling Twitter API, fetching and storing tweets, etc. is being done in separated modules (Separation of Concerns and Single Responsibility Principle etc.)

We have also different kinds of presentations (views if you will) for this application. One of them is a Console Based presentation for taking a look at the tweets in terminal. One part of this presentation is taking the latest 10 tweets and rendering them (in whatever way you like) to the user. Obviously the rendering related code lives in its own place separated from the logic, data storage etc. Imagine we have a Console class which gets those latest tweets and give them to the renderer object in order to show them to the user (for brevity I remove some parts):

def show_latest_tweets
    last_ten = Tweet.order(‘created_at DESC’).limit(10)
    renderer.render_tweets(last_ten)
end

and lets say we have a test for this piece to make sure that this method is calling the right things:

it “fetches last 10 tweets and renders them” do
    latest_tweets = [t1, …, t10]
    ordered_tweets = stub(:limit).with(10) { latest_tweets }
    Tweet.stub(:order).with(‘created_at DESC’) { ordered_tweets }
    renderer.should_receive(:render).with(latest_tweets)
    console.show_latest_tweets
end

**

If you look at this code there is an obvious violation of  Law of Demeter happening. We know that when we call order on Tweet what is being returned has a limit method that we can call to limit the results. How can you easily detect this? BECAUSE WE’RE DOING A NESTED STUBBING IN OUR TEST. We are setting up a nested stub cause we setup ordered_tweets as a result of calling order on Tweet which is a stub itself. This tells us that we know TOO MUCH about the inside of Tweet class and its details at this level (Console class) which we should NOT!

Right now we’re using something like ActiveRecord as the ORM for storage part of the application but what if we change that later to something else? There’s a good chance in that new ORM the mechanism for doing the same thing (getting the latest 10 tweets) will be different and we need to change our code appropriately. But with this code that we wrote here we have to change Console class for changing our storage mechanism which does not make any sense. Console SHOULD NOT know anything about the details of storage and storage should NOT be a REASON of change for Console.

We need to have a layer which hides this information from Console and give him what he wants instead of Console reaching for that information through method chains (Tell, Don’t Ask). As you can see Tweet is like a model (if you will) in this application. And he’s the one who should know about the storage mechanism in this app (how to be stored, retrieved etc.) [Or even taking it further there can be a TweetRepository class which is specifically handling storage-related stuff for Tweet, kind of like a Façade Pattern between Tweet and DB/File/etc.]

We can add a method to Tweet class like the following:

def self.last_n_tweets(n)
    Tweet.order(‘created_at DESC’).limit(n)
end

So now lets rewrite our test based on this change:

it “fetches last 10 tweets and renders them” do
    latest_tweets = [t1, …, t10]
    Tweet.stub(:last_n_tweets).with(10) { latest_tweets }
    renderer.should_receive(:render).with(latest_tweets)
    console.show_latest_tweets
end

And now the code for it:

def show_latest_tweets
    last_ten = Tweet.last_n_tweets(10)
    renderer.render_tweets(last_ten)
end

First of all, we don’t have that nested stubbing in our test method but that’s not the point, if you pay attention you see that we eliminated the coupling and dependency of Console to the Storage mechanism and it doesn’t have any knowledge about that part anymore which is a big advantage. If we decide to change the data storage part of this application or change our ORM, that change will be hidden from Console or any client of this functionality (Tweet retrieving stuff etc.)! Console will still call Tweet.last_n_tweets and how’s that being implemented is none of its business and it doesn’t care.

As you saw listening to our test can have interesting results and nice design hints. Whenever you feel that your test is more work than it should be, or it doesn’t seem to be right or it’s hard to write the test for the target piece of code, that means there’s a design problem in our code (MOST OF THE TIMES). So we should fix the problem in the right place not struggle with making that test happen anyway. It’s time to stop preaching.

Hope that helps and happy hacking.

** This test contains more than one assertion which is usually not a good idea (there are exceptions depending on the situation like anything else in programming)!  I wrote those here to show how things are supposed to work in that piece of code!

Few more characters better than jumping around

This is a short post for pointing out a quick and handy little thing that I found helpful in some cases while programming.

Some people are religiously against comments and they know the existence of comments in the code as a sign that means the code is not readable! (I was in that group for a while as well :P)

That’s not necessarily a true thing to say or believe. Comments can also exist in a readable and clean code. And they can be very helpful. In one specific case that I want to mention here, they can save you some scrolling and jumping around the file in order to get a sense out of the code you’re reading.

I’m a huge fan of the statement: “Imagine the next reader of your code is a serial killer and he knows where you live!” I always try to make my code as readable as I can at the moment. I like to be able to get a sense of my code with a glance without any need to dig deeper. Of course this is hard to achieve 100 percent but I try to make it really close when I can. One of the things that I noticed in my test code is that some times in order to have DRY (Don’t Repeat Yourself) I extract out a helper method or a setup block for setting up some data that I need to reuse in different tests that I’m writing.

Imagine you’re having a model named Event and you’re testing some validation rules for this model. Instead of keep repeating some values for its attributes in every single test it’s a good practice to put that as a setup in a “let” block in rspec as following:

let(:attributes) do
    {
        event_type: “CreateEvent”,
        generated_at: sometime,
        event_no: 1
    }
end

and  just change the one that is relevant to the validation rule that you’re about to test in your current test like the following:

it “requires a type” do
    event = Event.new(attributes.merge(event_type: nil))
    event.should_not be_valid
end

This makes your tests less polluted with repetitive values for the attributes over and over again. It’s a small thing with almost a big effect in the code cleanness.

Sometimes having this setup block makes you to scroll and jump around the test code to realize what are the values cause you’re depending on them in order to understand the test you’re reading and trying to understand. Not always you need to know the values. For instance in the above test with a quick glance you see what’s going on and you don’t care what are the values for the attributes cause the only thing that is matter (being tested) here is that event_type can’t be nil and it’s a required attribute.

But sometimes you need to know what’s going on in the attributes. Imagine a scenario that you need each event have a unique value for the event_no attribute. And you wanna test it like the following:

it “needs a unique event_no” do
    event1 = Event.create!(attributes)
    event2 = Event.new(attributes.merge(event_no: 2))
    event2.should be_valid
end

Now here you cannot understand the complete picture with just a glance. You can guess it obviously and I want to make the code completely readable so the reader won’t have to think/guess as much as possible or she won’t have to jump around the code/file to realize what’s going on. When you see this you can guess that the first event (event1) has a different event_no value so that’s why the second event (event2) is valid. Which is totally fine but I don’t think there is any harm in being even finer when it’s the least amount of effort required in order to get there:

it “needs a unique event_no” do
    event1 = Event.create!(attributes) #=> event_no = 1
    event2 = Event.new(attributes.merget(event_not: 2))
    event2.should be_valid
end

That’s it!!! Now with this few characters you saved your life (if the reader was a serial killer and he was pissed by having to scroll up and see the value for event_no in attributes)

I’m sure you think this is TOO MUCH. But again since it’s the least amount of effort and it makes the code even cleaner and more readable than before why not?! It’s so tempting to make it like this and save some scrolling and jumping or unnecessary guessing! Cause this makes the reader to get COMPLETELY what’s going on with just a glance which in my opinion is the best level of readability in the code => Glance-Understandable!!!

Anyway, I stop preaching right now and that’s all I have to say about this. I hope that will be useful.

Sometimes remove the tests after they served you!!!

I know it sounds a little bit strange when I say removing tests after they served you specially because we all know that one of the great benefits of having automated unit-tests is that you’ll end up with a regression suit that you can always run whenever you make a change in your code! So as long as you’re maintaining a project they are serving you in that matter!

What I’m targeting here is the tests that we write during the development directly for methods that should be private in the class-under-test (CUT)! I’ll show you what I mean with a SUPER simplified example. Technically I eliminated everything in this code for showing my main point!

Imagine you’re writing a microblogger which is using a third party API as a twitter client. As we know one of the features of twitter is Direct Message  (known as DM). For implementing this feature you will write a method in your MicroBlogger class named “dm” for example. Of course we’re doing it TDD so one test for this class would be like the following:

it “doesn't send message for someone who’s not following you” do
    twitter_client = stub(:followers). and_return([])
    microblogger.dm(“anyone”)
    twitter_client.should_not_receive(:dm)
end

Now we’re gonna write enough code to make this pass. When we start to implement the production code for this test we realize that we need to check whether the receiver is following us or not![1] This is a unit of work on its own! There are 2 options here for us. A) We start implementing the dm method and inside that method we insert the logic of this check and see whether the test will pass or not B) Make the current test pending and start TDD-ing that unit-of-work (checking whether the receiver is following us or not) and as soon as we have that functionality in place un-pending (weird word) the test and continue the development of dm method.

IMHO the second option is better for several reasons. First of all the first option is violating the rule of taking the smallest possible steps and going in the baby-step style that eXtreme Programming and TDD talk about it cause it’s obviously going through more than one unit-of-work. Also if the test fails after implementing the dm method with the first option we need to check more than one thing, either the check for followers has a problem or the other part of the dm method. And we should have the least number of reasons for a test to fail (1 is the best). And some other reasons which are not for this post.

Then we make the original test pending and start TDD-ing the check functionality for followers. Couple tests for something like that would be the followings:

it “knows when someone is following you” do
    …
end

it “knows when someone is NOT following you” do
    …
end

Then the production code for making them pass can be in a method called followed_by?(someone)

Now it’s time to un-pending the original test that led us to here and implement the dm method to make it pass. Part of the dm method for making the test pass could be like the following:

def dm(receiver)
    …
    return if !followed_by?(receiver)
    …
end

Perfect. Now think about the design of the class under test and how its interface is gonna be to the outside world (other classes/modules of the system)! The followed_by? method should not be visible to the outside world. This is a method that is being used internally in the class and we defined it cause we didn’t want our dm method to do more than one thing! Remember Uncle Bob definition of clean method: “A method should do one thing and only one thing and do it well!”

So what are we gonna do now? If we make the followed_by? method private then those couple tests will complain about it cause they can’t see it anymore and we have to remove them to shut them up! It’s A OK to do that in this case. Sometimes you can remove the test after they served you. We TDD-ed the functionality of followed_by? in our code and we’re using it with no problem in the dm method. And it’s completely a good choice to make that method private according to the circumstances and how it’s being used in the class. And followd_by? method is still being tested indirectly through the tests for the dm method, so we didn’t ruin the coverage of our automated tests.

And I know that we don’t have that ONE reason for failure anymore but again “Programming is the world of trade-off” and IMHO this time it’s a better idea to make the method private and remove the tests related to it cause they served their purposes during the development of that part. I learned this interesting technique and idea from Kent Beck TDD screencasts by Pragmatic Programmer which I highly recommend you to check them out! This technique came handy and very interesting in a bunch of places while I was writing code and I thought it’s a good idea to share it with you.

Hope that’ll be useful!

[1] When we’re doing Red-Green-Refactor cycle in this case we just define the dm method and the code will pass cause we’re not calling the dm method on the twitter_client (writing only enough production code to make the failure pass)! And then with more tests we drive the design and development of the dm method towards this whole idea of checking whether the receiver is following us or not! But again this is a blog post and I wanted to make another point here so I cut through the path a little bit 🙂

Just because you can, does not mean you should!

I tweeted about this thing a couple times and now I want to elaborate a little more on it with an example! A lot of times I hear people talking about the fact that something is much easier to do in a specific language and we can do it that way! And a lot of times that makes sense and is a good way to do that specific task in that specific language! But sometimes it’s just a violation of a principle that we’ve learned the hard way that it matters and we should be careful about it in our code base! For example imagine we want to write a user authenticator which determines whether user’s credentials are correct or not! You can see something similar to the following in a lot of code bases! (Remember that it’s just a very simple thing just for the sake of this post and by no means this is a user authenticator :D)

describe UserAuthenticator do
    it “does not authenticate with wrong password” do
        user = stub(:username => “bob”, :password => “pwd”)
        user_rep = stub(:get_user_by_username => user)
        DatabaseUserRepository.stub(:new => user_rep)
        authenticated = subject.authenticate(“bob”, “wrong password”)
        authenticated.should be_false
    end
end

class UserAuthenticator
    def initialize
        @user_rep = DatabaseUserRepository.new
    end

    def authenticate(username, password)
        user = @user_rep.get_user_by_username(username)
        password == user.password
    end
end

It’s not this easy to do such a thing in a static-type language like Java or C# but since ruby is a dynamic language (duck-type) so ruby and rspec let you do something like the above code! As long as it responds to “new” message you can stub it in any way you like so a lot of people just because they CAN do this they do something similar in their code! But we forgot the main point here! We are violating the Dependency Inversion Principle and instead of depending on abstraction we are tightly coupling ourselves to a concrete thing (DatabaseUserRepository)! I know that Dependency Inversion Principle in a dynamic language such as Ruby has a slightly different meaning and it’s not exactly as its meaning in Java or C#! But what matters for us here is something that can responds to the message “get_user_by_username”! That’s all that matters here! But we are making ourselves more dependent than what matters! Imagine we want to also support storing user information in flat files or some other mechanism! How are we gonna handle those parts of code for the same logic and functionality?! Are we gonna have another UserAuthenticator that instantiate FileUserRepository in its constructor and the rest will be the exact same?! Of course not and I can see you’re cringing right now! WTF?!

We have something which is exactly helpful in this kind of situation and it’s called Dependency Injection! (Of course there are other ways for handling this, e.g. Strategy Pattern, but I go with Dependency Injection for this post just for fun :D)

We inject the dependency and the only thing that matters to us is that it can respond to “get_user_by_username” message! So when it comes to support different kinds of storage mechanism and their appropriate repositories the code will be totally flexible in that scenario and all we need is a new instance of our UserAuthenticator with a FileUserRepository injected to it! So the code for such a design will be like the following:

describe UserAuthenticator do
    let(:user_rep) { stub }
    let(:authenticator) { UserAuthenticator.new(user_rep) }
    it “does not authenticate user with wrong password” do
        user = stub(:username => “bob”, :password => “pwd”)
        user_rep.stub(:get_user_by_username =>; user)
        authenticated = authenticator.authenticate(“bob”, “wrong password”)
        authenticated.should be_false
    end
end

class UserAuthenticator
    def  initialize(user_rep)
        @user_rep = user_rep
    end

    def authenticate(username, password)
        user = @user_rep.get_user_by_username(username)
        password == user.password
    end
end

The point is not just being able to write test for the code! The whole point of TDD is driving a better design for us so in a static-type language it’s not easy to do something like the first example hence it’s hard to write tests for it this way so it will drive us towards depending to abstraction and also injecting that dependency through constructor so we can easily replace the actual collaborator with a stub and test the unit of work that we want at the moment completely isolation! But because we can write a test like the first example in ruby and rspec it doesn’t mean we should do that! The point is having a better design not being able to write tests for the code!

Remember again, just because you can does NOT mean you SHOULD! Power brings responsibility so use the power in a right way and at the right place!

Something worth a try!

I hope you enjoyed it!

Fast Rails Tests, Smoother Development

If you’re a rails developer or had an experience of writing a rails application you know that running your tests is a really painful and long process because it loads the rails environment at first. I’m assuming you write unit-tests for your rails application because it’s really offensive not to write tests in kind of an application that creates a test/spec folder for you from the very first beginning of its life. It assumes that you write tests for your app because you care about your code and software. Once again I remind you Uncle Bob’s famous quote about writing tests and TDD: “you can’t call yourself a professional developer if you’re not doing TDD.” Anyway, running tests in rails is really painful and too slow. When you are doing your Red-Green-Refactor cycle whenever you run your tests you have to wait several seconds until you get a feedback from your tests. This is not the way how it should be done. This latency and delay is breaking the cycle and is really overwhelming. Pretty soon you won’t run your tests as frequent as you should, therefore you won’t be able to get quick feedback about what you’ve done and you miss a lot of TDD and unit-testing benefits in general.

I personally write my code with TDD approach and the quick feedback is really critical during my development cycle. So this kind of slow test won’t let me to be as productive as I should. Whenever I run my tests I am like: “Son of a Gun, why is this happening?!”

Recently I watched Corey Haines talk at Arrrrr Camp about fast-rails-tests. I can’t emphasize on watching this talk enough if you care about your craft and productivity as a developer. Note that I didn’t say rails developer because there are lots of good ideas that you can take from that talk as a developer and software craftsman in general. In that talk Corey mentions about separating your domain and business logic modules in a way that don’t depend on rails. Not surprisingly, isolation and separation of concerns is a golden solution here, AGAIN. According to Corey’s example imagine you have a ShoppingCart model which has many products in it (ActiveRecord model of course) and at some point you need to calculate the total price of the products in your ShoppingCart. By default when people face this kind of problem they write a method in the ShoppingCart model which calculates the total price of the products like this:

def total_price
    products.map(&:price).inject(0, &:+)
end

BTW this is one of the reasons we will end up with “Fat Models”. Let’s ask a question from ourselves at this point: “does this really have anything to do with rails and ActiveRecord?” No, it’s obvious. So, why we put it in the model? Why we made this logic depend on the ActiveRecord and rails in general? We shouldn’t do this. This is a calculation and logic and it should be separated. There are different ways for doing this according to Corey’s talk and I definitely recommend you to watch it and learn about them because in each situation you should pick the best solution. For instance, we can separate this logic and include it in the model so it has this functionality, but the functionality implementation itself is in the right place and separated rather than being mixed with the model. Here’s the test and code for it:

it “returns sum of price for multiple items” do
    TotalPrice.of(stub(:price => 10), stub(:price => 20)).should == 30
end

module TotalPrice
    def of(items)
        items.map(&:price).inject(0, &:+)
    end
end

Then we can use it like the following:

class ShoppingCart < ActiveRecord::Base
    has_many :products

    def total_price
        TotalPrice.of(products)
    end
end

Now you can have your tests for the total price calculation separately from the model and it has no dependency to rails which is the right thing to do. Your tests for this logic will run extremely fast with no latency and delay because of good separation of concerns. I love this idea and ever since I watched this talk by Corey I’m looking for potential Refactorings in my rails apps for separating my domain and business logics and calculations from my models. One of the perfect candidates for this was in my User model. I have some password machinery in my User model which encrypts the password before the user is saved in the database using the before_save ActiveRecord call back. It kind of looks like this: (I made it so much simpler for focusing on the purpose of this post):

class User < ActiveRecord::Base
    attr_accessible :password
    before_save :encrypt_password

    def encrypt_passowrd
        #some password encryption machinery here
    end
end

My tests for this encryption are really painful like all the rails dependent tests because they take too long to run. But then it hit me that this machinery has nothing to do with rails, it’s an encryption algorithm that I use for my password security issues. So I can SEPARATE this logic from the model perfectly. So I ended up writing the following module:

module PasswordEncryption
    def encrypt(str)
        #some encryption machinery here
    end
end

Then I changed my model accordingly:

class User < ActiveRecord::Base
    include PasswordEncryption

    attr_accessible :password
    before_save :encrypt_password

    def encrypt_password
        encrypt password
    end
end

Now I can run my tests during my development cycle very quickly and get fast feedback which is one of the main purposes of TDD and unit-testing in general. My development process for the business logic and calculations part of the application which BTW are most important parts, became so much smoother and faster and I can completely see the improvement in my productivity when I’m working on a rails app. Here are screenshots from running password encryption related tests in my app before and after this Refactoring and design change:

Look how much difference that design change made. I loved this point by Corey during his talk: “most of the times when people have troubles testing something in their code either about running time or testing the functionality itself they change their tests but it’s Test-DRIVEN-Development and it means that we should change our design because that difficulty in testing shouts about design problem which needs to be solved. So, we should change our design NOT our tests.”

I found this really simple trick which helps me a lot while I’m hunting for this kind of Refactoring and design candidates in my rails apps, whenever you face something in your code ask yourself this simple question: “does this have anything to do with rails?” when the answer is “No” go ahead and separate that logic. Try this and you’ll be surprised and thrilled about the fact that you will develop your app so much SMOOTHER and more FLUENT!

Hope that helps, it helped me a lot.

 

Related Posts:

Running Rails Rspec Tests – Without Rails

Making ActiveRecord Models Thin

Architecture The Lost Years (Uncle Bob)

Intermediate Variables, more Communicative Code

As you know one of the four rules of simple design is that the code should clearly express the intent of its author. So any attempt for making the code more communicative and expressive will pay off later. As Martin Fowler says: “Any fool can write a program that computer understands but the art is writing a program that humans can understand perfectly.”

Kent Beck has a book called Implementation Patterns this whole book is about the techniques and patterns for making the code as communicative  as possible. I strongly recommend reading this book at least twice because you will see how dramatically your code becomes more expressive as a result of these patterns.

We should try to write a clear code which communicates its intent perfectly. Because when someone will read it in future (doesn’t matter close or far) can understand what it does and what was our intent when we wrote it at the first place. Remember, most of the times we will be that someone so it is better to write our code in a way which won’t make us to curse ourselves.

One of the simplest and at the same time great ways for making the code more communicative is using expressive intermediate variables. Unfortunately we forget about this little technique a lot and the main reason for that is the attraction to making things succinct. It is nice to write succinct code but remember succinct means short AND useful. It doesn’t just mean short. But sometimes we forget that and we are so happy while we are making things done in ONE line. Sometimes this one line thing comes at a price and that is less readability and understandability of the code. We fade in this fake beauty so much that we forget about the main goal which is making things clear and expressive. There’s an old saying: “don’t make much at once.” We should consider this continuously during our programming.

Few days ago at work, we found a problem in our filtering process. When in the UI we set some filtering for data retrieving it worked correct but when we removed that filter and retrieved data again that same filtering was considered as before. For instance when I wanted to get users that their first name was John, I wrote John in the FirstName filter text area and it worked as it should but when I removed John from that filter text area again it gave me the users with first name John.

Finally I found out that the filter mechanism didn’t clear itself in some special circumstances and I tried to write a unit-test for it which of course failed and then I made it pass. Sure enough the problem was solved and the filtering process worked like a charm since then. (Sometimes when you are doing TDD you cannot think of all possible cases, so later you will face a problem in your system. The best reaction to that is writing an automated unit-test for it and reproducing the problem in that test and then making it pass. With this approach you will be sure that this situation will be covered all the time through your test suite after that.)

I did this process while our new employee sat next to me. The test was almost something like this:

[Test]
public void ShouldRetrieveAllDataWhenThereIsNoNeedForFiltering()
{
    var searchEngine = new SearchEngine();
    searchEngine.AddFilter(new Filter(“FirstName”, “John”));
    var users = searchEngine.GetUsers();
    Assert.AreEqual(10, users.Count);
    Assert.IsTrue(AllUsersNamesAre(“John”));
    searchEngine.AddFilter(new Filter(“FirstName”, null));
    users = searchEngine.GetUsers();
    Assert.AreEqual(50, users.Count);
}

When I asked her how she feels about this, she said: “what does it mean when you say new Filter(“FirstName”, null)? what is the meaning of null in this context? Does that mean users that their first name is null?” At that point it hit me. I was so excited about making the filtering process in one line that I forgot about the most important goal. I should make the code communicative so the reader can understand the intent perfectly. But apparently I screwed up. So I sat back and looked at the code and thought with myself: “why is this so obvious to me but not her?” and then I found out. I knew that when the search engine gets a filter it will only use it when that filter is effective. Somewhere in the GetUsers implementation the search engine will check this à filter.IsEffective() and IsEffective will return false when the value part of the filter is null etc. And when the filter is effective the search engine will use it otherwise it will ignore it. Because I knew about the implementation details of this code that test was clear to me. But that is the problem. The unit-test is supposed to be the live and executable documentation of the system and everyone should be able to find out about how the system works in general by looking at it not by knowing about the implementation details of the system.

So I made this one line change and everything turned out differently:

public void ShouldRetrieveAllDataWhenThereIsNoNeedForFiltering()
{
    …
    var ineffectiveFilter = new Filter(“FirstName”, null);
    searchEngine.AddFilter(ineffectiveFilter);
    …
}

After this she said: “oh, it is an ineffective filter so it will do nothing and then the search engine should return all the users.”

With that one line and introducing an expressive intermediate variable with appropriate name the whole intent of the code became obvious even to someone who had no idea of the implementation details of the production code.

Conclusion

The main goal is communicating the intent through the code as much as possible. Even simple techniques like expressive intermediate variables can change things dramatically. Remember that these little things make the difference between a crap and a well-crafted code. Do not forget about the goal along the way. Always keep in mind that by expressing the intent clearly the reader will thank you and be able to understand the code perfectly. MOST OF THE TIMES THE READER WILL BE YOURSELF.

Thanks for reading, hope that helps.