Thread: Caller passing itself to child object?

I ran into a code pattern today that I know isn't right and I know what to do about it. What I don't know is the best way to document for the original developer just why it needs to be changed. Obviously he thought it was a good design when he did it so I want to be prepared to respond with better than, "because I said" or "just because" when I discuss it.

I have two classes, for example, ClassA and ClassB. ClassA has a ClassB. ClassA passes a reference to itself into ClassB and calls the configure() method in its ClassB instance.

public class ClassA {
ClassB instance = new ClassB(this);
private String field1;
private String field2;
private String field3;
 
public void main(String[] args) {
instance.configure(classAData);
}
 
public void setField1(String value) {
field1 = value;
}
 
public void setField2(String value) {
field2 = value;
}
 
public void setField2(String value) {
field2 = value;
}
 
}
 
public class ClassB {
public ClassB(ClassA classA) {
this.classA = classA; 
}
 
ClassA classA;
 
public void configure(String[] classAData) {
classA.setField1(classAData[0]);
classA.setField2(classAData[1]);
classA.setField3(classAData[2]);
}
}

I see no reason to even have ClassB. In my view, ClassB knowing about its caller, ClassA, is definitely a bad smell in code. Since ClassB.configure only operates on ClassA objects, it really wants to be in ClassA.

One option is to move Field1, Field2, and Field3 into ClassB and let it be a data holder object for ClassA, if that fits the model. Another option is to eliminate ClassB all together. This is going to be my initial preference since the fields are used extensively in ClassA. As I refactor more I may find that I want a data object for ClassA but I don't see the point yet. In my real code, ClassA is a pretty extensive bit of code strewn with spaghetti and I'm trying to clean it up so I can make what is, as it is, a very difficult addition to the code. I am trying to refactor it to make it more object-oriented so that my change can be done in 30 minutes instead of a couple days - and then future changes will also be minutes instead of days.

What rules am I forgetting or do I need to research about children knowing about parent? It's clearly not loosely-coupled but I'd like to find documented theories or practices around this if I can. Any suggestions?

Thanks,

Dale

Is this the actual code? There are many many problems..surely this code is not in use

Assuming some of the problems are no more than typos/oversights in a quick example rather than the actual code in question, this type of design provides a layer of security between ClassA's members and ClassB's access to them. In the sample given ClassB can not get the values of the fields, just set them. Not that such a thing is necessarily useful, that would depend on the actual code and what is being done.
Would need more info/clarification on the specifics, but it looks like it could be cleaned up or minimized to one class with what is given so far

Thanks for the reply.

THis is just sample code; I would have to jump through a ton of hoops to get permission to post actual code. The issue here is not so much the question of security to the fields; or not a question of using the getter or setter.

Once ClassB sets the values in ClassA, ClassB is never referred to again in code. It appears to be a very-well-intentioned attempt at keeping classes small and focused where someone extracted some small set of behavior into its own class. I just don't think this gives the results that might be intended (I will find out more about the intentions in a later discussion with the developer).

I'm a big fan of extracting data classes and in ClassA there are long lists of related fields with which I can do that. There are also objects I will extract with inheritance to eliminate a bunch of case or if-else statements. Overall, I see a bunch of good intention in the code but perhaps falling short in execution.

So this code should demonstrate the principle, I hope. The question is about the owned object knowing details about its owner. It certainly means it can only have one owner type, ClassA, or owners that also own or know about ClassA objects to pass in to ClassB.

I hope this helps clear up what I'm asking about.

Thanks,

Dale

It sounds like you already have it figured out but are not confident with your findings.
If there is no reason to have ClassB, then that is the answer. Nothing seems clearer except that you have a solution you are hesitating with.
I know this post is not much more help than the last one, but there is not much to go on, except your word that ClassB is not needed, so get rid of it and see how it goes
I know its hard to ask questions without getting thrown into the fire on something like this, good luck with it.

Thanks. I'm pretty confident in my finding; I was just looking for documentation to support it.

I don't mind being thrown into the fire; I'm a big boy . I know it's hard to answer without clear code sometimes.

Thanks for the help.

Dale