2

I have the following classes, which have quite similar method implementations. Only the classes' method inputs and outputs seem to be of different types. When I put it like this, it sounds like a case for inheritance, however, the fact that the inputs and outputs are different and are related to two lambdas, make me wonder if they should remain without any relationship, as one lambda cannot be thought of in place of another (To be a case for inheritance).

My first class looks like the following.

public class JobPersistenceManager {
    private String jobIndexName;
    private JobLambda JobLambda;
    private MyDataPersistence myDataPersistence;
    private DataProcessorUtils dataProcessorUtils;
    private static final String JOB_ID = "jobId";
    private static final String JOB_NAME = "jobName";

    @Inject
    public JobPersistenceManager(@Named("jobIndex") String jobIndexName,
                                 JobLambda JobLambda,
                                 MyDataPersistence myDataPersistence) {
        this.jobIndexName = jobIndexName;
        this.JobLambda = JobLambda;
        this.myDataPersistence = myDataPersistence;
        createIndexIfNotExists(this.jobIndexName);
    }

    public SearchDocumentResult searchJob(MyJobInput myJobInput) throws IOException {
        return myDataPersistence
                .searchDocument(this.jobIndexName,
                        dataProcessorUtils.transformObjectDataPayloadToMap(myJobInput));
    }

    public MyJobOutput invokeCreateJobLambdaAndIndexData(final MyJobInput myJobInput)
            throws IOException {
        String personRequestPayload = dataProcessorUtils.transformObjectDataInputJson(myJobInput);
        Map<String, String> createdJobOutput = this.JobLambda.invokeLambda(personRequestPayload);
        this.indexCreatedJob(myJobInput, createdPersonOutput);
        return MyJobOutput.builder().withJobID(createdJobOutput.get(JOB_ID))
                .withJobName(createdJobOutput.get(JOB_NAME)).build();
    }

    public int indexCreatedJob(final MyJobInput myJobInput,
                               final Map<String, String> createdJobOutput) throws IOException {
        myJobInput = modifyJobInput(myJobInput);
        String documentToIndex = dataProcessorUtils.transformObjectDataInputJson(myJobInput);
        return myDataPersistence.indexDocument(this.jobIndexName, documentToIndex);
    }

    private void createIndexIfNotExists(final String indexName) {
        if (!myDataPersistence.doesIndexExist(indexName)) {
            myDataPersistence.createIndex(CreateIndexInput.builder().indexName(indexName).build());
        }
    }
}

My second class looks like the following.

public class EmployeePersistenceManager {
    private EmployeeLambda employeeLambda;
    private MyTestDataPersistence myTestDataPersistence;
    private DataProcessorUtils dataProcessorUtils;
    private String employeeIndexName;
    private static final String PERSON_ID_KEY = "personId";
    private static final String PERSON_NAME_KEY = "personName";

    @Inject
    public EmployeePersistenceManager(@Named("employeeIndex") String employeeIndexName,
                                    EmployeeLambda employeeLambda,
                                    MyTestDataPersistence myTestDataPersistence,
                                    DataProcessorUtils dataProcessorUtils) {
        this.employeeIndexName = employeeIndexName;
        this.employeeLambda = employeeLambda;
        this.myTestDataPersistence = myTestDataPersistence;
        this.dataProcessorUtils = dataProcessorUtils;
        createIndexIfNotExists(employeeIndexName);
    }

    public SearchDocumentResult searchPerson(EmployeeInput employeeInput) throws IOException {
        return myTestDataPersistence
                .searchDocument(employeeIndexName,
                        dataProcessorUtils.transformObjectDataPayloadToMap(employeeInput));
    }

    public EmployeeOutput invokeCreatePersonLambdaAndIndexData(final EmployeeInput employeeInput)
            throws IOException {
        String personRequestPayload = dataProcessorUtils.transformObjectDataInputJson(employeeInput);
        Map<String, String> createdPersonOutput = this.employeeLambda.invokeLambda(personRequestPayload);
        this.indexCreatedEmployee(employeeInput, createdPersonOutput);
        return EmployeeOutput.builder().withPersonId(createdPersonOutput.get(PERSON_ID_KEY))
                .withPersonName(createdPersonOutput.get(PERSON_NAME_KEY)).build();
    }

