4
\$\begingroup\$

I am new to Test Driven Development and currently practicing it with some problem statement. Following is an Assembly Line Problem statement I solved using TDD approach in java. Please provide me some guideline on improving it, also if I am doing it the correct way?

Orignal Problem Statement is : http://www.geeksforgeeks.org/dynamic-programming-set-34-assembly-line-scheduling/ . In summary, a car factory has two assembly lines (\$i \in \{1,2\}\$), each with \$n\$ stations (\$j \in \{1,2,\ldots,n\}\$). The time taken per station is \$a_{i,j}\$. The car chassis can move one station forward in the same line, or one station diagonally in the other line (incurring an extra delay \$t_{i, j}\$ to move to station \$j\$ from line \$i\$). Compute the minimum time it will take to build a car chassis.

My Git Hub Link To Solution is : https://github.com/sarangshinde/TDD/tree/master/AssemblyLineProblem

Here I am adding just glimpse of what I tried. test class is as follows:

public class TestCarAssemblyTest {

    CarChassisBuilder carAssemble;
    @Before
    public void setUp() throws Exception {
        carAssemble= new CarChassisBuilder();
    }

    @Test
    public void shouldReturn_MinimumTimeSpentInCarChassisBuild_whenNoStationsOnBothAsemblyLine() {
        Station line1Stations [] = null;
        Station line2Stations [] = null;
        AssembleLine line1 = new AssembleLine("Line1",15,25,line1Stations);
        AssembleLine line2= new AssembleLine("Line2",10,20,line2Stations);
        int assembleTime = carAssemble.buildCarChassis(line1,line2);
        assertEquals(30, assembleTime);
    }


    @Test
    public void shouldReturn_MinimumTimeSpentInCarChassisBuild_whenOneStationOnAsemblyLine1() {
        Station line2Stations [] = {new Station(1,10,5)};
        Station line1Stations [] = null;
        AssembleLine line1 = new AssembleLine("Line1",25,25,line1Stations);
        AssembleLine line2= new AssembleLine("Line2",10,20,line2Stations);
        int assembleTime = carAssemble.buildCarChassis(line1,line2);
        assertEquals(40, assembleTime);
    }

    @Test
    public void shouldReturn_MinimumTimeSpentInCarChassisBuild_whenOneStationOnBothAsemblyLine() {
        Station line2Stations [] = {new Station(1,50,5)};
        Station line1Stations [] = {new Station(1,10,5)};
        AssembleLine line1 = new AssembleLine("Line1",25,25,line1Stations);
        AssembleLine line2= new AssembleLine("Line2",10,20,line2Stations);
        int assembleTime = carAssemble.buildCarChassis(line1,line2);
        assertEquals(60, assembleTime);
    }   

    @Test
    public void shouldReturn_MinimumTimeSpentInCarChassisBuild_whenTwoStationsOnAsemblyLine2() {
        Station line2Stations [] = {new Station(1,30,5),new Station(2,20,5)};
        Station line1Stations [] = {new Station(1,10,5)};
        AssembleLine line1 = new AssembleLine("Line1",10,20,line1Stations);
        AssembleLine line2= new AssembleLine("Line2",10,20,line2Stations);
        int assembleTime = carAssemble.buildCarChassis(line1,line2);
        assertEquals(40, assembleTime);
    }

    @Test
    public void shouldReturn_MinimumTimeSpentInCarChassisBuild_whenTwoStationsOnBothAsemblyLine() {
        Station line2Stations [] = {new Station(1,30,5),new Station(2,10,-10)};
        Station line1Stations [] = {new Station(1,45,5),new Station(2,5,2)};
        AssembleLine line1 = new AssembleLine("Line1",10,20,line1Stations);
        AssembleLine line2= new AssembleLine("Line2",10,20,line2Stations);
        int assembleTime = carAssemble.buildCarChassis(line1,line2);
        assertEquals(60, assembleTime);
    }
}

src classes as follows:

    public class Station {
public int getStationNumber() {
    return stationNumber;
}
public void setStationNumber(int stationNumber) {
    this.stationNumber = stationNumber;
}
public int getTimeSpentAtStation() {
    return timeSpentAtStation;
}
public void setTimeSpentAtStation(int timeSpentAtStation) {
    this.timeSpentAtStation = timeSpentAtStation;
}
private int stationNumber;
private int lineChangeCost;
public int getLineChangeCost() {
    return lineChangeCost;
}
public void setLineChangeCost(int lineChangeCost) {
    this.lineChangeCost = lineChangeCost;
}
public Station(int stationNumber, int timeSpentAtStation,int lineChangeCost) {
    super();
    this.stationNumber = stationNumber;
    this.timeSpentAtStation = timeSpentAtStation;
    this.lineChangeCost=lineChangeCost;
}
private int timeSpentAtStation;}


    public class AssembleLine {
public Station[] getStations() {
    return stations;
}
public void setStations(Station[] stations) {
    this.stations = stations;
}
public String getName() {
    return name;
}
public void setName(String name) {
    this.name = name;
}
private Station stations [];
private String name;
private int entryTime;
public AssembleLine(String name,int entryTime, int exitTime,Station [] stations) {
    this.name=name;
    this.entryTime = entryTime;
    this.exitTime = exitTime;
    this.stations=stations;
}
private int exitTime;
public int getEntryTime() {
    return entryTime;
}
public void setEntryTime(int entryTime) {
    this.entryTime = entryTime;
}
public int getExitTime() {
    return exitTime;
}
public void setExitTime(int exitTime) {
    this.exitTime = exitTime;
}
}



public class CarChassisBuilder {

    private int buildCarChassisTime(AssembleLine line) {
        Station[] stations = line.getStations();
        int assembleTime= line.getEntryTime() + line.getExitTime();
        if(stations==null || stations.length==0)
            return assembleTime;
        else if( stations.length==1){
            return assembleTime+Arrays.stream(stations)
                                      .mapToInt(station -> station.getTimeSpentAtStation())
                                      .sum();
        }
        else {
                int timeSpentAtAllStations = IntStream.range(1, stations.length)
                     .map(station -> CarChassisBuilder.min(CarChassisBuilder.timeSpentBetweenTwoStations(stations[station-1],stations[station],false),
                             CarChassisBuilder.timeSpentBetweenTwoStations(stations[station-1],stations[station],true)))
                     .sum();
            return assembleTime+timeSpentAtAllStations;
        }
    }

    private static int timeSpentBetweenTwoStations(Station prevStation, Station nextStation, boolean changeLineFlag) {
        return changeLineFlag ?
               prevStation.getTimeSpentAtStation()+nextStation.getLineChangeCost()+nextStation.getTimeSpentAtStation():
               prevStation.getTimeSpentAtStation()+nextStation.getTimeSpentAtStation(); 
    }

    public int buildCarChassis(AssembleLine line1, AssembleLine line2) {
        return min(buildCarChassisTime(line1),buildCarChassisTime(line2));  
    }

    private static int min(int assembleLine1Time, int assembleLine2Time) {
        return assembleLine1Time < assembleLine2Time ? assembleLine1Time : assembleLine2Time;
    }
}

public class MainClass {
    public static void main(String []args){
        CarChassisBuilder carAssemble= new CarChassisBuilder();
        Station line2Stations [] = {new Station(1,30,5),new Station(2,10,-10),new Station(3,5,5)};
        Station line1Stations [] = {new Station(1,45,5),new Station(2,5,2)};
        AssembleLine line1 = new AssembleLine("Line1",10,20,line1Stations);
        AssembleLine line2= new AssembleLine("Line2",10,20,line2Stations);
        int carChassisBuilTime = carAssemble.buildCarChassis(line1,line2);
        System.out.println("Minimum Time To Buil Car Chassis Is :" + carChassisBuilTime);
    }
}

Please feel free to provide review comments on it. Also provide guidance on if my code is following SOLID principles,if modeling of problem statement is right and how to improve its design.

\$\endgroup\$
1
\$\begingroup\$

I am new to Test Driven Development and currently practicing it with some problem statement. Following is an Assembly Line Problem statement I solved using TDD approach in java. Please provide me some guideline on improving it, also if I am doing it the correct way?

TDD is a process and can hardly be evaluated by the results you got out of it. If you are really interested in that you should commit every step of the TDD cycle as a single commit (that means every time before you change to the next of test, code, refacor). Alternatively create a screen cast video of how you do TDD.


Beside this your code does have some points to talk about. I only consider the Test class:

pros

  • naming of your identifiers follows Java naming conventions or break it where it improves readability (test method names)
  • your identifier have meaningful names
  • your test method have the three parts arrange, act and assert separated.

cons

where is light theres also shadow... ;o)

Again I only look at the test class.

magic numbers

You code contains some literal numbers that have no common obvious meaning (like 0)

You should declare constants for them to enlight the reader.

real dependency objects

Your tests instantiate objects of class Station and some of your tests rely on these objects to return expected values based on the input given to them.

when that other objects change for any reason (e.g. they get another constructor parameter) your test will break although the change is completely unrelated to your tests. Since not compiling is failing your tests fail for the wrong reason. It at least means more work when refactoring the Station class.

You should better use a mocking framework to create mock instances of your dependencies which can be configured to return values completely independent of the dependencies real implementation. One more advantage of mocking dependencies is that they don't have to be (completely) implemented when you write your test for the depending cut. This is what we mean by test a unit in isolation.

An exception here are data transfer objects (DTOs) that only hold data and have no business logic. Mocking them is usually a bit "over the edge"...

naming

All your test methods begin with the same string shouldReturn_ and have the pattern _when somewhere later. It basically repeats what the code inside the method expresses itself.

I understand that it is easier for a beginner to write that words in the test method names. But since they are the same throughout all your test methods they do not convey any relevant information.

On the other hand there is important information missing: what is the name of the method in your cut being called? And what is the business requirements this test verifies?

The proposal of Roy Osherove in "The Art of UnitTesting" is to name the test methods like this:

methodBeingCalled_precondition_expectedBehavior()

where precondition`` andexpectedBehavior` are noted in terms of the business case.

I personally extend this to:

requirementReference_methodBeingCalled__precondition__expectedBehavior()

one of the good thing with this naming variant is that the names group nicely per called method (or per requirement in my version) when shown in alphabetically order, as most IDEs do in their outline view.

visual separation of arrange, act, assert

Your test method do not contain empty lines to separate those parts visually.

This is no problem with small tests like that, but soon you will have larger tests with more complexity in either part and then it will help the reader to understand your test.

\$\endgroup\$
  • \$\begingroup\$ Thank you Timothy for your valuable response. Sure i will follow your suggestions. But I have seen mocking mostly external dependencies e.g database connection and quries or webservice request shall we need to use mocking always in our code whenever we have dependencies between objects. \$\endgroup\$ – Sarang Shinde Jan 22 '17 at 14:43
  • \$\begingroup\$ @SarangShinde: The definition of a UnitTest is tests a piece of production code, that provides a certain behavior, in isolation. By that definition it is quite opinion based what is part of the unit and what not (and should therefore be mocked for testing). I (again) like Roy Osheroves statement about that: "A unit is a part of the code that changes for the same reason". This means anything that has a reason to change that does not affect the tested units behavior or interface should be mocked. \$\endgroup\$ – Timothy Truckle Jan 22 '17 at 15:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.