4
\$\begingroup\$

I've written this code keeping the following cases in mind and I think I've covered them all. I'd really appreciate if someone could help me determine if I've covered all of them.

Cases I considered:

  1. No comments at all
  2. Text /* Text */ Text
  3. Text /* Text */ Text /* Text */
  4. Text /* /* Text */ */ Text
  5. Text /* /* Text */ Text
  6. Text /* Text
  7. Text */ Text
private static String stripComments(final String input) {
    StringBuilder builder = new StringBuilder();
    boolean startSeen = false;

    for (int i = 0; i < input.length();) {
        int moveAheadBy = 1;

        if (startSeen) {
            if (input.charAt(i) == '*' && (i != input.length() - 1 && input.charAt(i + 1) == '/')) {
                startSeen = false;
                moveAheadBy = 2;
            }
        } else {
            if (input.charAt(i) == '/' && (i != input.length() - 1 && input.charAt(i + 1) == '*')) {
                startSeen = true;
                moveAheadBy = 2;
            } else {
                builder.append(input.charAt(i));
            }
        }
        i += moveAheadBy;
    }

    return builder.toString();
}
\$\endgroup\$
  • \$\begingroup\$ There seem to be (at least) a few cases you haven't considered. For example, (at least if memory serves), a /* inside a string literal does not start a comment. \$\endgroup\$ – Jerry Coffin May 31 '14 at 23:13
  • \$\begingroup\$ That is an interesting case.Thank you. Please let me know if you can think of more cases like this. \$\endgroup\$ – user1071840 Jun 1 '14 at 14:26
1
\$\begingroup\$

You didn't specify whether or not nested comments are allowed, but your code trims only the shorter comment. For example, case 4 would yield Text Text with nesting but Text */ Text without. It would be helpful to include some unit tests to demonstrate the intent.

Your solution is pretty straight-forward with only one real complication. I would remove the moveAheadBy incrementer and instead perform an extra increment where you're setting moveAheadBy to 2. It's minor, but it removes unnecessary state.

To help with the readability, introduce a variable for the length of the input since it's used in so many places.

Another solution would be to use indexOf to seek ahead to the next open/close. The algorithm will still be \$O(n)\$, but the fewer number of append calls could add up with very, very long strings. This is more to show an alternate method than to improve on yours.

public static String stripComments(final String input) {
    StringBuilder buf = new StringBuilder();
    int current = 0;
    int end = input.length();

    while (current < end) {
        int nextOpen = input.indexOf("/*", current);
        if (nextOpen == -1) {
            // no more comments
            buf.append(input.substring(current));
            break;
        }
        buf.append(input.substring(current, nextOpen));
        int nextClose = input.indexOf("*/", nextOpen + 2);
        if (nextClose == -1) {
            // final comment not closed
            break;
        }
        current = nextClose + 2;
    }

    return buf.toString();
}

Unit Tests

Now that the method is public, let's add some tests.

@Test
public void testNone() {
    assertThat(stripComments("No comments at all"), is("No comments at all"));
}

@Test
public void testSingle() {
    assertThat(stripComments("Single /* foo */ comment"), is("Single  comment"));
}

@Test
public void testDisjoint() {
    assertThat(stripComments("Two /* foo */ disjoint /* bar */ comments"), is("Two  disjoint  comments"));
}

@Test
public void testNested() {
    assertThat(stripComments("Two /* foo /* nested */ baz */ comments"), is("Two  baz */ comments"));
}

@Test
public void testUnclosed() {
    assertThat(stripComments("Unclosed /* foo comment"), is("Unclosed "));
}

@Test
public void testUnopened() {
    assertThat(stripComments("Unopened */ comment"), is("Unopened */ comment"));
}
\$\endgroup\$
  • \$\begingroup\$ Thank you for the response. I've dealt with the nested comments /* /* */ */ case like most IDEs do. \$\endgroup\$ – user1071840 Jun 1 '14 at 14:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.