• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Encapsulate collection type

 
Haina Minawa
Ranch Hand
Posts: 119
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have a bean that contains a collection of type ArrayList<String>. I write following code to encapsulate the collection:



Please debate the code is good or not, suggestions are all welcome.
 
Harsha Smith
Ranch Hand
Posts: 287
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I think titles are unique. so I am using Set instead of list. so here is the code. And also I've changed the return type to let the client programmer know if the intended operation was successful or not.

 
Rob Spoor
Sheriff
Pie
Posts: 20493
54
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
That code has one flaw - setTitles should clear the set first, otherwise it's not setting the titles but adding to the titles. I would also let the method take a Set or perhaps even a Collection instead of a List.

I agree with letting getTitles return a direct reference and not a copy. You already allow it to be modified indirectly using addTitle and removeTitle, so there's little reason to not return a direct reference. That allows calling code to call other methods on the Set / List, like addAll or clear. If they need a copy they can create one as needed.
 
Harsha Smith
Ranch Hand
Posts: 287
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
As a penalty for the flaw, I am providing an extra method for client programmer.



 
Winston Gutkowski
Bartender
Pie
Posts: 10087
55
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Harsha Smith wrote:As a penalty for the flaw, I am providing an extra method for client programmer.

<being picky>

Shouldn't both your setTitles() methods behave the same way?
Seems to me like:might be slightly better.

</being picky>

Winston
 
Haina Minawa
Ranch Hand
Posts: 119
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Harsha Smith wrote:I think titles are unique. so I am using Set instead of list. so here is the code. And also I've changed the return type to let the client programmer know if the intended operation was successful or not.


I thinnk unique or not unique - that depends on context. I agree to change return type to boolean.
 
Haina Minawa
Ranch Hand
Posts: 119
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Rob Spoor wrote:That code has one flaw - setTitles should clear the set first, otherwise it's not setting the titles but adding to the titles. I would also let the method take a Set or perhaps even a Collection instead of a List.

I agree with letting getTitles return a direct reference and not a copy. You already allow it to be modified indirectly using addTitle and removeTitle, so there's little reason to not return a direct reference. That allows calling code to call other methods on the Set / List, like addAll or clear. If they need a copy they can create one as needed.


I think different about returning the direct reference of the collection. I prefer returning a copy to prevent the collection from unwanted modifications like setting it to null or to another object. If one wants to change the collection, he must call the bean's methods and that ensures the collection is modified correctly and avoids having potential bugs in future.
 
Rob Spoor
Sheriff
Pie
Posts: 20493
54
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
By returning a direct reference, you are actually returning a copy of your reference field. You can't change the value of the reference field; in other words, you can't set it to null or another object. You can only modify the object the reference refers to, and you can already do that now through the add / remove methods.
 
Harsha Smith
Ranch Hand
Posts: 287
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote:
<being picky>

Shouldn't both your setTitles() methods behave the same way?

</being picky>

Winston



Both the method needn't behave the same way. For example on an empty priority queue object if you invoke element() method....NoSuchElementException is thrown where as peek() method returns null. The use of both the methods is to retrieve but not remove the top element.
 
Winston Gutkowski
Bartender
Pie
Posts: 10087
55
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Harsha Smith wrote:Both the method needn't behave the same way. For example on an empty priority queue object if you invoke element() method....NoSuchElementException is thrown where as peek() method returns null. The use of both the methods is to retrieve but not remove the top element.

Yes but those methods aren't overloaded. I would expect a named method to have its own signature and behaviour, but not an overloaded one. But it's not wildly important either way.

Winston
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic