In my constructor, is it bad practice to set class variables by calling getters on an object I pass-CodePudding

So imagine I have a simple class with three variables which are ultimately derived from another object. I could insert that other object in the constructor like:

public class A {

    private int x;
    private int y;
    private List<> list;

    public A(B b) {
        this.x = b.getX();
        this.y = b.getY();
        this.list = b.getList();
    }
}

public class B {

    private int x;
    private int y;
    private int z;
    private String string;
    private Set<> set;
    private List<> list;

//constructor, setters and getters
}

A colleague has suggested I should instead do the following:

public class A {
  
    private B b;

    public A(B b) {
        this.b=b
    }

}

Now in the application using their option will require a lot of refactoring from

a.getX() to a.getB().getX() or a.getList() to a.getB().getList() for example.

What I'm struggling to find is what is there some best practice defined in this scenario and why.

My argument is I don't want objects that use A to be coupled to any class structure imposed by B. I think my colleague is concerned that my constructor should not be accepting B as an argument only to call getters on it's variables within the constructor of A.

An alternative would be to use a factory or something to create A from B but that isn't really feasible with my use case. (In my specific use case A is being constructed in a HQL query and B is an Entity class)

EDIT: I typed this question out on my phone but realise I missed some details so will add them here.

A consists of a subset of properties from B. A also consists of a subset of properties of another class C. The properties of C are currently individually assigned in the constructor in what you would consider a normal manner (because there are only a few). Because A takes a lot of properties from B I am inserting it in the manner illustrated above.

B and C are Entity classes that represent a row from two separate (but related) tables.

A is a class which is we use to do some business logic with somewhere else.

CodePudding user response：

Maybe this is what you are looking for. A is a more general type and B is a more specialize type of A. For example, a Person class and a Student class.

public class A {
    private int x;
    private int y;
    private List<> list;

    public A(int x, int y, List<> list) {
        this.x = x;
        this.y = y;
        this.list = b.getList();
    }
}

public class B extends A {
    private int z;
    private String string;
    private Set<> set;
    
    public B(int x, int y, List<> list, String string, Set<> set){
       super(x,y,list);
       this.string = string;
       this.set = set;
    }
}

CodePudding user response：

With all the information that you have given, your colleague's suggestion of having B object as a class field of A is a bad design. As well as passing B on the A constructor.

The main problem here is on the A class constructor, not it's field. Passing B object directly to A constructor is a bad design/practice even if A takes a lot of properties from B. If someone were to construct an A object that takes B as an argument, most of the time they would assume that A uses B or that they have a strong relationship. If your concern here is avoiding a lengthy constructor, then you're better off using A setters to take the B properties as shown below:

   A aClass = new A();
   aclass.setX(bClass.getX());
   aclass.setY(bClass.getY());

Another way is creating a method in class A that takes required properties from B by taking B as an argument like:

  public class A {
      \\fields
      public void setRequiredPropertiesFromBClass(B bClass) {
          this.x = bClass.getX();
          \\\other getters
     }
  }

That way the intent of your code is clear, which is A needs certain properties from B not B itself. The example above can still be improved. I couldn't think of much better method name since I don't really know the complete implementations of these classes. Lastly, you mentioned that B and C are related so you can turn the method into a generic one so it can take B and C.

If all of these suggestions are still not helpful. Just search for Transfer Object Pattern on the internet.