This is yet another follow up question about Object oriented design in general. I am trying to break them down to separate questions per advice I got from @Jay earlier.
I get a new question, almost every step of the OO design process. I tried to get the ideas from books, articles and the web. It just gets more muddied up. I would like to get some advice from the experts here, so I can get my concepts right.
Say I have the below Objects:
public class AddressRecord {
protected String addressLine1;
protected String addressLine2;
protected String city;
protected String state;
protected String postalCode;
protected String country;
....
}
public class AddressValidationRequest {
private AddressRecord addresses[];
protected ArrayList actions;
protected ArrayList columns;
protected Properties options;
...
}
Should I keep AddressRecord completely hidden inside Request (is it Aggregation?) or expose and access the AddressRecord from outside (Composition)?
(To me, even inheritance seemed like an option to extend AddressRecord to a Request, though I feel they are distinct entities in real world, so I dropped that idea).
What I mean is, should I have AddressRecord as a private object inside Request and add getters/setters in the Request itself for each field in AddressRecord?
Or just add getter/setter to get/set AddressRecord[] from the Request and then set it up outside, say in a controller code?
(This later idea I got when I tried to extract the AddressRecord class from Request in Eclipse like below:
public class AddressSearchRequest {
AddressRecord address;
public AddressSearchRequest(String format, String customerId, String addressLine1, String suite,
String city, String state, String postalCode) {
this.format = format;
this.customerId = customerId;
this.address.setAddressLine1(addressLine1);
this.address.setAddressLine2(Suite);
this.address.setCity(city);
this.address.setState(state);
this.address.setPostalCode(postalcode);
}
public void setAddressLine1(String addressLine1) {
this.address.setAddressLine1(addressLine1);
}
public String getSuite() {
return address.getAddressLine2();
}
public void setSuite(String suite) {
this.address.setAddressLine2(suite);
}
public String getCity() {
return address.getCity();
}
public void setCity(String city) {
this.address.setCity(city);
}
public String getState() {
return address.getState();
}
public void setState(String state) {
this.address.setState(state);
}
public String getPostalCode() {
return address.getPostalCode();
}
public void setPostalCode(String postalCode) {
this.address.setPostalCode(postalCode);
}
}
UPDATE:
There are actually 2 different types of Requests I am dealing with - a Search Request - takes only one address and may return multiple matches.
A Validation Request may take one or more addresses.
I had them both as Request causing some confusion. corrected it.
Note: I am using AddressRecord also in other types of Request(s) and Response classes.
3 Answers 3
The difference between aggregation and composition is ownership. Composition is stronger. If Request
is composed of AddressRecord
s, then AddressRecord
cannot exist outside of a Request
. If Request
aggregates AddressRecord
s, then you can create AddressRecord
without the existence of a Request
.
I don't have a full understanding of your system, but it seems like an AddressRecord
can exist outside of your Request
. This would mean that the approach where you provide an accessor and mutator for your collection of AddressRecord
s. This could take many forms: you can provide a get/set by index, a get/set for the entire collection (in this case, the array), add/remove methods, and so on. Either way, I would recommend that you create AddressRecord
s outside of Request
and use them (along with other data) to populate the Request
.
-
Thanks for taking the time to explain in detail. I had the AddressRecord outside before, as this is typically the design I see; until I tried to extract some fields and Eclipse did Aggregation. FYI- I am working on a Web Service client to call a 3rd party web service to get our addresses validated and Geocoded. Our main application is in n-tier PowerBuilder running inside old Sybase EAServer. I have to use Java mostly for low level tasks to handle multi-tasking, multi-threads.svaratech– svaratech11/20/2015 23:37:53Commented Nov 20, 2015 at 23:37
-
@SamV What do you mean by "Eclipse did Aggregation"? You want aggregation, which is what I described. But I'm not sure how Eclipse does it.11/20/2015 23:41:42Commented Nov 20, 2015 at 23:41
-
I got your answer fine. But, I originally had all the fields in the request. Then I try to get "smart" with OO, started pulling out classes. At some point, I selected a bunch of fields inside the class, chose to Refactor -> Extract Class, intending a fully independent class. But, instead that created a new subclass outside (I guess Aggregated) with all the getter/setters for the subclass fields.svaratech– svaratech11/20/2015 23:47:15Commented Nov 20, 2015 at 23:47
-
@SamV Yes, there are configuration options and a few different ways. It sounds like you're new to Java - that was Eclipse doing what you told it to do, not necessarily suggesting the appropriate extraction. There should be a way to extract and move methods to other classes in different ways. I'd have to open Eclipse, though, and poke around.11/20/2015 23:49:39Commented Nov 20, 2015 at 23:49
-
thanks. Let me just, I finally got a chance to get back into Java after years. Catching up with many new tools, features, concepts. Still manage to get the tasks done so far. I am trying to get it done the right way this time :)svaratech– svaratech11/21/2015 00:12:10Commented Nov 21, 2015 at 0:12
There are two different cases: either your Request
contains one AddressRecord
, or it contains multiple ones. (The question is a bit confusing in this regard.)
If your Request
is to contain multiple AddressRecord
s, then the answer is clearly that it has to expose methods to obtain AddressRecord
s by index or by some other key, so that the caller obtains an AddressRecord
and works with it. The Request
might then also offer methods to add or delete AddressRecord
s, and specifically the add()
method might either accept an existing AddressRecord
to add to the Request
, in which case the Request
does not own the AddressRecords
that it contains, so you have aggregation, or it might create an AddressRecord
internally, in which case it owns its contents. The choice is yours.
If your Request
is to contain a single AddressRecord
, the same principles apply, you can think of it as being a collection of one item, so there is no need to add or delete items, and the accessor does not need an index or a key. But if I were in your shoes I would definitely use an accessor, so the caller would have to invoke the accessor to obtain a reference to the one and only AddressRecord
object contained within Rquest
, and then the caller would have to invoke the AddressRecord
object directly.
Adding mutator methods to Request
which delegate to a contained AddressRecord
is a lot of unnecessary work, it makes your classes hard to maintain, and it gives the false appearance that Request
is derived from AddressRecord
, since it exposes a superset of its methods, but without actually having inheritance, since you cannot pass a Request
there where an AddressRecord
is expected.
Note: another strategy commonly followed in more modern versions of java and in languages with more complete and more flexible collection models (like C#) would have Request
expose an actual List<AddressRecord> getAddressRecords()
, but with java 1.4 I would not try such a thing, due to the lack of type safety.
-
1I don't agree that if
Request
contains multipleAddressRecord
s, you need to get/set them by index or key. Why can't you have avoid setAddressRecords(AddressRecord[])
andAddressRecord[] getAddressRecords()
methods? You can replace the array with the appropriate collection (or, preferably, collection interface). Not that I'm advocating that approach, it could always be useful to have a batch operation to set/get all records at once if they are a collection.11/20/2015 23:47:55Commented Nov 20, 2015 at 23:47 -
@ThomasOwens well, one does not preclude the other. Anyway, you wrote your comment as I was amending my answer to address exactly that.Mike Nakis– Mike Nakis11/20/2015 23:50:16Commented Nov 20, 2015 at 23:50
-
Indeed. I think your edits addressed my concerns.11/20/2015 23:55:39Commented Nov 20, 2015 at 23:55
-
I do have void setAddressRecords(AddressRecord[]) and AddressRecord[] getAddressRecords(). The Request allows multiple addresses, but most of the time, I will be sending in 1 address record. So, is it proper to also to have setAddressRecord(AddressRecord) and getAddressRecord() to get/set 1 record and internally set it to AddressRecord[1] as special cases?svaratech– svaratech11/20/2015 23:57:48Commented Nov 20, 2015 at 23:57
-
@Mike, Thanks for the edits - helps a lot. I agree my original post is a bit confusing. I do have multiple addresses inside. Like I mentioned, as a special case, I deal with mostly one address. I was going back and forth. That's the reason for the 2nd part of the code (I copied it from a different pass of editing in Eclipse). I will try to correct it.svaratech– svaratech11/21/2015 00:02:47Commented Nov 21, 2015 at 0:02
The other answers go into some good details about different approaches to relating the classes to one another, which is central to what the question asked. But I would just reinforce the notion that the ultimate best choice of design approaches can and will naturally flow from an understanding of what the application does and what data is in it.
For example, are the Request records simply "log entries" whenever a request is made? Or are they API "messages" that need to be tracked and resolved? And what about the Validation Requests? And so on.
The most important advantage of object-oriented design isn't any programming power or functionality it might provide to reuse code. OOP's great advantage is that it helps conceptually clarify the code (with fewer comments) for developers who read it. If an OO design devolves into a muddle of obtuse relationships (which it quickly can)...then it has defeated its main purpose.