    public int indexCreatedEmployee(final EmployeeInput employeeInput,
                                  final Map<String, String> createdPersonOutput) throws IOException {
        employeeInput = modifyEmployeeInput(employeeInput);
        String documentToIndex = dataProcessorUtils.transformObjectDataInputJson(employeeInput);
        return myTestDataPersistence.indexDocument(this.employeeIndexName, documentToIndex);
    }

    public Map.Entry<String, Map<String, String>> invokeLambda(final String payload) {
        return new AbstractMap.SimpleEntry<>(payload, this.employeeLambda.invokeLambda(payload));
    }

    private void createIndexIfNotExists(final String indexName) {
        if (!myTestDataPersistence.doesIndexExist(indexName)) {
            myTestDataPersistence.createIndex(CreateIndexInput.builder().indexName(indexName).build());
        }
    }
}

As you can see, the methods perform almost the same actions. Only the indexCreatedEmployee and indexCreatedJob methods from the classes have an extra step of processing the input.

Should I keep these classes as they are now without any relationships between them, or should I create an abstract persistence manager class and perform the following.

  1. Move createIndexIfNotExists to the abstract class
  2. Create abstract methods search(), invokeLambda() and indexCreatedData() methods and implement them in each child class. The data types MyJobInput and MyEmployeeInput are POJO classes that don't have any relationship. So I guess these methods I mentioned would then take "Object" parameters?

EmployeeLambda and JobLambda are again classes with no relationship between them. Another concern I had towards creating some sort of inheritance was that, Employee Lambda and JobLambda cannot be used inter-changeably. So was wondering if they should inherit the same parent class just because they're both lambda classes.

OR is there another way to go about this? Any advice would be much appreciated. Thank you very much in advance.

8
  • If both managers are truly the same, behave exactly the same way regardless of the input and the output. Why don't you just use generics? – Laiv May 31 at 19:48
  • Thank you for the response @Laiv I was actually thinking about Generics too, but then if I use Generics, I would have an abstract class with abstract methods holding the generic data types, and then implementations for both the managers would still be the same lines right? Or am I getting this wrong? Would appreciate if you could guide me with an example on how you picture it – AnOldSoul May 31 at 20:00
  • If all Managers behave always the same way regardless of the parameters, then there will be only one Manager, the same way you only have only 1 HashMap<K, V> and it always behaves the same way no matter the type of the key or the value. Note that your Managers (both), XXXInput and XXXOutput do nothing! They could have been anything, so you could easily implement a single Manager<I,O> parametrized. If I were you, I would even drop generics to find a decent abstraction for Input and Output. Inheritance won't make you any favour here. – Laiv May 31 at 20:19
  • hmm so in the case of using a Manager<I,O>, how would I pass the index name that is unique for both the managers? Perhaps with a setter method? Because currently the constructor values are being injected through Guice – AnOldSoul May 31 at 20:34
  • 1
    Is there a need for common ancestry? Abstraction for abstraction's sake isn't really necessary. What is it you're ultimately trying to accomplish by creating a shared ancestry (whether inheritance, interface, composition, ...) for these two similar-looking methods? Note that I'm not saying there isn't a need for it, I'm genuinely asking whether there is something you'd like to be able to do which you're not able to do in the current version of the code. – Flater Jun 1 at 8:27
3

As you say, both classes do the same, abstractly speaking. Perhaps it would be worth trying this:

  • Forget about inheritance here, it would add unnecesary complexity in this specific case. The behavior is almost the same, and even if it were behavior that changed, it's better to do it by injecting it (composition)
  • Extract interfaces from both classes (IDE will help)
  • Compare both interfaces
  • Merge both interfaces by generalizing names like, for example changing EmployeePersistenceManager to PersistenceManager, so you end up with only one interface implemented by the two classes
  • Generalize the parameters via Generics
  • Extract some interfaces from the parameter objects as well or use one common interface they already implement which I'm almost sure they already do.
  • With some minor changes (like renaming JOB_ID to just ID ) you can keep just one of the two classes and deprecate the other.

Furthermore, I bet you will end up using the resulting generic class for "managing the persistence" of things other than Employees and Jobs.

1
  • Thank you very much for the detailed explanation! I took the above approach of creating a generics class and generalizing the logic into a PersistenceManager – AnOldSoul Jun 1 at 21:45

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.