Easy Groovy Iteration

The problem

Let's say we want to check the correct mimetype and file extension of the file being uploaded. In a typical Java way I would write something like this:

boolean isRightType = "image/jpeg".equals(contentType)
    || "image/png".equals(contentType)
    || "image/x-png".equals(contentType);

boolean isRightFileExt = filename.endsWith(".png")
    || filename.endsWith(".jpg")
    || filename.endsWith(".jpeg");

Groovy solution

The Groovy way is much simpler and more elegant.

private static CONTENT_TYPES = ["image/jpeg", "image/png", "image/x-png"]
private static FILE_EXTENTIONS = [".png", ".jpg", ".jpeg"]
...
boolean isRightType = CONTENT_TYPES.any { it.equals(contentType) }
boolean isRightFileExt = FILE_EXTENTIONS.any { filename.endsWith(it) }

This was just a small example. What I really find useful are some of the following constructs that make my programming life really pleasant

Groovy Iteration

each

Simple collection iteration

def fibList = [1, 1, 2, 3, 5, 8, 13]
fibList.each { println it }  // prints all of the numbers in the list

any

If you need to find out if any element of the collection meets the condition.

def fibList = [1, 1, 2, 3, 5, 8, 13]
assert fibList.any { it == 3 }
assert fibList.any { it - 2 > 10 }

This is pretty much equivalent to a Java construct

boolean any(List list, Condition cond)
    for (E e : list) {
        if (cond.meets(e)) {
            return true;
        }
    }
    return false;
}

every

If you need to find out if any element of the collection meets the condition.

def fibList = [1, 1, 2, 3, 5, 8, 13]
assert fibList.every { it > 0 }

This is pretty much equivalent to a Java construct

boolean every(List list, Condition cond)
    for (E e : list) {
        if (!cond.meets(e)) {
            return false;
        }
    }
    return true;
}

collect

If you need to create a new collection that contains each element of the original collection transformed in some way.

def fibList = [1, 1, 2, 3, 5, 8, 13]
assert fibList.collect { it - 1 } == [0, 0, 1, 2, 4, 7, 12]

This is pretty much equivalent to a Java construct

List every(List list, Command command)
    List result = new ArrayList(list.size());
    for (E e : list) {
        result.add(command(e));
    }
    return result;
}

findAll

If you need to create a new collection that contains all elements that meet the condition.

def fibList = [1, 1, 2, 3, 5, 8, 13]
assert fibList.findAll { it > 1 && it < 5 } == [2, 3]

This is pretty much equivalent to a Java construct

List findAll(List list, Condition cond)
    List result = new ArrayList(list.size());
    for (E e : list) {
        if (!cond.meets(e)) {
            result.add(e);
        }
    }
    return result;
}

find

If you need to find first element that matches the condition.

def fibList = [1, 1, 2, 3, 5, 8, 13]
assert fibList.find { it > 1 } == 2

This is pretty much equivalent to a Java construct

E findAll(List list, Condition cond)
    for (E e : list) {
        if (!cond.meets(e)) {
            return e;
        }
    }
    return null;
}

References

• Groovy Makes Iteration Easy by Ted Naleid

Refactoring or Redesign?

If you ever worked on a software that has grown organically over time, you will agree that many candidates for refactoring can be found in it. These are identified as pain points, smells or simply things that suck. The reason is that as the software grows, requirements change and code needs to adapt in order to support these changes.

JIRA is not an exception. JIRA team will spend around 30 hours in each eight week release cycle on refactoring of existing code, improving design, making it easier to extend the current code base with new features. At the iteration planning meeting last week, my colleagues and I were discussing how to spend this time in the most efficient way, debating what is refactoring and what is not. One of my colleagues raised a question: "Is is refactoring or redesign?"

The answer is: "It's both."

According to The Pragmatic Programmer: From Journeyman to Master written by Andrew Hunt and David Thomas:

At its heart, refactoring is redesign. Anything that you or others on your team designed can be redesigned in light of new facts, deeper understandings, changing requirements, and so on. But if you proceed to rip up vast quantities of code with wild abandon, you may find yourself in a worse position that when you started.

Clearly, refactoring is an activity that needs to be undertaken slowly, deliberately, and carefully. Martin Fowler offers the following simple tips on how to refactor without doing more harm than good:

