4

I just got some feedback about a job application Java coding exercise. They did not like the solution and two problems where stated in the feedback (which I'm very grateful for as it's very rare feedback is given):

  • I did not use TDD approach, apparently.
  • I overused static methods, I know static methods are anti OO but I only used them in validation and util type methods.

So two questions here:

What are the possible tell-tale signs of not using TDD approach?

What coding style or patterns can be an alternative to static methods?

Following the first two responses I have another question.

Do you agree that using static methods is only bad when it limits the testability of your code and not in them selves bad.

So going back to my job application exercise solution if the static methods do not limit the testability of my code is it still bad to use? my validate method was very simple 'Validator.notNull(p,"paramName")' now why would I ever want to mock that?

Many thanks.

jakstack
  • 2,143
  • 3
  • 20
  • 37

2 Answers2

5

A tell-tale sign of not using TDD is usage of static methods and static class members for collaborators. You cannot override a static method, so you cannot substitute a mock to test the class using such methods in isolation.

Instead of using static collaborators or static methods on the collaborators, you can use dependency injection. In a simple coding exercise you would inject dependency via a constructor or via the setters by hand. In the real life you can use one of available dependency frameworks.

Olaf
  • 6,249
  • 1
  • 19
  • 37
  • Thanks Olaf, so if I have someBizObj.someBusinessMethod(SomeParam param); and in this method I call Validation.notNull(param,"pramName"); how would you change this based on your suggestion? – jakstack Feb 20 '13 at 18:59
  • +1 for answer. @Ramo see answer below for a trivial example. hope it helps – bas Feb 20 '13 at 20:53
2

Your static Validaton method seems something that should be part of an object to me.

Say you have an class Drink

public class Drink
{
    private readonly string _name;
    private readonly double _temperature;

    public Drink(string name, double temperature)
    {
        _name = name;
        _temperature = temperature;
    }
}

Your businesslogic would be able to instantiate all kinds of drinks, 7up, cola, whatever. You'd like to make sure that a drink has the appropriate temperature to drink it, so you have the need for a Validate method. You could follow your approach:

public void TakeAZip()
{
    if (Validation.HasAppropriateTemp)
    {
        // implement drink
    }
}

'

Alternatives for static classes

That way you have an hard dependency on your static Validation class.

Alternatively you could make use of dependency injection.

public void TakeAZip(ITemperatureValidator validator)
{
    if (validator.HasAppropriateTemp)
    {
        // implement drink
    }
}

If more convenient you could also choose to pass the Validator via the constructor

    private readonly string _name;
    private readonly double _temperature;
    private ITemperatureValidator _validator;

    public Drink(
        string name, 
        double temperature, 
        ITemperatureValidator validator)
    {
        _name = name;
        _temperature = temperature;
        _validator = validator;
    }

Now you can mock the behavior of your validator and you can isolate your Drink class from all external behavior.

bas
  • 13,550
  • 20
  • 69
  • 146
  • Many thanks @bas, so the validator can be implemented as a singleton and passed around as required and when testing we only mock the the validator interface and inject the mock. – jakstack Feb 20 '13 at 23:05
  • 2
    @Ramo Hmm, did you have to use that word... :). Singletons are not your best friend. If you are looking for tell-tales you got one with Singletons. Read this thread as a reference http://stackoverflow.com/questions/5486562/should-i-never-use-static-methods-and-classes-and-singletons-when-following-the – bas Feb 21 '13 at 08:46