Connecting to an insurance database

Question

I'm developing an Android application which would remotely connect to an insurance database and perform some basic CRUD operations and I'd like to learn how to keep clean app's architecture from Fernando Cejas' and Uncle Bob's blog entries. Presented below domain layer contains entities and use cases which I don't want to be affected by any database or UI - they are pure general business rules, but I have few questions regarding their implementation before I proceed to writing data layer, which will obtain data from a cloud, or presenter layer, which would be an Android GUI.

Oc.java

package com.domain.model;

import java.util.Collection;

public class Oc {

    private final String ocId;

    public Oc(String ocId) {
        this.ocId = ocId;
    }

    (...)
    private Collection<Discount> discounts;

    public Collection<Discount> getDiscounts() {
        return discounts;
    }

    public void setDiscounts(Collection<Discount> discounts) {
        this.discounts = discounts;
    }
    (...)
}

Discount.java

package com.domain.model;

public interface Discount {
    enum Type {ADD, MULTIPLY}
}

DiscountImpl.java

package com.domain.model;

public class DiscountImpl implements Discount {
    private String name;
    private float value;
    private Type type;

    public DiscountImpl(String name) {
        this.name = name;
    }

    public String getName() {
        return name;
    }
    (...)
}

I have many empty interfaces for Oc's objects like Discount.java as I wanted to follow ISP and DI. Is that quantity of interfaces a good approach? Should I implement custom annotations for them?

OcRepository.java

package com.domain.repository;

import com.domain.exception.ErrorBundle;
import com.domain.model.Customer;
import com.domain.model.Oc;

import java.util.Collection;

public interface OcRepository {
    void getOcRecentList(final int userId, OcListCallback ocListCallback);
    void getOcById(final int ocId, OcCallback ocCallback);
    void getOcByRegistrationNumber(int ocRegistrationNumber, OcCallback ocCallback);
    void getOcByCustomer(Customer customer, OcListCallback ocListCallback);
    void addOc(Oc oc, OcAddCallback ocAddCallback);
    void removeOc(final int ocId, OcRemoveCallback ocRemoveCallback);
    void modifyOc(Oc oc, OcModifyCallback ocModifyCallback);


    interface ErrorHandle{
        void onError(ErrorBundle errorBundle);
    }

    interface OcListCallback extends ErrorHandle{
        void onOcListReceived(Collection<Oc> ocCollection);
    }

    interface OcCallback extends ErrorHandle{
        void onOcReceived(Oc oc);
    }

    interface OcAddCallback extends ErrorHandle{
        void onOcAdded();
    }

    interface OcRemoveCallback extends ErrorHandle{
        void onOcRemoved();
    }

    interface OcModifyCallback extends ErrorHandle{
        void onOcModified();
    }
}

Will these Repository pattern's hard-coded methods (getOc by id, registrationNumber, Customer...) make my OcRepository rigid, and should I come up with some criteria solution? Can callbacks be shared by a few methods (like OcCallback in this case)?

package com.domain.test;

import com.domain.model.Car;
import com.domain.model.Customer;
import com.domain.model.CustomerImpl;
import com.domain.model.Discount;
import com.domain.model.DiscountImpl;
import com.domain.model.Oc;
import com.domain.model.User;
import com.domain.model.UserImpl;
import com.domain.model.Vehicle;
import com.domain.model.Zone;
import com.domain.model.ZoneImpl;

import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;

public class OcTest {

    private Oc oc;
    private Customer customer;
    private Vehicle car;
    private Collection<Discount> discounts = new List<Discount>() {(...)};
    private Zone zone;
    private User user;

    @Before
    public void setUp() throws Exception {
        /*Discounts*/
        DiscountImpl discount1 = new DiscountImpl("New car");
        discount1.setType(Discount.Type.ADD);
        discount1.setValue(50.0f);

        DiscountImpl discount2 = new DiscountImpl("Disabled");
        discount2.setType(Discount.Type.MULTIPLY);
        discount2.setValue(0.5f);

        this.discounts.add(discount1);
        this.discounts.add(discount2);

        /*Car*/
        Car car = new Car("ASD 5256");
        car.setBrand("Audi");
        car.setModel("A4");
        car.setVin("ASPLEMD1236521012");
        car.setCapacity(1.2f);

        this.car = car;

        /*Zone*/
        ZoneImpl zone = new ZoneImpl(1);
        zone.setZoneType(Zone.Type.ADD);

        this.zone = zone;

        /*Customer*/
        CustomerImpl customer = new CustomerImpl("9876541231");
        customer.setCity("Warsaw");
        customer.setHouseNumber("5");
        customer.setName("Jan");
        customer.setSurname("Abc");
        customer.setStreet("Xyz");

        /*User*/
        UserImpl user = new UserImpl("X001");
        user.setSurname("Smith");
        user.setName("John");

        /*Oc*/
        this.oc = new Oc("0266");
    }

    @After
    public void tearDown() throws Exception {

    }

    @Test
    public void testSetCustomer() throws Exception {
        oc.setCustomer(this.customer);
        Assert.assertEquals(this.customer, oc.getCustomer());
    }

    @Test
    public void testSetVehicle() throws Exception {
        oc.setVehicle(this.car);
        Assert.assertEquals(this.car, oc.getVehicle());
    }

    @Test
    public void testSetDiscounts() throws Exception {
        oc.setDiscounts(this.discounts);
        Assert.assertEquals(this.discounts, oc.getDiscounts());
    }

    @Test
    public void testSetInsurer() throws Exception {
        oc.setInsurer(this.user);
        Assert.assertEquals(this.user, oc.getInsurer());
    }

}

Whole source code

Are there any other unit tests I should perform on such entities' relations? I didn't come up with anything more that this to check for any errors, and simple checking setters and getters is useless. Any other comments regarding the design pattern, SOLID principles, unit testing or whatever will be appreciated.

Answer 1

After cloning your project and reviewing the blog post you mentioned I find that your structure is close. I can see that you tried copying what he did, but missed a few points: Your tests are in the same directory as your production code. This is frowned upon in all words of languages that I've been in (C# being my current, but Java and Android all apply). Looking at Android10's github link and I see that he correctly put his tests in a different directory (domain/src/main for production code, domain/src/test for his tests) than his production code. So that is my first item in your review: Move your tests out of your production code. I would be angry if I saw JUnit library floating around on my android device!

Short blurb about names. What is OC? I've read the source, I've read your tests, but can't figure out what OC stands for. Use a better name. Uncle Bob would have wagged his finger at you for that. :)

OcTest: I'm not sure if this is just the initial phase of your tests, but they are not very good. In general testing anything with a Cyclomatic Complexity (Wiki Page) of one is not needed nor is it helpful. The reason for that is is that it adds no value to your code. Doing this is akin to testing the framework and not the behavior of your system; It makes for brittle tests. I say brittle because properties change, structures change and if Oc is going to only be a model with getters and setters you don't want to have to change all your tests around it because of a small change like that. Look through Android10's tests and see how he does his tests. From what I've read so far his tests are good.

If you really really still wanted to have unit tests on your model, at least come up with a system that uses reflection to get all your getters and setters and exercises them. This way your model and change, but your tests won't.

I see that your code base is kind of big so there are other things you could open a new thread about. I'd be more than happy to review those as well if you put them up here.