Whether this is Best Practice, it is possible to write like this in general, namely, let us say if an element already exists in the collection to throw an exception with a message.

static private Set<A> a = new HashSet<>(); public A save(A object) { if (!a.contains(object)) { a.add(object); return object; } else { throw new IllegalArgumentException("This object exists in the system already!"); } }
  • If this satisfies the business logic, and the method is not under high load, then why not. - Nofate ♦
  • You can write, but I do not think that this is Best Practice. - Russtam

1 answer 1

HashSet collection is based on HashMap . In both cases, for the contains() method and add() , it iterates through the elements of the collection. For HashSet / HashMap , in general, these are fast operations that are performed in O(1) , i.e. for constant time. The article in the topic .

However, unnecessary traversal of the collection can be avoided by deleting the contains() method since the add (E) method and so checks for the presence of an element and returns a boolean:

 if ( !a.add(object) ) { throw new IllegalArgumentException("..."); } return object;

Also, I would think whether it is necessary to return anything from the method at all. From the above code, it makes no sense to return a reference to an object that is passed to the method as an argument. If the element is not added, RuntimeException , that is enough.

In general, the usual code, without an explicit code smell. By one method it is not always possible to judge the effectiveness of the algorithm. If you need an expedition in this place - throw an expection. This is determined only by the logic of your application.

  • one
    This answer is not entirely to the question that the vehicle had in my opinion. - Nofate ♦
  • one
    returning an argument is a small convenience; you can write save( otherObject.getSomeA() ).someAMethod().anotherMethod() . Saving semicolons :) But the question really lacks an example of use. - zRrr