1. Don't try to refactor and add functionality at the same time.
2. Make sure you have good tests before you begin refactoring. Run the tests as often as possible. That way you will know quickly if your changes have broken anything.
3. Take short, deliberate steps: move a field from one class to another, fuse two similar methods into a superclass. Refactoring often involves making many localized changes that result in a larger-scale change. If you keep your steps small, and test after each step, you will avoid prolonged debugging.

Recursion vs. Loop

Problem

Imagine that your application is meant to process an XML file. It is given a tag and needs to find a "special tag" that this tag might be nested in. This problem has several solutions. I tackled a similar problem some time ago and I will present you with some of the solutions that I considered.

While loop

private SpecialTag findSpecialTag()
{
  Tag parent = this.getParentTag();
  while (parent != null && !(parent instanceof SpecialTag))
  {
    parent = parent.getParentTag();
  }
  return (SpecialTag) parent;
}

Do-while loop

private SpecialTag findSpecialTag()
{
  Tag parent = this;
  do
  {
    parent = parent.getParentTag();
  }
  while (parent != null && !(parent instanceof SpecialTag));
  return (SpecialTag) parent;
}

Recursion

private SpecialTag findSpecialTag()
{
  return findSpecialTag(this.getParentTag());
}

private SpecialTag findSpecialTag(Tag tag)
{
  if (tag == null || tag instanceof SpecialTag)
  {
    return (SpecialTag) tag;
  }
  else
  {
    return findSpecialTag(tag.getParentTag());
  }
}

Solution

In my personal opinion, I find the while loop cleanest and simplest to understand. Do-while loop gets a bit messy with the condition at the end and recursion is probably the worst. I find recursion a bit hard to follow. It can also introduce an extra method just for its own sake (as in this example).

I do not have a strong opinion on this topic. If you like to program recursions, while loops, do-while loops, or for loops, it's fine by me. I certainly will not change your code, but if you ask me to write some code, it'd most likely be a while (or for) loop.

I use while and for loops interchangeably. The previous example with while loop would look like this with for loop:

private SpecialTag findSpecialTag()
{
  Tag p = this.getParentTag();
  for(; p != null && !(p instanceof SpecialTag); p = p.getParentTag());
  return (SpecialTag) parent;
}

What is your preference?

First element in the List

Imagine this scenario. You are working on an existing application. There is a framework used. One of the benefits that this framework gives you is retrieving and processing parameters coming from the client, let's say a web application. The framework gives you all parameters as a List. Now in this particular case you always get only one parameter, but is it enclosed in the List. How do you get it out efficiently?

The intended method would be described: Get the list of parameters and if not empty, return first element in the list otherwise return null. The code was looked similar to this:

1 public E getSingleParam(List params) {
2     E param = null;
3     if (params != null && params.size() >= 1) {
4         param = params.iterator().next();
5     }
6 }

This code works fine but it has several points for improvement. First is how the code at line 3 expresses the intention of check whether the list of parameters is not null and not empty, specifically the later. List interface defines boolean isEmpty() method that is in most cases more efficient to run than getting the size and comparing it to 0 (greater than) or 1 (grater than or equal). That is if the list implements an internal flag for its "emptiness" state or has an internal counter as opposed to re-counting its elements. In the worst case scenario the efficiency will be the same as getting the size and comparing it with 0. In that case isEmpty() method is still a nice convenience method to call and should be preferred before the size() > 0 alternative. Another reason is that it speaks for itself. All we need to know is whether there are any elements in the list or not. Getting the size should be used to for other purposes (e.g. calculating the width of the table column when displaying the results in a tabular form – 100% / size()).

1 public E getSingleParam(List params) {
2     E param = null;
3     if (params != null && !params.isEmpty()) {
4         param = params.iterator().next();
5     }
6 }

The second point of making the code to perform better and to make it easier to read is the line 4. On this line we are getting the first element of the list. As shown in the example above, this is an inefficient way as we construct an Iterator only for the purpose of one iteration. Constructing new objects and disposing of them is usually expensive and should be avoided. A better way is to access the element directly, such as:

1 public E getSingleParam(List params) {
2     E param = null;
3     if (params != null && !params.isEmpty()) {
4         param = params.get(0);
5     }
6 }

This way no new objects are constructed (no Iterator). Another difference between the two is the exception that could be possibly thrown. The exception would be thrown if we did not have the previous check or in the case of concurrent modification of the list which is very unlikely in this case. The exception thrown in first case would be a NoSuchElementException. In the second way, an IndexOutOfBoundsException would be thrown. Both of them are runtime exceptions and do not have to be declared. In this case getting one exception or the other should not make any difference.