Using an alternative for a switch/state-based custom parser

Question

I'm currently developing a small tool that applies a number of regex-based match/replacement-pairs to the contents of a file (or, more generally speaking, a string).
Note that regex-knowledge is not relevant to this answer, which is the reason why I didn't add it to the tags.

Now, this tool also supports saving all of your match/replace-pairs to a file. Since I've made bad experiences when using GSON with lists or arrays in the past, I decided to use my own format - considering it just has to save some kind of match-replace-separator-match-replace-separator-etc-style structure.

Now, my code works so far, but during refactoring, I felt like using a switch-statement with states for parsing my format is ugly and/or bad style.
However, if you believe that this is in fact a good solution, contrary to my assessment, I would of course also be happy with that answer.

The private static final int values in the snippet below will be replaced with an enum eventually, but for this purpose, I figured this would be more readable (by removing the need for an additional class).

private static final String SEPARATOR = "###";
private static final int SEPARATOR_EXPECTED = 0;
private static final int MATCH_EXPECTED = 1;
private static final int REPLACE_EXPECTED = 2;

public static List<MatchReplacePair> readRegexesFromFile(File file) {
    List<MatchReplacePair> list = new ArrayList<>();
    try (BufferedReader br = new BufferedReader(new FileReader(file))) {
        String line = br.readLine();
        int state = SEPARATOR_EXPECTED;
        MatchReplacePair pair = new MatchReplacePair("", "");
        while (line != null) {
            switch (state) {
            case SEPARATOR_EXPECTED:
                if (line.startsWith(SEPARATOR)) {
                    state = MATCH_EXPECTED;
                } else {
                    throw new IllegalStateException("Separator expected, but not found.");
                }
                break;
            case MATCH_EXPECTED:
                pair.setMatch(line);
                state = REPLACE_EXPECTED;
                break;
            case REPLACE_EXPECTED:
                pair.setReplace(line);
                // note: I have overridden the clone() method for this purpose, so it actually does what it's supposed to.
                list.add(pair.clone());
                state = SEPARATOR_EXPECTED;
                break;
            default:
                throw new IllegalStateException("Unknown state. Please contact the developer.");
            }
            line = br.readLine();
        }
        if (state != SEPARATOR_EXPECTED) {
            throw new IllegalStateException(
                    "Incorrect file contents: make sure the file ends with a \"replace\" block.");
        }
    } catch (IOException e) {
        e.printStackTrace(log);
    }
    return list;
}

A sample file to parse:

### //This is where one could write comments if one was so inclined.
match
replace

You don't need an additional class file to create an enum. Just create a private enum inside your existing class instead of the int constants. — DodgyCodeException
– DodgyCodeException, Commented Feb 9, 2018 at 14:46
Also, instead of having a dummy MatchReplacePair with a custom clone() method, consider writing a MatchReplacePair.Builder class - that's what builders are for. — DodgyCodeException
– DodgyCodeException, Commented Feb 9, 2018 at 14:50

Sharon Ben Asher · Accepted Answer · 2018-02-09 14:18:10Z

1

Although you didn't share MatchReplacePair I assume it holds the Strings that were loaded from the file. So in fact you read all the file's contents into memory.

Starting with Java 7, this can be achieved in one method call:

List<String> lines = Files.readAllLines(file.toPath());

then, you can skip the state machine implementation and just iterate over the list

if (lines.size() % 3 != 0) {
   // Incorrect file contents
}
Iterator<String> itr = lines.iterator();
// the above check ensures you can iterate over three lines safely
while (itr.hasNext()) {
  itr.next(); // ignore separator line? or you can verify if you wish
  list.add(new MatchReplacePair(itr.next(), itr.next()));
}

This also eliminates the need for clone() which was used as constructor replacement.

edited Feb 9, 2018 at 14:18

answered Feb 8, 2018 at 21:34

Sharon Ben Asher

3,67712 silver badges15 bronze badges

\$\begingroup\$ thanks, appreciate it! And yes, MatchReplacePair basically just contains two strings and some utility methods. \$\endgroup\$

PixelMaster
– PixelMaster

2018-02-09 12:45:04 +00:00
Commented Feb 9, 2018 at 12:45

Add a comment |

Stack Exchange Network

Using an alternative for a switch/state-based custom parser

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Using an alternative for a switch/state-based custom parser

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions