2

I have 2 POJO classes, DatabaseRecord and MyCustomRecord. In my service logic, I wanted to convert the object of type DatabaseRecord to object of type MyCustomRecord.

The code for converting should be as simple as below:

public MyCustomRecord toMyCustomRecord(DatabaseRecord databaseRecord) {
  MyCustomRecord customRecord = new MyCustomRecord();
  customRecord.setA(databaseRecord.getA());
  customRecord.setB(databaseRecord.getB());
  ...
  ...
  return customRecord;
}

Where should this method should live? These are possible places I see:

  1. DatabaseRecord POJO class
  2. MyCustomRecord POJO class
  3. The class which is doing this transformation. Can be a private class.
  4. Separate class using pattern like Adapter.

What are the reasons to choose/not to choose these locations for doing this transformation?

2
  • My preference on these cases is to make it a constructor in the relevant class. e.g. MyCustomRecord has a constructor taking a DatabaseRecord as an argument. Or use a static factory if you prefer.
    – user949300
    Nov 18, 2020 at 6:25
  • @user949300 You can also add a method on DatabaseRecord called toMyCustomRecord which is simply: return new toMyCustomRecord(this);
    – JimmyJames
    Nov 18, 2020 at 17:35

4 Answers 4

2

Whats wrong with creating a constructor in the respected class? or using a parsing decorator/object?

I.E.

public final class MyCustomRecordFromDB implements CustomRecordInterface {
    private final DatabaseRecord innerDBRecord;
    public MyCustomRecordFromDB(DatabaseRecord dbRecord) {
        this.innerDBRecord = dbRecord;
    }

    public Something A()
    {
        return ... ;
    }

    public Something B()
    {
        return ... ;
    }
}

Where CustomRecordInterface is the interface that MyCustomRecord implements (i.e. MyCustomRecordFromDB should be able to be a substitute to MyCustomRecord) as you should always code to interfaces in OOP (in case you want to follow OOP).

it is really similar to adapter object pattern.

Note: I've assumed that your setA(...) and setB(...) are public methods that have getter such as A() and B() respectfully, you can omit that if this isn't the case.

2

Few things I would consider when using a class in another. Constructor or No constructor.

  1. Coupling: I would avoid tight dependency between classes when not required or can be avoided.
  2. Encapsulation: If you change something, maybe a field in one class, you would end up changing the other class as well. This is a side effect. Desirable?
  3. The conversion might be simple for now - but in case you want to add logic/validations later, is constructor the right place for that?
  4. Single Responsibility Principle: Let your POJO do what it is meant to do. Why have another reason for the POJO to change?
  5. Testing: No matter how trivial the code is, you might want to make sure you can unit test it. Testing static methods, as others have mentioned, is difficult.

Based on these, I think it is better for you to follow the standard adapter pattern, create an adapter interface, and have a separate class provide its implementation.

Your POJOs will be simple, logic moved out, the coupling is reduced, can be tested easily.

Further, for your use case, you can also try using some object mappers that are readily available. Can avoid a lot of boilerplate code.

1
public class MyCustomRecord 
{
    private TypeA A; 
    private TypeB B;
    MyCustomRecord(TypeA A, TypeB B)
    {
        this.A = A;
        this.B = B;
    }
    public static MyCustomRecord toMyCustomRecord(DatabaseRecord databaseRecord) 
    {
        MyCustomRecord customRecord = new MyCustomRecord(
              databaseRecord.getA(), databaseRecord.getB());
    }
}
0

Frankly, this is pure logic. It can easily be a static method attached to anything. The only criteria for where it should go is entirely dependent on what you find more readable.

That being said, there is overhead and hypothetical downsides to some of the approaches.

  1. DatabaseRecord Class

If you were to make it an instance method on the DatabaseRecord class, the only downside is that maybe you want the DatabaseRecord "layer" to not have any code related to a "higher layer".

public final class DatabaseRecord {
    public MyCustomRecord toMyCustomRecord() {
        return ...;
    }
}
  1. Same hypothetical issue as (1), but thats always going to be a "hypothetical issue" no matter where you put it because every layer might not want to know about other ones. In my opinion this is the place where it makes the most sense, but I have nothing but feelings to back that up.
public final class MyCustomRecord {
    public static MyCustomRecord fromDatabaseRecord(DatabaseRecord databaseRecord) {

    }
}

You could also just call the method from if you feel like that doesn't lose an important amount of readability.

  1. Class which is doing this transformation, maybe private.

No issue placing a small helper in whatever class you feel like, but as boring as it is - this is code that is easy to test and could potentially have stupid bugs in it, so it is important that it is public enough that you can test the method. That is, if you do TDD.

  1. Adapter class

I mean no reason why not, but it has dubious value. Lets say you created your adapter class.

public final class DatabaseRecordAdapter {
    public MyCustomRecord adapt(DatabaseRecord) {

    }
}

One, this doesn't really add much other than indirection on just calling a static method or an associated instance method. Two, you need to make instances every the time unless you make it a singleton. Making instances is cheap, but it isn't totally free. So to make it a singleton you would.

public enum DatabaseRecordAdapter {
    INSTANCE;
    public MyCustomRecord adapt(DatabaseRecord) {

    }
}

and prior to java 8, this might have had some concrete uses, as a custom adapter class can implement interfaces and be used in a generic way.

public enum DatabaseRecordAdapter implements Adapter<DatabaseRecord, MyCustomRecord> {
    INSTANCE;
    public MyCustomRecord adapt(DatabaseRecord) {

    }
}

And you could give it to anything expecting an Adapter by passing DatabaseRecordAdapter.INSTANCE. Nowadays this isn't a real benefit since you can use method references to pass the static method in case 2 like MyCustomRecord:::fromDatabaseRecord. That being said, its not a bad approach - just make sure it makes sense in the context of your program and isn't just pure overhead.

3
  • 1
    Using statics for anything at all is an antipattern and should be avoided. It is really hard to test static things or predict their behaviour in a multi-threaded or multi-classloader system. Java is always multithreaded, and multiple classloaders occur in the majority of IoC frameworks. Static is not safe, not even in the enum approach shown here. Nov 18, 2020 at 8:07
  • 1
    @SoftwareEngineer: If static methods are pure, they are thread safe, and their behavior is easy to predict. They are definitely not an anti-pattern. Besides, constructors are kind of static method as well. Nov 18, 2020 at 14:32
  • 1
    @SoftwareEngineer There is a nugget of truth in the "you can't mock static methods" claim , but when code doesn't perform a side effect that isn't an issue at all because if you were to mock that, you wouldn't actually be testing your code. Static methods are just fine. Nov 18, 2020 at 21:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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