0

I was reading through testable code that follows LoD, but got all messed up in my head. So please any guidance regarding this piece of code would be appreciated.

public class HouseConfiguration {

    private final int noOfDoors;
    private final int noOfWindows;
    //.... And so on

    public HouseConfiguration(int noOfDoors, int noOfWindows){
        this.noOfDoors = noOfDoors;
        this.noOfWindows = noOfWindows;
        //.....
    }

        //getters for config
}

public class Contractor {

    public House buildHouse(HouseConfiguration houseConfiguration) throws Exception{
        validateHouseConfiguration(houseConfiguration);
        Window[] windows = new Window[houseConfiguration.getNoOfDoors()];
        return new House(windows);
    }

    private void validateHouseConfiguration(HouseConfiguration houseConfiguration) {
        //validation logic
    }
}

public class House {

    private Window[] windows;

    //Now as the parameters increase it becomes a problem to pass so many arguments in constructor
    public House(Window[] windows /*Also other arguments */) {
        this.windows = windows;
    }

}

Now as the arguments of the House constructor increase it would be difficult to manage it using constructor. Lots of dependencies would be getting passed. Is it a correct thing to do? Or is there a better way that this code can be refactored?

EDIT: Referring to the House constructor arguments

Narendra Pathai
  • 41,187
  • 18
  • 82
  • 120
  • 2
    Well, organize your House object into sub-objects, themselves organized into sub-objects, etc.: A house has walls, roof and rooms. A wall has windows and door. A window has wood and glass, etc. – JB Nizet Jan 05 '13 at 15:46
  • @JBNizet could you also please comment on testability. I mean is there some other guideline that needs to be followed in the above code to be easily testable? – Narendra Pathai Jan 05 '13 at 15:49
  • What would you like to test? This code looks testable to me. – JB Nizet Jan 05 '13 at 15:58
  • @JBNizet I am getting confused about at which point should actually the objects be instantiated? Is it okay to create `Window` object in `Contractor`? How would you test the Contractor? At which point should the injecting of dependency stop and actual objects be created? Sorry for asking a lot of questions.. – Narendra Pathai Jan 05 '13 at 16:06
  • 1
    If it's the responsibility of the contractor to create windows, then there's no problem in creating them in the Contractor class. Window is not really a dependency. It's sort of a "data object". A dependency would be another service object like "architect" or "windowStore". To test the contractor, you would pass it an invalid configuration and check that it throws an exception. Then you would pass it a valid configuration and you would check that the created house has the appropriate number of windows. – JB Nizet Jan 05 '13 at 16:11
  • @JBNizet Thanks that makes sense. But IMO writing code that is testable as well as follows LoD is a challenging task. But it is a great way to think about design! – Narendra Pathai Jan 05 '13 at 16:17

2 Answers2

1

...as the arguments of the House constructor increase it would be difficult to manage it using constructor.

Use the Builder pattern (Effective Java item 2):

public class House {

    //these are immutable if you wish
    private final int mNumberOfWindows;
    private final int mNumberOfDoors;

    private House(HouseBuilder builder) {
        mNumberOfWindows = builder.mNumberOfWindows;
        mNumberOfDoors = builder.mNumberOfDoors;
    }

    public static class HouseBuilder {
        private int mNumberOfWindows; //set defaults here, make final and pass in builder constructor if they must be set
        private int mNumberOfDoors;

        public HouseBuilder windows(int numberOfWindows) {
            mNumberOfWindows = numberOfWindows;
            return this;
        }

        public HouseBuilder doors(int numberODoors) {
            mNumberOfDoors = numberODoors;
            return this;
        }

        public House build() {
            return new House(this);
        }
    }

    public int getNumberOfWindows() {
        return mNumberOfWindows;
    }

    public int getNumberOfDoors() {
        return mNumberOfDoors;
    }
}

Usage:

    House.HouseBuilder hb = new House.HouseBuilder();
    hb.doors(4);
    hb.windows(3);
    House h = hb.build();

    House h2 = new House.HouseBuilder().doors(4)
            .windows(3).build();
weston
  • 54,145
  • 21
  • 145
  • 203
  • Your House constructor could simply take a HouseBuilder as argument. – JB Nizet Jan 05 '13 at 16:54
  • 1
    The whole point of the Builder pattern is to increase immutability, so naming those methods set.. is probably not a great idea. Would make more sense to have: House house = new House.Builder().doors(4).windows(3).build(). – Rob Jan 06 '13 at 14:44
  • @Rob I'll agree that's better. I didn't like the way the setter returned a builder. I also got rid of the getters. I disagree that any of that affected the main objects immutability. – weston Jan 06 '13 at 15:51
  • @weston I didn't say it did. I said it is a poor choice because it makes it seem like the class has setters. Does Bloch use builder method names starting with set? No he doesn't. So why don't we just leave it as you used Bloch's example, but you changed it because it was apparently wrong. – Rob Jan 06 '13 at 16:20
  • @Rob No need to take that line mate. I just did it wrong from memory, and took on board your correction which I appreciate. – weston Jan 06 '13 at 17:34
  • @Rob I know where I've seen that style now (with the `set`) Android `DialogBuilder` e.g. `builder.setMessage(...).setPositiveButton(...).create()` – weston Jan 06 '13 at 18:42
  • Yeah, @weston, I was not criticizing your choice at all, just thought you were getting snippy with me. :) I am a huge believer in the Static Builder! (Figures the Android guys were the manglers.. :) ) – Rob Jan 07 '13 at 02:13
0

I was do it in this way (sorry, it is a C#):

public interface IHouseConfiguration
{
    int noOfDoors { get; set; }
    int noOfWindows { get; set; }
    bool IsValid();
}

public class HouseConfiguration : IHouseConfiguration
{
    public HouseConfiguration(int noOfDoors, int noOfWindows)
    {
        this.noOfDoors = noOfDoors;
        this.noOfWindows = noOfWindows;
    }

    public int noOfDoors
    {
        get { return noOfDoors; }
        set { noOfDoors = value; }
    }

    public int noOfWindows
    {
        get { return noOfWindows; }
        set { noOfWindows = value; }
    }

    public bool IsValid()
    {
        //do validate calculus and return value. For a true, i do not sure it is IHouseConfiguration's duty...
        return true;
    }
}

public class Contractor
{
    public House buildHouse(IHouseConfiguration houseConfiguration)
    {
        if (houseConfiguration.IsValid())
        {
            return new House(houseConfiguration);
        }

        return null;
    }
}

public class House
{
    private Window[] windows;

    public House(IHouseConfiguration houseConfig) //Now, all incapsulated in IHouseConfiguration
    {
        Window[] windows = new Window[houseConfig.noOfDoors];
        this.windows = windows;
    }
}
zzfima
  • 1,528
  • 1
  • 14
  • 21