HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Clean, efficient and extensible code for querying collections

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
extensibleefficientqueryingcodeforcollectionsandclean

Problem

I recently wrote code that would query a collection of objects and rank them according to a given criteria and then filter based on an object property. I attach the code below. I would like to know if my approach can be improved upon such as made more efficient and extensible. Is there a better approach or just another approach?

I know that lambdas in Java 8 would make the code much cleaner. What other improvements are available?

Here's the scenario:

Rank journals numerically by score, those with a shared rank ordered lexicographically and filter out journals that are review journals.

Here's the data:

Given the following journals have scores:

Journal A = 2.2 
Journal B = 6.2
Journal C = 6.2
Journal D = 1.2


And Journal D is a review journal.

The result should be:

Rank  Journal Name    Score

 1    Journal B        6.2  
 1    Journal C        6.2  
 3    Journal A        2.2


Note: Journal D is filtered out of the list.

Here's my code:

This code produces the actual sort and filter:

List journals = // add journals to collection.
Collate collateJournals = new Collate<>();
Collate collatedJournals = collateJournals.from(journals).filter(new OutReview()).sort(new ByScoreThenName(Direction.ASC)).rank(new Numerical());


This code controls the collation of sorted and filtered data.

```
public class Collate {

List collection = new ArrayList<>();

public Collate sort(Comparator sortComparator) {
Collections.sort(collection, sortComparator);
return this;
}
public T get(int i) {
return this.collection.get(i);
}
public boolean contains(T element){
return collection.contains(element);
}
public Collate from(List collection) {
this.collection = collection;
return this;
}
public Collate filter(Predicate predicate) {
List result = new ArrayList();
for (T element : (Collection) collection) {
if (predicate.

Solution

Lambdas

I know that lambdas in Java 8 would make the code much cleaner. What other improvements are available?

I am not sure I agree with the Lambdas in J8 statement. They have their place, but clean code is readable code, and only a small subset of problems have improved readability from Lambdas, and also, Lambdas are a 'meta level' function.... Implementing 'foundation logic' using Lambdas would be a mistake because then integrating with traditional Java would be harder.

Right tool for the job, etc. In this case, I don't think Lambdas would necessarily be the right tool, even in Java8.

Specifically, your code requires cross-referencing Journals against each other (to sort them), and this would be very messy in a Stream.
Generics

You have collections of Journal throughout your code.... This is not a problem, but what I have a concern about is the inconsistency of your own Generic classes. For example, you have:

public class OutReview implements Predicate{


and

public class Numerical implements Rank {


Which make specific implementations of generic Interfaces.

I have no problem with this, it is nice, and clean.

Then you also have:

public class Collate {


This is a generic class that has this neat ability to collate generically typed data based on matching Rank and Predicate instances....

On it's own, it is nice, clean, and general purpose.

But, you put them together, and you have a bit of a mess:

List journals = // add journals to collection.
Collate collateJournals = new Collate<>();
Collate collatedJournals = collateJournals.from(journals)
      .filter(new OutReview())
      .sort(new ByScoreThenName(Direction.ASC))
      .rank(new Numerical());


For consistency, the Collate class should be hard-coded to work with Journal, or your actual OutReview and other classes should be generic too.
Naming

List collection = new ArrayList<>();


Don't call a List collection. It makes the following line very confusing:

Collections.sort(collection, sortComparator);


Because Collections.sort( .... ) has only got a List<> version of the argument.

I would recommend a name like "journals" if you make the Collate class Journal specific instead of generic. Alternatively, something that describes the content, not the structure... like 'data', or 'values'.
Leaks

You have a few encapsulation leaks. The biggest is this one:

public Collate from(List collection) {
    this.collection = collection;
    return this;
}


you initialize your class with an empty ArrayList but then you throw it away and replace it with the from()'s argument.

Any changes outside your class to that collection (bad name) will now also be reflected in your class. For example, you may sort your collection, but then someone can add stuff out of order.....

You should be copying the collection content, not the actual reference:

public Collate from(List input) {
    data.clear();
    data.addAll(input);
    return this;
}


This will also save some interesting and embarrassing bugs like when someone calls your from method with:

collate.from(Collections.unmodifiableList(somedata));

Code Snippets

public class OutReview implements Predicate<Journal>{
public class Numerical implements Rank<Journal> {
public class Collate<T> {
List<Journal> journals = // add journals to collection.
Collate<Journal> collateJournals = new Collate<>();
Collate<Journal> collatedJournals = collateJournals.from(journals)
      .filter(new OutReview())
      .sort(new ByScoreThenName(Direction.ASC))
      .rank(new Numerical());
List<T> collection = new ArrayList<>();

Context

StackExchange Code Review Q#44756, answer score: 5

Revisions (0)

No revisions yet.