-2
\$\begingroup\$

I created this code

@Service
public class TestService {

@Autowired
private Test1Repository test1Repository;

@Autowired
private Test2Repository test2Repository;

@Autowired
private Test1Mapper test1Mapper;

@Autowired
private Test2Mapper test2Mapper;

public void test(Map<String, Object> body){
    if("test1".equals(body.get("type"))){
        Test1 test1 = test1Mapper.mapToTest1(body);
        // logic here
        test1Repository.save(test1);
    } else if("test2".equals(body.get("type"))){
        Test2 test2 = test2Mapper.mapToTest2(body);
        //logic here
        test2Repository.save(test2);
    }
}

}

I think it's not so good to create if statement for each type of request because in the future I need to add more types. I try to use enum

TestType.valueOf((String) body.get("type")).saveInDB();

But in the enum, I can not inject repositories and mappers. I searching for some way to solve this problem but I can not find it. Does anyone have a source with an explanation to solve the problem with if statements or some can explain how to solve it?

Thank you in advance.

\$\endgroup\$

closed as off-topic by 200_success, yuri, Sᴀᴍ Onᴇᴌᴀ, IEatBagels, Toby Speight Apr 29 at 15:26

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – 200_success, yuri, Sᴀᴍ Onᴇᴌᴀ, IEatBagels, Toby Speight
If this question can be reworded to fit the rules in the help center, please edit the question.

1
\$\begingroup\$
  • Your repositories should have a common method save() and your mappers a mapToTest() that you can define as your interface or base class. Also your test-objects should have a common interface or base class. Let's assume that you can do that. If you cannot do that, then your code inside the if-statements is too different and you could only solve that by a map with key=TestType and value=lambda-expression (or in other words, value = object of abstract classes derived from Runnable with the code inside each run-method.).
  • Do you really need so much new classes and instances for each new if-statement? That means for a third if-statement you would need to write new classes Test3, Test3Mapper, Tests3Repository and need to insert additional member variables into your main method for it? I fear your code quickly becomes unmaintainable. Because of all these definitions, my code example below can not be done simpler anymore. Try to write only a few classes instead and define a factory which returns different instances of it. The code example below would be much shorter if you for example would have an array of TestRepositories (or their factory names) instead of single variables test1Repository, test2Repository etc.
  • You wrote that you cannot inject repositories and mappers in your enum. But you can inject them in your main class and pass them to the enum instances.
  • Your enum name is "TEST1", but in your map "body" you have stored "test1". So you must somehow map this label to the enum. For that, you need an internal static hash map inside the enum. You will not need it if you store ""TEST1" inside your map instead. But let's assume both are independent and cannot be converted via toUpperCase() etc., so we need to define the method valueOfLabel() in the enum and pass this label in the constructor.

Here is the enum:

private enum TestType {
    TEST1 ("test1", test1Repository, test1Mapper),
    TEST2 ("test2", test2Repository, test2Mapper);

    private String label;
    private TestRepository testRepository;
    private TestMapper testMapper;

    private static final Map<String, TestType> labelMap = new HashMap<>();
    static {
        for (TestType testType: values()) {
            labelMap.put(testType.label, testType);
        }
    }
    public static TestType valueOfLabel(String label) {
        return labelMap.get(label);
    }

    TestType(String label, TestRepository testRepository, TestMapper testMapper) {
        this.label = label;
        this.testRepository = testRepository;
        this.testMapper = testMapper;
    }

    public void saveInDB(Map<String, Object> body) {
        Test test = testMapper.mapToTest(body);
        // TODO logic here
        System.out.println("test=" + test.getClass().getSimpleName() + ", this=" + this);
        testRepository.save(test);
    }

    public String toString() {
        return "TestType [label=" + label + ", testRepository=" + testRepository.getClass().getSimpleName() + ", testMapper=" + testMapper.getClass().getSimpleName() + "]";
    }
}

Here is the replacement for your if-statements and testing this method:

private void test(Map<String, Object> body) {
    TestType.valueOfLabel((String) body.get("type")).saveInDB(body);
}

public static void main(String[] args) {
    Map<String, Object> body = new HashMap<>();
    body.put("type", "test1");
    new Main().test(body);
    body.put("type", "test2");
    new Main().test(body);
}

Here is the test console output:

test=Test1, this=TestType [label=test1, testRepository=Test1Repository, testMapper=Test1Mapper]
test=Test2, this=TestType [label=test2, testRepository=Test2Repository, testMapper=Test2Mapper]

Here is the full program with all the clases needed to run it. Not everybody has Spring or has it set up, so to simplify the solution that everybody can run, I just created the needed classes. Feel free to change the code to autowire them instead.

package main;

import java.util.HashMap;
import java.util.Map;

public class Main
{
    // tests
    static class Test {
    }
    static class Test1 extends Test {
    }
    static class Test2 extends Test {
    }

    // repositories
    static class TestRepository {
        public void save(Test test)  {
            // TODO save it
        }
    }
    static class Test1Repository extends TestRepository {
    }
    static class Test2Repository extends TestRepository {
    }

    // mappers
    static class TestMapper {
       private Test test;
       public TestMapper(Test test) {
             this.test = test;
        }
        public Test mapToTest(Object body) {
            // TODO get some data out of body and call test with it
            return test;
        }
    }
    static class Test1Mapper extends TestMapper {
        public Test1Mapper(Test test) {
            super(test);
        }
    }
    static class Test2Mapper extends TestMapper {
        public Test2Mapper(Test test) {
            super(test);
        }
    }

    // autowired
    private static Test1Repository test1Repository = new Test1Repository();
    private static Test2Repository test2Repository = new Test2Repository();
    private static Test1Mapper test1Mapper = new Test1Mapper(new Test1());
    private static Test2Mapper test2Mapper = new Test2Mapper(new Test2());

    // enum
    private enum TestType {
        TEST1 ("test1", test1Repository, test1Mapper),
        TEST2 ("test2", test2Repository, test2Mapper);

        private String label;
        private TestRepository testRepository;
        private TestMapper testMapper;

        private static final Map<String, TestType> labelMap = new HashMap<>();
        static {
            for (TestType testType: values()) {
                labelMap.put(testType.label, testType);
            }
        }
        public static TestType valueOfLabel(String label) {
            return labelMap.get(label);
        }

        TestType(String label, TestRepository testRepository, TestMapper testMapper) {
            this.label = label;
            this.testRepository = testRepository;
            this.testMapper = testMapper;
        }

        public void saveInDB(Map<String, Object> body) {
            Test test = testMapper.mapToTest(body);
            // TODO logic here
            System.out.println("test=" + test.getClass().getSimpleName() + ", this=" + this);
            testRepository.save(test);
        }

        public String toString() {
            return "TestType [label=" + label + ", testRepository=" + testRepository.getClass().getSimpleName() + ", testMapper=" + testMapper.getClass().getSimpleName() + "]";
        }
    }

    // test
    public static void main(String[] args) {
        Map<String, Object> body = new HashMap<>();
        body.put("type", "test1");
        new Main().test(body);
        body.put("type", "test2");
        new Main().test(body);
    } 

    // method under test    
    private void test(Map<String, Object> body) {
        TestType.valueOfLabel((String) body.get("type")).saveInDB(body);
    }
}
\$\endgroup\$

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