Win a copy of Kotlin in Action this week in the Kotlin forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

HashSet contains() method fail ?  RSS feed

 
Sandeep Advani
Ranch Hand
Posts: 78
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Help !

I am adding agent-id�s to a HashSet in a server application. Incase, if the HashSet contains the agent-id, I return a message to the client saying that this id exists and kindly send another agent-id. Code snippet is as follows:


HashSet agents = new HashSet();

public String serviceMethod(byte[] arg, String agentId)
{
if( agents.contains(agentId) )
{
return "Change the agent id.";
}
else
{
System.out.println("Adding new agent id < " + agentId + " > to the database\n");
}
agents.add(agentId);
��.
}

Irrespective of my client sending the same agent-id, the agents.contains check fails to identify the redundant element in the hashset and adds it repeatedly. Interestingly, there are no duplicate elements; hence the size of HashSet is always one.

Question is why does the HashSet contains() method fail ? I don�t want to enter the agent id if I already have one in the Collection. Perhaps, I should use some other Collection class here.

Sandeep Advani
 
Nathaniel Stoddard
Ranch Hand
Posts: 1258
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Shouldn't you place your "agents.add(agentId)" within the "else" block to do what you want?
 
Sandeep Advani
Ranch Hand
Posts: 78
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Doesn't matter. if the control enters the 'if block', I return from the program. Else I execute the print statement followed by adding the Object to the Collection.

Before I explain my problem, a little of background stuff. This server appln. was deployed as a Web Service. Each time, client made a call to the method, the Collection was getting initialized and the class was loaded irrespective of where the declaration of HashSet was written. That means, there was no data in the set during the start of the code. Data was added only after the if-else block. In order to remain consistent, I had to use database and not any collection.

Isnt Dbase another big collection :-)

Thanks.

Sandeep
 
Stan James
(instanceof Sidekick)
Ranch Hand
Posts: 8791
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Ah, you hit the nail on the head with the collection being initialized every time. Someplace up the call stack you're getting a new instance of this class every time in, so a new collection. You could stash the collection somewhere else, maybe on the system context (?) so it would persist from one call to the next. You could also just make it static. Either way, you should probably guard access with a synchronized block.

And now an uninvited style note. The confusion from one reader about your if/else clause points up a problem. The System.out.print line and the agents.add line both happen when there is no duplicate, but they're in different code blocks at different levels of indentation. Why? Well, there is no good reason. An "else" after a return or throw like that is meaningless and confusing. Maybe it's just me being too picky, but it hurts to read it. Lose the else and two brackets. Less code, better match of syntax & semantics and exactly the same results.

Cheers!
[ June 18, 2004: Message edited by: Stan James ]
 
John Smith
Ranch Hand
Posts: 2937
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
And now an uninvited style note. The confusion from one reader about your if/else clause points up a problem. The System.out.print line and the agents.add line both happen when there is no duplicate, but they're in different code blocks at different levels of indentation.

I would add to this that the original code looks like C++ rather than Java. More specifically, it uses the return argument to identify some exceptional condition. The caller of this method is required to evaluate the return to make a decision, and even worse, the return is a String, so it's easy to make a typo. The Java way to go is simply throw an exception. The code can be refactored as follows:


And don't get me started on the naming of the classes, variables, and methods. If I were an employer and needed to cut expenses, a developer who names a method "serviceMethod" would be the first one to go.
[ June 19, 2004: Message edited by: Eugene Kononov ]
 
Warren Dew
blacksmith
Ranch Hand
Posts: 1332
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Eugene Kononov:

I would add to this that the original code looks like C++ rather than Java. More specifically, it uses the return argument to identify some exceptional condition.... The Java way to go is simply throw an exception.

Er, C++ has exceptions too, you know.

It's not clear from the code example just how exceptional that condition actually is. I agree "serviceMethod" is a bad function name, though.
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!