why is this class not good for Junit testing and how can i improve it?

Question

I have no answer to the question but i assume its because it only has void methods that are only counting anyway. Or is there a better explaination? I also need to make a suggestion to improve it.

public class PlayingField implements Security{
    private int kidsCount;

    @Override
    public void addPerson() {
        kidsCount++;
        soundAlarm();

    }

    @Override
    public void removePerson() {
        kidsCount--;

    }

    @Override
    public int getPersonCount() {
        return kidsCount;
    }

    @Override
    public void soundAlarm() {
        if (kidsCount> 50) {
            System.out.println("cant add a kid to the PlayingField");
            kidsCount--;
        }
    }

}

the methods are called from another class with the following methods:

public class KidsShop extends FashionShop {

    private PlayingField playingField;

    public KidsShop(String name, int area, int rent) {
        super(name, area, rent);
        this.playingField = new PlayingField();
    }
    public void addKid() {
        playingField.addPerson();
    }
    public void pickUpKid() {
        playingField.removePerson();
    }
    public int getNumberOfKids() {
        return playingField.getPersonCount();
    }

yes, i need to explain why playingField is bad for junit testing — nikva, May 25 '20 at 16:53
You can test all the funcs in PlayingField class except `soundAlarm` (I think). — denvercoder9, May 25 '20 at 16:55
i assume its because when i wanna test soundAlarm(), the test doesnt have the value of kidsCount since there is no object but only a private field for all the methods? — nikva, May 25 '20 at 17:03
@nikva as I mentioned in my answer, your class lacks the ability to mock `kidsCount`. It will force you to call add method 50 times before you can test that soundAlarm gives the console output — Steyrix, May 25 '20 at 17:06
so the solution would be to create an Object playingField that has the field kidsCount to make it better for unit testing? — nikva, May 25 '20 at 17:22
I guess it is too boilerplate to create an another class, which will hold only one field. You can provide the ability to mock the field by setter or constructor parameter — Steyrix, May 25 '20 at 17:30

Steyrix · Accepted Answer · 2020-05-25T17:02:53.303

0

I assume PlayingField is hard to test because it only processes one private field, which cannot be mocked properly.

As sonnet mentioned in the comments, all the methods except for soundAlarm() can be tested.

My suggestion to improve it is to make soundAlarm() private, as it is only called by the class itself. Or even better: move the checking logic from soundAlarm() to addPerson method. I think it will be better to check the number of kids before adding another one, so you won't need to call decrement of kidsCount.

edited May 25 '20 at 17:02

answered May 25 '20 at 16:53

Steyrix

2,796
1
9
23

the task demands me to have it in soundAlarm() and i cant make it private because its public in the interface. – nikva May 25 '20 at 17:27
Maybe you should add the setter for kidsCount, or set it via constructor, so you dont need to add 50 times to test soundAlarm – Steyrix May 25 '20 at 17:28

score 0 · Answer 2 · answered May 25 '20 at 17:02

0

I guess it's related to SOLID. PlayingField is initialized right inside the constructor and there is no way to provide another adjusted PlayingField(proxy, successor, mock etc). KidsShop breaks some of SOLID principles, especially Single Responsibility as it knows how to instantiate PlayingField(think about Dependency Injection here thru the constructor). Another principle which is broken is that PlayingField should be an interface, take a look at Liskov substitution principle

answered May 25 '20 at 17:02

slesh

1,902
1
18
29

so a solution would be to not create the playingfield in the kidsShop constructor but rather make kidsCount a field in KidsShop? – nikva May 25 '20 at 17:28
Not clearly get the last part of your sentense, but more correct way from SOLID prospective is to pass PlayingField as a constructor parameter. – slesh May 25 '20 at 17:41

why is this class not good for Junit testing and how can i improve it?

2 Answers2