How to refactor "long" if statment in lexer? [closed]

Question

I have a Tokenizer doing lexer thing.

It is used like

String line = "class Main {"

ArrayList<String> tokens = LineTokenizer.tokenize(line)

The overall logic of LineTokenizer.tokenize is like below and tokenize method is static

for ( char c : line ){
    if (condition){
        doSomething1(); //like arrayList.add(c)
        doSomething2(); //like tempWord += c
    }else if(condition2) {
        doSomething2();
    }else if(condition3) {
        doSomething2();
        doSomething3();
    }
}

And There are 8 conditions which are functionized just because of readability.

    private static boolean isOpeningQuotationMark(char currentChar) {
        return (currentChar == '\"' && progressInQuotationMark == false);
    }

    private static boolean isClosingQuotationMark(char currentChar) {
        return (currentChar == '\"' && progressInQuotationMark == true);
    }

    private static boolean isTailWhiteSpace(char currentChar) {
        return Character.isWhitespace(currentChar) && !tempWord.isEmpty();
    }

    private static boolean isHeadWhiteSpace(char currentChar) {
        return (Character.isWhitespace(currentChar) && tempWord.isEmpty());
    }
    
    private static boolean isSymbol(char currentChar){
        return Symbol.contains(currentChar);
    }

    private static boolean metSymbolAtEmpty(char currentChar) {
        return Symbol.contains(currentChar) && tempWord.isEmpty();
    }

    private static boolean metSymbolAtFull(char currentChar) {
        return Symbol.contains(currentChar) && !tempWord.isEmpty();
    }

And This class have 2 fields which are just needed to make 8 conditions static for the tokenize which is also static method.

private static boolean progressInQuotationMark = false;
private static String tempWord = "";

And This is whole code

public class LineTokenizer {
    private static boolean progressInQuotationMark = false;
    private static String tempWord = "";
    private static boolean isOpeningQuotationMark(char currentChar) {
        return (currentChar == '\"' && progressInQuotationMark == false);
    }

    private static boolean isClosingQuotationMark(char currentChar) {
        return (currentChar == '\"' && progressInQuotationMark == true);
    }

    private static boolean isTailWhiteSpace(char currentChar) {
        return Character.isWhitespace(currentChar) && !tempWord.isEmpty();
    }

    private static boolean isHeadWhiteSpace(char currentChar) {
        return (Character.isWhitespace(currentChar) && tempWord.isEmpty());
    }
    
    private static boolean isSymbol(char currentChar){
        return Symbol.contains(currentChar);
    }

    private static boolean metSymbolAtEmpty(char currentChar) {
        return Symbol.contains(currentChar) && tempWord.isEmpty();
    }

    private static boolean metSymbolAtFull(char currentChar) {
        return Symbol.contains(currentChar) && !tempWord.isEmpty();
    }
    
    public static ArrayList<String> tokenize(String line){
        ArrayList<String> tokenizedLine = new ArrayList<>();

        for (char currentChar : line.toCharArray()) {
            if (isOpeningQuotationMark(currentChar)) {
                progressInQuotationMark = true;
            } else if (isClosingQuotationMark(currentChar)) {
                tokenizedLine.add('\"' + tempWord + '\"');
                tempWord = "";
                progressInQuotationMark = false;
            } else if (progressInQuotationMark) {
                tempWord += currentChar;
            } else if (isTailWhiteSpace(currentChar)) {
                tokenizedLine.add(tempWord);
                tempWord = "";
            } else if (isHeadWhiteSpace(currentChar)) {
                ;
            } else if (!isSymbol(currentChar)) {
                tempWord += currentChar;
            } else if (metSymbolAtFull(currentChar)) {
                tokenizedLine.add(tempWord);
                tokenizedLine.add(String.valueOf(currentChar));
                tempWord = "";
            } else if (metSymbolAtEmpty(currentChar)) {
                tokenizedLine.add(String.valueOf(currentChar));
                tempWord = "";
            }
        }
        return tokenizedLine;
    }

}

This code works fine. But I think there are code smells because of

1. "long" if statement.
2. I think it violates Open Closed Principle.
3. The order of if statement is hard to change.
4. If I want to change or add new condition, I have to think all the conditions again.

What I tried is

make a new class for a char in lines. class CodeChar . But This Class needed a tempWord of original logic and some conditions in caller class. I made a enum for each conditions and made them type attribute of CodeChar. So I can make Set dictionary of <CodeCharType, Runnable> <- Runnable in this Set is logic inside if statements But Those also caused dependency of each other and looks kinda "dirty"

How can I solve this problem? Is there a pattern for this?

Before I consider changing the if statement, Are There already fundamental design problem?

Or Is that okay to be like that?

And This is another problem but If It is okay to ask,

ArrayList<String> tokens = LineTokenizer.tokenize(line) In this code, does tokenize should not be a verb? because It is not void?

Answer 1

Define Your Tokens

Take a moment to define what your tokens look like and what types you will have. For example a lexical parser that parses mathematical equations will have two types of tokens Numbers and Symbols(or Operators). This gives a good basis for how we want to parse any given input.

In your sample input "class Main {", this looks like it would be a form of code syntax parser.

What would our tokens for a simple code parser look like? In the example input we would have 3 tokens probably something like:

1. Identifier ("class", and maybe others like "enum", "struct",etc)
2. IdentifierDeclaration ("Main")
3. Block ("{", and everything in between until "}")

Based on these simple definitions we know that something like "classMain" is invalid syntax and we should report as such. It also makes sense to do a simple split on the input first splitting by spaces instead of iterating char by char because we now know for a fact what our keywords will be and can just look for those, the chars 'c','l','a','s','s' by themselves will never be a valid token, so there is no point in iterating over each char.

I recommend further reading on this as it is a very complex subject. A couple good places to start: EBNF and Java's formally defined Syntax using the aforementioned EBNF style.

Answer 2

By using a static method with static class variables you have basically created a singleton. That is almost never a good idea. Instead, create a regular class with non-static methods. To use it you need 1 extra line of code to first instantiate the tokenizer:

LineTokenizer tokenizer = new LineTokenizer();
ArrayList<String> tokens = tokenizer.tokenize(line);

Avoiding a singleton helps you in several ways:

1. You can use dependency injection to instruct another class to use a specific tokenizer (you provide it as argument to its constructor)

2. You can replace the tokenize by a test double

3. You can create multiple tokenizers each with their own state and run them in parallel (e.g. in a multi-threaded program)