Overloaded applyRestrictionsToCriteria(…) methods

We want to refactor two methods that are exactly the same, except for one difference: one takes an org.hibernate.Criteria and the other org.hibernate.criterion.DetachedCriteria. These two do implement a mutual interface (org.hibernate.criterion.CriteriaSpecification), but this only contains some final static fields, and no methods.

Here are the methods (removed comments and javadoc for compactness):

```
public static DetachedCriteria applyRestrictionsToCriteria(final DetachedCriteria criteria,
final Vector filter) {
final Map subCriteriaMap = new HashMap<>();
if (filter != null) {
final Iterator itp = filter.iterator();
while (itp.hasNext()) {
final RestrictionsHelper restric = itp.next();
if (restric.getClassname().equals("")) {

final Iterator ir = restric.getCriterions().iterator();
while (ir.hasNext()) {
final Criterion criterion = ir.next();
criteria.add(criterion);
if (criterion.toString().contains("Happening_fk")) {
criteria.setFetchMode("Happeningdetails", FetchMode.JOIN);
}
}

final Iterator or = restric.getOrders().iterator();
while (or.hasNext()) {
criteria.addOrder(or.next());
}
} else {
final String[] buff = restric.getClassname().split("\\.");

DetachedCriteria subcriteria = criteria;
String path = "";
for (final String element : buff) {
final String[] name = getNameAndAlias(element);
path += name[0];
final DetachedCriteria exsubcriteria = subCriteriaMap.get(path);
if (exsubcriteria == null) {
subcriteria = subcriteria.createCriteria(name[0], name[1], CriteriaSpecification.LEFT_JOIN);

Vector vs List/ArrayList

Vector was retrofitted to be part of the Java Collections framework, and if you do not need the synchronization feature, you should update to the ArrayList class. In fact, you should opt for the List interface, so that callers of these methods can eventually be refactored to pass in other List implementations, like ArrayList, and these two methods only know they are dealing with Lists.

Indentation

public static DetachedCriteria applyRestrictionsToCriteria(DetachedCriteria criteria,
                    Vector filter) {
    final Map subCriteriaMap = new HashMap<>();
    if (filter != null) {
        // processing goes here
    }
    return criteria;
}

If you do an early return from the null-check, you can reduce one level of indentation. You also eliminate the possibly redundant new HashMap<>() declaration too, when the null-check holds true:

public static DetachedCriteria applyRestrictionsToCriteria(DetachedCriteria criteria,
                    List filter) { // changed Vector -> List
    if (filter == null) {
        return criteria;
    }
    Map subCriteriaMap = new HashMap<>();
    // processing goes here
    return criteria;
}

My take on final modifiers on method arguments and variables these days is that they are largely redundant, as long as you can easily observe that they are not carelessly reassigned. If you happen to come from a (programming) culture where this is done way too often, and thus you are introducing final to check such practices, then feel free to leave them in until such 'reminders' can be removed.

Looping via iteration

Another way of doing looping via iteration is to rely on the standard for-loop as such:

for (Iterator helpers : filter.iterator(); helpers.hasNext(); ) {
    // more processing goes here
}

This scopes the Iterator to within the for-loop itself. The simpler way is to use the enhanced for-each loop:

for (RestrictionsHelper helper : filter) {
    // more processing goes here
}

Deduplicating code blocks, part 1

final Iterator or = restric.getOrders().iterator();
while (or.hasNext()) {
    subcriteria.addOrder(or.next());
}

Since this is done regardless of restric.getClassname().equals(""), you can perform it outside of the if-block (illustrating only for DetachedCriteria):

public static DetachedCriteria applyRestrictionsToCriteria(DetachedCriteria criteria,
                    List filter) {
    if (filter == null) {
        return criteria;
    }
    Map subCriteriaMap = new HashMap<>();
    for (RestrictionsHelper helper : filter) {
        DetachedCriteria currentCriteria;
        if (helper.getClassname().isEmpty()) { // instead of String.equals("")
            currentCriteria = criteria;
            // some processing here
        } else {
            // some processing here
            // use currentCriteria instead of subcriteria
        }
        for (Order order : helper.getOrders()) {
            currentCriteria.addOrder(order);
        }
    }
    return criteria;
}

Declaring variables closer to usage

Now let's take a look at the Map declaration again:

Map subCriteriaMap = new HashMap<>();

It's only being used when RestrictionsHelper.getClassname() is not empty. In addition, the only thing that code block seems to be doing is to eventually have currentCriteria be the final DetachedCriteria (following the example from the previous section) after splitting the class name. This suggests that we can convert this block into a method:

private static DetachedCriteria processClassName(DetachedCriteria criteria, 
                                                    String className) {
    DetachedCriteria result = critera;
    StringBuilder path = new StringBuilder();
    Map subCriteriaMap = new HashMap<>();
    for (String element : className.split("\\.")) {
        String[] name = getNameAndAlias(element);
        path.append(name[0]);
        DetachedCriteria temp = subCriteriaMap.get(path.toString());
        if (temp == null) {
            result = result.createCriteria(name[0], name[1],
                                            CriteriaSpecification.LEFT_JOIN);
            subCriteriaMap.put(path.toString(), result);
        } else {
            result = temp;
        }
        path.append('.');
    }
    return result;
}

Deduplicating code blocks, part 2

The method in question now looks much shorter:

```
public static DetachedCriteria applyRestrictionsToCriteria(DetachedCriteria criteria,
List filter) {
if (filter == null) {
return criteria;
}
for (RestrictionsHelper helper : filter) {
DetachedCriteria currentCriteria;
if (helper.getClassname().isEmpty()) {
currentCriteria = criteria;
for (Criterion criterion = helper.getCriterions()) {
criteria.add(criterion);
if (criterion.toString().co

StackExchange Code Review Q#129191, answer score: 4