Looping Strategy: Change behaviour after first iteration

Question

I have often encountered the following situation. I want to call a method in a Loop, but want to skip the call at the first run. It do not have to be a method, it also can be some lines of code that I want to skip. Here is one example:

I have a file which looks like that.

headline:headline1
content:lorem ipsum dolor sit amet
content2:lorem ipsum dolor sit amet
content3:lorem ipsum dolor sit amet
headline:headline2
content1:lorem ipsum dolor sit amet
.
.
.

I iterate through the lines and generate following html:

<div> headline1 </div>
  <ul> 
    <li> lorem ipsum dolor sit amet </li>
    <li> lorem ipsum dolor sit amet </li>
    <li> lorem ipsum dolor sit amet </li>
  </ul>
<div> headline2 </div>
  <ul> 
    <li> lorem ipsum dolor sit amet </li>
  </ul>
  .
  .
  .

My Code looks like that:

private String createList(String file) {
        StringBuilder output = new StringBuilder();
        String[] lines = file.split("\n");

        int headlineCounter = 0;
        for (String line : lines) {                    
            String[] fields = line.split(":");    
            if (fields[0].equals("headline")) {
                if (headlineCounter > 0) {
                    output.append("</ul>");
                }
                output.append("<div>").append(fields[1]).append("</div>");
                output.append("<ul>");
                headlineCounter++;
            } else {
                output.append("<li>").append(fields[1]).append("</li>");    
            }
        }       
        output.append("</ul>");
        return output.toString();
    }

The content always is wrapped with an <ul> tag. When a new headline appears the content of the last headline is at the end and has to be closed with </ul> except it's the first headline of course, because there is no content before, that has to be closed.

In my example I am using a counter. On this way it works, but there are other and better ways I think. How would you solve this problem? This is just an example there are of course other and better ways to generate html, I just wanted to show you an example to my question.

Answer 1

One elegant way to handle this is to parse the input into a more usable structure before transforming it to HTML. For example, each section could be represented as:

private class Section {
  final String headline;
  final List<String> contents = new ArrayList<String>();

  Section(String headline) {
    this.headline = headline;
  }

  String asHtml() {
    StringBuilder sb = new StringBuilde();

    sb.append("<div>").append(headline).append("</div>");
    sb.append("<ul>");
    for (String item : contents) {
      sb.append("<li>").append(item).append("</li>");
    }
    sb.append("</ul>");

    return sb.toString();
  }
}

Note how the code clearly shows the structure of the resulting HTML.

The parsing code now has a more focused responsibility: parsing and validating the input. It has nothing to do with the output format any more.

private Iterable<Section> parseList(String fileContents) {
  List<Section> sections = new LinkedList<Section>();

  for (String line : fileContents.split("\n")) {
    String[] fields = line.split(":");
    if (fields.length != 2) {
      // throw some exception
    }

    if ("headline".equals(fields[0])) {
      sections.add(new Section(fields[1]));
      continue;
    }

    if (sections.isEmpty()) {
      // throw some error because "content" came before "headline"
    }

    sections.getLast().contents.add(fields[1]);
  }

  return sections;
}

I pointed out some lines with comments where you should do input validation. If you don't do this, you might get surprised by ArrayIndexOutOfBoundsExceptions if the input string is invalid.

Building an object that represents each section has the advantage that invalid HTML cannot be produces – your current code could emit a <li> outside of an <ul>.

Then in your main code, the sections are simply concatenated together:

StringBuilder output = new StringBuilder();
for (Sections section : parseList(...)) {
  output.append(section.asHtml());
}

The advantage of separating the responsibilities of parsing and formatting is that it is now much easier to adapt a single part (e.g. to a new input or output format). My suggestion for the Section class has to be criticized here, because it does not follow the Single Responsibility Principle: it both represents a section, and formats the output. Those should ideally be separated into two different classes, so that the formatting can vary without Section having to change.

Answer 2

You probably want line.split(":", 2) if your text has any chance of containing colons.

You can't concatenate just any text into HTML willy nilly. To prevent HTML injection flaws, you need to escape characters that may have special significance in HTML. You could use StringEscapeUtils in Apache Commons Lang or roll your own. The important thing is that you do it, for the sake of correctness and security.

Answer 3

The problem with the input is that the content elements are not grouped. This is why the code uses the headline element, and it gives rise to the need to distinguish the first occurrence from all others.

---

• headlineCounter is misleading. A boolean flag (e.g. closingULneeded) would have sufficed. By using an int, the code somewhat hides that fact.

• To know that the current content is the last one, the code would have to look ahead to the next line. Since it uses an iterator, this would have to be rewritten into a "standard" for loop (indexing into lines). Since the for loop would expose the position inside the array, the code then could just use a nested loop to process all following content elements; and btw write the opening and closing <ul> itself. No need to track any state information.

• The whole content of the file is passed to the method. Depending on the size of the file, this could get nasty. The code could easily avoid the string copy by not passing a string. In the same vein, don't return a string; this will display the same copy behaviour.

• Eventually, the creation of that lines array might be avoided, too.

• It looks like the code doesn't need to check for the whole headline string to decide whether the current line is a headline or content.

• The name of the method could reveal its intent a bit more explicitly. Something like ConvertContentToHtmlFragment; eventually containing the name of that particular file format, if existing.

• Should you know the format will complicate, perhaps a more DSLish approach might prove useful.

Answer 4

Besides the nice OO-solution, another (procedural) solution to your looping problem is possible:

If you have cases, where you iterate over a collection of x and only the first element needs special treatment, you could shift it off your collection, treat it like you intended to and iterate over the rest, so in your case: parse the text until "headline" is reached and iterate as intended over the rest. You could make use of ArrayList.remove(0) to shift the first element.

Pseudocode:

var current = Collection.shift();
while(current!="headline") Collection.shift();
//do whatever with your first occurence of "headline"
foreach(lines in Collection)
// do the other nasty stuff