CODEWITHSUNDEEP
oopencapsulation
user1719287
user1719287

Reputation: 61

OO Design Encapsulation

I have a question with regard to encapsulation. As I know, encapsulation enables to hide the implementation details using private/protected data members and provides public methods and properties to operate on the data. The idea here is to prevent the direct modification of the data members by the class consumers.

But I have a concern with the property getters or other public methods which return private/protected data members. For ex: if I have class like this

public class Inventory
{
    private List<Guitar> guitars = new List<Guitar>();

    public void AddGuitar(string serialnumber, string price)
    {
        Guitar guitar = new Guitar(serialnumber, price);
        guitars.Add(guitar);
    }

    public List<Guitar> GetGuitars()
    {
        return guitars;
    } 
}

Now if the Inventory class consumer calls GetGuitars, he is going to get the list of guitars being maintained in the Inventory class. Now the consumer can modify the list, like delete/add/modify the items. For me it looks like we are not encapsulating. I think that I should be returning a copy of the Guitar list items in the GetGuitars(). What do you think?.

Is my understanding of the encapsulation right?.

Thanks

Upvotes: 5

Views: 231

Answers (6)

Michael Burrows
Michael Burrows

Reputation: 331

There are (at least) two issues here.

The first is about hiding the implementation. You could change the "guitars" field to an array or a database but you could leave the signature of the methods AddGuitar and getGuitars unchanged so client code wouldn't break.

The second is about whether or not you want to return a defensive copy of the guitar list or not. Once you have the list of guitars do you want to add and delete elements? Since you have a method to add guitars I would assume not.

Upvotes: 0

Chirlo
Chirlo

Reputation: 6130

In terms of risk, it is indeed better if you return a copy of your list of make it unmodifiable (create a whole new unmodifiable list when you add a guitar, functional programming-style).

In terms of encapsulation, it would be better to get rid of the getGuitars() method and then Inventory class should offer the functionality associated with it ( for example, printInventoryReport() or whatever). This way, no client class needs to know at all how you store your guitars and you keep the related code into the Inventory class. The tradeoff is that this class gets bigger and every time you need something new from the guitar list you need to modify the Inventory.

I recommend a good article : http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html It was quite incendiary back in the day, but i think there's a lot of truth in there.

And if you stay with the getter, a small tip would be to choose if you need it to be a List or a Collection can do. Maybe even an Iterable! This way you tell as less as possible about your implementation, which results in better encapsulation.

Upvotes: 1

user983652
user983652

Reputation:

if u want your List array cannot be modified, why u dont use AsReadOnly method: http://msdn.microsoft.com/en-us/library/e78dcd75.aspx

about encapsulation inside members are only writable and readable through the methods where members are not available from outside.

Upvotes: 1

Adam Dyga
Adam Dyga

Reputation: 8926

You are right. With a setter like that clients are able to modify the list. If adding a guitar requires some special handling, this is not desired. In this case you have two choices:

  1. Return a copy of the list (as you already suggested).
  2. Wrap it with ReadOnlyCollection within the getter.

Both cases should be documented in method description so that clients are not "surprised" when they attempt to modify the list externally.

Upvotes: 1

David Osborne
David Osborne

Reputation: 6801

Encapsulating lists of objects can be achieved quite nicely by restricting access to them using a suitable interface.

I think you're right to control additions to your list via your AddGuitar method as you can exert control over what goes in. You can reinforce this design, IMHO, by altering GetGuitars to return IEnumerable instead of List.

This reduces the control the caller has on your list, whilst also being non-committal in returning an abstract type. This way your internal data structure can change without the public interface needing to also.

Upvotes: 1

skunkfrukt
skunkfrukt

Reputation: 1570

I would agree that returning the list leaves something to be desired in terms on encapsulation. You may want to consider writing a getter for individual items, or possibly an iterator. The list seems like an implementation detail, so other classes really have no business accessing it directly.

Upvotes: 0

Related Questions