1
\$\begingroup\$

To give a little context before showing the problem:

I am creating a RESTful web application. A User can have multiple addresses, emails, phones and projects. Each of these is represented similar to the following object model, (Address in this case).

public class Address {
    String type; //home or work etc.
    String streetname;
    /*some more things*/

    Privacy privacy; //Privacy object
}

Every model contains a privacy object since a user can decide which of his addresses may be set to private (for instance only show the address to his contacts). The privacy object contains the following:

public class Privacy {
    @Id
    private String id;
    private boolean showForNonContacts;
    private boolean onlyShowForGroups;

    /*
    getters and setters
    */
}

The problem:

In my controller, I am fetching the userprofile object which contains these models. For every model, I have to check the following:

List<Address> visibleAddresses = new ArrayList<Address>();
for(Address address : userProfile.getAddresses()) {
    if(address.getPrivacy().isShowForNonContacts() || hasContact) {
        visibleAddresses.add(address);
    } 
    userProfile.setAddresses(visibleAddresses);
}

//Same code repeats but now for email/phone/projects

I have to do this for the emails, phones and projects. This clearly is a bad design and I would like to make it better and cleaner than repeating the above code for every model. Does anybody have any tip of how I can make the above code work? I thought of using generics, interfaces where this method would be some what common, but I really don't know since I'd like to keep the model class clean.

Any tips/advice is greatly appreciated.

The UserProfile class should still contain a List for instance, since my db mapper (MongoDB/Spring data) has to recognize it as an embedded object.

EDIT: Based on the answer of Greg Burghardt, I decided to create a generic class called GenericCollection. Each of the models implements a separate interface. GenericCollection extends this interface and makes use of the method which controls the privacy (as defined in the interface).

Now the code is a lot cleaner.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

How about creating an AddressCollection class that holds all the logic of determining the "visible" addresses:

public class AddressCollection
{
    private ArrayList<Address> items;

    public AddressCollection()
    {
        items = new ArrayList<Address>();
    }

    public AddressCollection(ArrayList<Address> addresses) {
        items = addresses;
    }

    public void add(Address address) {
        items.add(address);
    }

    public ArrayList<Address> getVisibleAddresses() {
        ArrayList<Address> visibleAddresses = new ArrayList<Address>();

        for (Address addr : items) {
            if (addr.getPrivacy().isShowForNonContacts()) {
                visibleAddresses.add(addr);
            }
        }

        return visibleAddresses;
    }

    public ArrayList<Address> getItems() {
        return items;
    }
}

Each model that needs this functionality is reduced to a little boiler plate code, but at least you aren't repeating the basic domain logic:

public class UserProfile
{
    private AddressCollection addresses;

    public UserProfile() {
        addresses = new AddressCollection();
    }

    public void setAddresses(ArrayList<Address> addresses) {
        this.addresses = new AddressCollection(addresses);
    }

    public ArrayList<Address> getAddresses() {
        return addresses.getItems();
    }

    public ArrayList<Address> getVisibleAddresses() {
        return addresses.getVisibleAddresses();
    }
}

public class SomethingElse
{
    private AddressCollection addresses;

    public UserProfile() {
        addresses = new AddressCollection();
    }

    public void setAddresses(ArrayList<Address> addresses) {
        this.addresses = new AddressCollection(addresses);
    }

    public ArrayList<Address> getAddresses() {
        return addresses.getItems();
    }

    public ArrayList<Address> getVisibleAddresses() {
        return addresses.getVisibleAddresses();
    }
}
\$\endgroup\$
4
  • \$\begingroup\$ Thank you for your comment! I think the problem with this approach is that I have to have List<Address> inside the UserProfile class, since my database (or more specifically db abstraction) which is MongoDB does otherwise not see it as an embedded collection. I'm very sorry for not mentioning that. However, I am still seeing whether I can fit your code. \$\endgroup\$
    – Moody
    May 4, 2015 at 19:53
  • \$\begingroup\$ Actually, based on your idea, I might have something! I'll get back asap! \$\endgroup\$
    – Moody
    May 4, 2015 at 20:15
  • \$\begingroup\$ Based on your idea, I managed to do it differently (see update above). But you gave me the idea, so therefore, thank you & also thanks for your time! +1 \$\endgroup\$
    – Moody
    May 4, 2015 at 21:42
  • \$\begingroup\$ Efficient and clean! +1 \$\endgroup\$ May 5, 2015 at 13:13

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.