Stefan Jankovic
Stefan Jankovic

Reputation: 33

Creating generic methods for different entities

I have a problem where right now I need to create a method with same function for every class where I want to consume it.

So, for example I have 10 entities, for all of that entities I have repo class and also for all of them I have as well services and controllers class. I created a new service class that is called QueryEntities

QueryEntitiesImpl:

@Service
@Component
@EnableAutoConfiguration
public class QueryEntitiesImpl implements QueryEntitiesService {

private static final Queue<AdventureHolidays> elementsToReturn = new LinkedList<>();

@Autowired
private final MongoTemplate mongoTemplate;

public QueryEntitiesImpl(MongoTemplate mongoTemplate) {
    this.mongoTemplate = mongoTemplate;
}

@Override
public AdventureHolidays queryEntity(String where, String is) {
    if (elementsToReturn.size() == 0) {
        Query query = new Query();
        query.addCriteria(Criteria.where(where).is(is));
        List<AdventureHolidays> newData = mongoTemplate.find(query, AdventureHolidays.class);
        Collections.shuffle(newData);
        elementsToReturn.addAll(newData);
    }
    return elementsToReturn.poll();
}
}

As you can see I have queryEntity method and inside that method you can see that I read size of collection, then I have some criteria and at the end I'm returning a random value from it. That works fine.

As you can see, I'm using private static final Queue<AdventureHolidays> elementsToReturn = new LinkedList<>();

and public AdventureHolidays queryEntity so that mean for every other entity I need to create same method just with different entity.

For example, I have another one entity called Backpacking so that will mean I need another method that has same functionality just for Backpacking

private static final Queue<Backpacking> elementsToReturn = new LinkedList<>();

    @Override
public Backpacking queryEntity(String where, String is) {
    if (elementsToReturn.size() == 0) {
        Query query = new Query();
        query.addCriteria(Criteria.where(where).is(is));
        List<Backpacking> newData = mongoTemplate.find(query, Backpacking.class);
        Collections.shuffle(newData);
        elementsToReturn.addAll(newData);
    }
    return elementsToReturn.poll();
}
}

Is there a way to create a method where will I just change Entity when I'm calling it?

I used generic methods before but I'm stuck with this when I need to create also this private static final Queue<AdventureHolidays> elementsToReturn = new LinkedList<>(); and method with Entity

Upvotes: 0

Views: 377

Answers (1)

rzwitserloot
rzwitserloot

Reputation: 103254

Your code appears broken to me, and we need to cover that first, because the broken part has limited your options. Just get rid of the broken part and you now have more options.

Specifically, your code works like this, right now. Which could be intentional, but I doubt it:

  • One can call queryEntity and specify any where clause and any is clause.
  • The code will then do one of two completely different things: "Cache lookup mode" or "Query mode".
  • IF this is the first time you run this query or the last time you run it in "Query mode" produced 0 results / you processed all results, you get query mode. Otherwise, you get cache lookup mode.
  • In cache lookup mode, the where and is clauses are completely ignored - you get 'the next result'.
  • In query mode, you set up the where and is clauses for all future calls to this method, until the 'list' is 'exhausted'.
  • A caller has no way to tell if the list is now exhausted. The method returns an adventures object in either mode, and said object cannot be used to determine if the 'hopper' is now empty or not.

The problem here is that cache lookup mode stuff: It means that not completely emptying out the cache makes your QueryEntitiesImpl object be in a broken state: You can call queryEntity with completely different where and is clauses, but all it will do is return results from the half-processed earlier query.

A complete redesign is pretty much required regardless of your additional request that it also be more generic than this version. And once we're completely redesigning it, the problem of 'specifically, in this exact code style that I have so far, how do I make it generic' ceases to be a relevant question. This is one of those cases where 'do both at once' is a lot simpler than doing them separately. ("Fix this code so that its API design leaves things in bizarre states" and "Make it more generic" being the two things you are trying to do).

It feels like your design is simply to have a method that you can call in order to produce 1 random AdventureHolidays object, any time you call it, forever (because right now you run a query, shuffle it, start returning one element at a time, unil the list is empty, in which case a call will combine "refill the list from the database and shuffle it" with "return the next item from it". From the point of view of the caller, as long as you provide consistent values for when and is, every call produces a random holidays object, whether it queried or not isn't even material to the 'experience' of the caller. Provide inconsistent values for where and is and the class acts like a drunken sailor, returning seemingly random and unrelated stuff all the time.

If that's indeed your point, you can make a much nicer API:

public T random(Class<T> type, String where, String is) {
  Query query = new Query();
  query.addCriteria(Criteria.where(where).is(is));
  List<T> newData = mongoTemplate.find(query, type);
  if (newData.isEmpty()) throw new NoSuchElementException();
  int rndIndex = rnd.nextInt(newData.size());
  return newData.get(rndIndex);
}

You'd call with for example Backpacking b = random(Backpacking.class, "raftingLevel", ">4"); or what not.

This code does not cache - every call results in another db query. However, your code caches in a bizarre fashion: It caches results for one 'go through' all adventures, but then re-pings the database. Your code also ensures full coverage: Each distinct adventure is hit exactly once (in random order). I'm not sure you specifically intended for that exact behaviour. We can re-introduce the cache, but take the concept of 'cache' more seriously: Obviously, the nature of the cached results depend on all 3 parameters: You should delve into that cache ONLY if the caller has identical where, is, and type parameters! We can do that - make a cache whose key is a tuple of [where, is, type]. But that's complicated. Much easier to change how this method works: Instead of providing all 3 to get a random thing back where the object remembers what was returned before for a given [where, is, type] tuple, instead change how it works: You provide a type, an is, and a where clause, and out comes a thing you can ask for random stuff. This even lets you cache even more powerfully: We can state that this object lives entirely outside of the DB, and can be asked for random stuff forever without ever re-querying the DB (and also being really fast at it, as a consequence). If you want to 'refresh the cache' just toss the object out and make a new one.

Looks something like this:

public <T> Source<T> queryEntity(Class<T> type, String where, String is) {
  Query query = new Query();
  query.addCriteria(Criteria.where(where).is(is));
  List<T> newData = mongoTemplate.find(query, type);
  return new Source<T>(mongoTemplate.find(query, type));
}

public static class Source<T> {
  private final List<T> elements;
  private int idx;

  Source<T>(List<T> elements) {
    this.elements = elements;
    this.idx = elements.size();
  }

  public T getRandom() {
    if (elements.size() == 0) throw new NoSuchElementException();
    if (idx == elements.size()) {
      idx = 0;
      Collections.shuffle(elements);
    }
    return elements.get(idx++);
  }
}

The Source object now tracks all this state instead of having a field do it. This gives control of the cache to the user of this API (they can use this Source object for as long as they want, and just toss it without fully 'emptying the hopper' so to speak), the API itself is no longer bizarre (no part of this API needs to be documented with "in such and such cases, these parameters are just completely ignored", unlike your take on this API), and in passing, making it generic was a lot simpler.

Usage:

Source<Backpacking> trips = queryEntity("raftingLevel", ">4");
for (int i = 0; i < 5; i++) {
  System.out.println("Random high-adrenaline backpacking trip idea #" + i + ": " + trips.getRandom().getDescription());
}

Upvotes: 2

Related Questions