1

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.

1

The difference between aggregation and composition is ownership. Composition is stronger. If Request is composed of AddressRecords, then AddressRecord cannot exist outside of a Request. If Request aggregates AddressRecords, 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 AddressRecords. 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 AddressRecords 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. – SamV Nov 20 '15 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. – Thomas Owens Nov 20 '15 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. – SamV Nov 20 '15 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. – Thomas Owens Nov 20 '15 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 :) – SamV Nov 21 '15 at 0:12
1

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 AddressRecords, then the answer is clearly that it has to expose methods to obtain AddressRecords 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 AddressRecords, 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.

  • 1
    I don't agree that if Request contains multiple AddressRecords, you need to get/set them by index or key. Why can't you have a void setAddressRecords(AddressRecord[]) and AddressRecord[] 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. – Thomas Owens Nov 20 '15 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 Nov 20 '15 at 23:50
  • Indeed. I think your edits addressed my concerns. – Thomas Owens Nov 20 '15 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? – SamV Nov 20 '15 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. – SamV Nov 21 '15 at 0:02
1

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.

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.