Being Cellfish

Stuff I wished I've found in some blog (and sometimes did)

January, 2009

Change of Address
This blog has moved to
  • Being Cellfish

    Can code reviews really change design?


    The other day I wrote about effective code reviews. Once problem I've seen with check in reviews are that once the review take place it is common that one or two days of work have been put into the change so any comments on how the design of the code can be changed are typically too late once we reach a check in review. A bad design is usually changed but an OK design is rarely changed into a great design basically since the design is OK so both reviewer and author does not feel it is worth another day just to improve the design. So check in reviews are not the silver bullet for better designs. You have to do something else too.

    On our team we have design reviews. A design review is something each developer calls out (just like a check in review is called out) and then the developer goes over a proposed design or a few test cases with another developer before he starts implementing things in code. These design reviews have proved to be valuable and has resulted in less design comments during check in reviews.

    As of yet we have not tried pair programming which would replace both the design reviews and the check in reviews in a natural way.

  • Being Cellfish

    Effective code reviews


    Before I started to work at Microsoft I worked with a firewall software. At that company the whole team reviewed code together after a new module was completed and made ready for deployment. Reviewing a module could take days and involved people who had not seen the code once before the review. I don't think this was a very effective way to perform reviews, especially since design decisions that were bad often were only pointed out but not fixed since the module was "done". only fixes regarding security were really fixed after these reviews.

    I think you have to review code continuously in order to make them effective and valuable. And you should have a simple rule making sure all code is reviewed and the simplest rule you can have that works is "all code checked in should be reviewed". This does not mean you have to review before check in or after. Reviewing before check in is advantageous since you get less crappy code check in. The drawback is that a change that others would benefit from takes longer to make it into the code base. Also you don't get a history of the code since the "before check in code" is not checked in. If you on the other hand review code after a check in you can do it whenever you get the time. I think the major drawback with this approach is that it is easy to forget or ignore code reviews if that can be made at a later date.

    The next option you have is who will perform the review. I know some people favor doing reviews alone when you have time. This way no one needs to be interrupted in what they do just to do a review. Another option is to have the reviewer sit down with the author of the code under review and look at the code together. I find this kind of review the most valuable and effective since the reviewer can ask questions directly to the author and discuss things directly rather than compiling a "review report". And the "someone is interrupted" problem is no real problem in reality. In a team of developers it is rare (in my experience) that everybody is 100% occupied. Almost all the time you have somebody being "between tasks" or someone doing boring repetitive updates or just being stuck on some problem. Under all these circumstances the reviewer is happy to take a break from his (or her) current tasks and perform a review. If however nobody is available right away somebody is usually able to do the review within 15 minutes without being seriously interrupted.

    In our team we do these face-to-face reviews prior to check in. Actually our process in very similar to the process described here. And here are my personal guidelines for effective, valuable check in reviews:

    • Author of code prepares the review at his workstation.
    • Author describes the purpose of the changes to the reviewer.
    • The reviewer is in command of the review. This means the reviewer is in control of the keyboard and mouse and the reviewer asks questions.
    • The author should not comment on anything unless asked by the reviewer.
    • Reviewer decides if changes needed as a result of the review should be reviewed again or not.
    It is very important that the reviewer is in control of the review. otherwise authors tend to go to fast and reviewers say nothing because they don't want to look stupid ask the author to slow down.

    Another common question is if there are reviews so simple that you don't have to review them. Examples might include single line bug fixes or correcting spelling errors in comments. Personally I think it is dangerous to have rules where somebody has the opportunity to decide for them selfs. A good rule is unambiguous. So the rule should be "all check ins" since then you don't have to decide where to draw the line. And really simple reviews will be over in seconds so why not have them...

  • Being Cellfish

    How to test private methods


    A question that arises from time to time is how to test private methods. Assuming we're using BDD/TDD the first counter-question is: If you're using BDD how did you create the private method in the first place? I guess there are two relevant answers to this question. Either you have some private method that is made private in order to make the compiler not create implicit methods. This is quite common in C++ in order to prevent implicit type conversions and to hide some operator. The body of the method in these cases is always empty or throws an exception. The other relevant answer is that you have tests covering the private method indirectly but a bug (or new requirement) makes you want to test the method directly because it is impossible indirectly.

    To handle the "private methods to trick the compiler" situation I think you have to be pragmatic. You're doing things in order to make sure you get compiler errors for things you don't want to do. Frankly I don't think it makes much sense in having tests for these things. Just accept that this is OK.

    In the case where you have a private method that you want to start testing directly you basically have two options. The fastest and also the ugliest thing to do is to make the private method protected and then subclass that object to get access to the protected method. This will however not work at all if the class is sealed. Also you're now exposing what was intended to be internal implementation specific things to everybody that inherits from your object. So you're actually changing the behavior of the object quite radically and that is not a good thing. A much better approach is to break out the functionality in the private method into a new class. In this class the previously private method will be public so it is easy to reuse the object elsewhere. Also much easier to test since you need no sub classing.

    Finally I want to mention that having specific tests for private methods are a bad code smell. Either the functionality is so complex that it should have been a separate object in the first place or you should be able to test the functionality of the private method indirectly as parts of other tests. There is no point in having one test for each method in each class - you should have one test for each behavior (at both high and low level) in your application.

  • Being Cellfish

    Joining with previous and/or next row


    Sometimes in my career I've had to create SQL queries that uses data from the previous and/or next row in the result. That is, something I wont on row X depends on row X-1 or X+1 in the result. Most of the time I have not done this in the SQL query itself but rather in worked on the result but a few times I just had to do it in SQL. This leads to a construction with sub-selects so the resulting query is both hard to read and understand and it is slow due to a large number of sub-selects.

    This morning I read a short article describing the use of row_number method introduced in SQL Server 2005. Worth reading if you need to solve similar problem since it removes the need for sub-selects. At least the same amount of sub-selects. To read the article you have to register with the site but it is free and you don't get spammed.

  • Being Cellfish

    Taks boards in real life


    Relocation Task Board

    Relocating half the way around the world takes some planning. As usual when we have a lot of stuff that needs to be done, me and my wife use Post-Its (in this case super sticky sortable cards) and write down every task that needs to be done.

    Generally we also sort the tasks having the most important ones at the top but since our relocation to the US is kind of a fixed mile stone where everything has to be done (not very Agile but...) before we move we haven't bother prioritizing this list (yet).

    Also this time (since we use larger cards) we have grouped related tasks on each index card and when one task is done we mark it with a green "strike over pen" and as soon as everything on one card is done, we throw the card away.

    In order to remember we have to do all this stuff the cards are put up on the side of a cupboard in the kitchen where everybody sees it several times every day.

    I guess this is an example of how you can apply things you do in your agile software development team onto other things in your life and get the same benefits. Using this task board makes it visible to everybody in the family all the things that must be done before we relocate.

    Oh, and in order to make sure we have progress we make sure that we each day have completed at least one task on the board. If that doesn't happen we talk about what can be done the next day and by who in order to get back on track.

  • Being Cellfish

    CAT - Continuous Acceptance Testing


    A few days ago I wrote about Continuous Unit Testing (CUT) and why the need for CUT was a sign of other problems. The last days I have been thinking about CUT a little bit more and I still find CUT questionable but there is another thing - CAT (Continuous Acceptance Testing) that I find very useful.

    Acceptance tests are typically slower than unit tests since they involve several components that are not faked in any way and that may need extensive setup work. So acceptance tests are often much slower than unit tests. The unit tests created as part of the BDD (or TDD) cycle are probably enough to catch most problems but in some cases things will be caught in automated acceptance tests. And since that is the case you have much to gain from running those acceptance tests as often as possible. So why not run them AL the time (continuously) in the background?

  • Being Cellfish

    Phishing Test


    Every time I hear of somebody falling for a phishing attempt it puzzles me since I found it so obvious when I see a phishing attempt. So yesterday I did the Phishing and Spam IQ test. Some of the questions were quite hard since you don't know if you have ordered such a service mentioned or not. But I can feel safe since I scored 10 out of 10 correct. What is your score?

  • Being Cellfish

    (A-Z) Driven (Development|Design)


    In Swedish there is a saying that translates to something like "a loved child is known by many names". Probably there is an English saying for that to but what do I know. Anyway I was thinking the other day about the fact that BDD is known as "TDD done right" and EDD is known to be "a better name for TDD". We also have ATDD, UGDD, DDD and a lot of other double D's. So I saw a challenge in this... Coming up with double D's for the complete (English) alphabet. And I did... Not all of them are that serious but I definitely could find myself using all of them in a conversation. Here is the list - feel free to help me add more...

    Also I feel I must comment on the Development/Design thing. What is the difference? Development things are things used during development all the time. Design things are things done early and/or at a high level when designing the application and should typically include no details on implementation.

  • Being Cellfish

    Unit testing graphical user interfaces


    Once you start using BDD (or TDD (whenever I write BDD I could have written TDD) one common obstacle you encounter is writing tests for your GUI. Most people I know feels it is so hard that they actually don't write tests for their GUI. Instead they keep the GUI layer of the application as thin as possible and reduces the number of defects there that way. Others use tools like Selenium to add tests once the GUI is created. In both cases you're not really being a strict BDD practitioner.

    Actually I think this is a very common, pragmatic decision made by many developers since most people find GUIs very hard to test. But are they really? I recently looked at a company internal web cast on testing where the speaker said something like: "I've written unit tests for user interfaces most part of my career. When I started nobody told med unit testing user interfaces was hard so I just did it". This is a very important observation. If you are a convinced BDD practitioner I think you believe that the use of BDD will lead to a better and more testable design. You have also probably experienced how BDD have changed the way you design your code as compared to before you started with BDD and did everything the old fashioned way. So in other areas you have evolved the way you design your code. But still you write your GUI the same way you did before.

    Why do we stick with the same old GUI code then (other than that everybody keeps saying it is impossible)? I think our development tools are one problem here. Our tools are used to design our GUIs and generates lots of code. And that code is not testable so if we want to write testable code we believe we have to write more code manually. And writing GUI code manually is boring, right?

    Albert Einstein once said: "We can't solve problems by using the same kind of thinking we used when we created them". And I think that is very true for this case too. If we want to write unit tests for our GUIs in a BDD manner I think we must rethink how we write our GUIs. One way of doing it is described in this lightning talk which is in Swedish.

  • Being Cellfish

    Why slow unit tests are a bad smell:


    Earlier I promised to elaborate on why slow unit tests are a sign of problems (or a smell if you like). So here it goes.

    The first thing I would like to look at is when the complete test suite takes too long to run to be part of a tight TDD-loop. If it is just the number of tests that makes the difference and each single test is super fast you will have a very large number of tests. Probably several thousands of tests. This can only mean two things. The least bad thing is that you have a very large application that really need all these tests. But you could probably split things up into different modules and test each module by it self thus reducing the number of tests that need to be run when you're working on a single module. Any dependencies between the modules can be faked in your day to day work. A much worse problem is that you're over-testing your code. That means testing the same thing in several similar (but not identical) ways. While over-testing it self should not be a problem it impacts TDD bad in two ways. First of all writing more tests takes more time so you get the feel that TDD is much slower than "developing the old way". It also makes the feedback slower since tests takes longer to run and that means to loose focus.

    The second reason for slow tests is when a single test actually takes to long to execute just by itself. This is a sign of trouble since it probably means you're having badly written tests or even worse; badly designed production code. The reason for the test to be slow is typically because it takes some time to set up the circumstances for the test. The reason for this is typically a bad design since it is difficult or impossible to fake/stub/mock dependencies away in your test code. If you're having trouble faking dependencies you probably have a too tightly coupled design which most people I know tend to think is a bad design.

    So whenever you feel the complete test suite takes too long to run, don't start looking at what is wrong with the tests - start looking at what is wrong with your production code...

Page 1 of 2 (13 items) 12