Return to Revisions

1. Cyclomatic complexity in NightScanner

You can reduce cyclomatic complexity (at cost of some memory overhead, usually acceptable) by replacing switch with map lookups:

    private static final Map<Integer, Codepoints> codePointMap = buildCodePointMap();
    // Alternative: build Map with Map.of(K1,V1, K2,V2, ...) in java 9

    private static Map<Integer, TokenType> buildCodePointMap() {
        //...
        map.put(Codepoints.AMPERSAND, TokenType.AMPERSAND);
        map.put(Codepoints.EQUALS_SIGN, TokenType.ASSIGNMENT);
        return map;
    }

    private OptionalToken matchSymbol(int codePoint, TextIterator iterator) {
        TokenType result = symbolMap.get(codePoint);
        return result != null ? OptionalToken.of(result, iterator) : matchSymbolEdgeCase(codePoint, TextIterator);
        // let matchSymbolEdgeCase deal with things like FULL_STOP or no match
    }

You could then refactor the map out to a separate class, allowing NightScanner to delegate those details instead of doing everything.

For the complexity in scan(String text), it should be possible to extract the textIterator.next() outside of the if statements and get rid of the continue statements. Roughly, something like this may work in the while:

    boolean err = false;
    //... existing if statements
    if (err) textIterator.next();
    else tokens.add(token.getToken());

Then probably just return new ScanResults(tokens, errors), modify ScanResults so that the lists are never null, and then change isSuccess() to return errors.isEmpty().

2. Issues with ScanResults and OptionalToken

Actually, ScanResults currently seems like a slightly unnecessary abstraction (some kind of exception strategy might be better), but it can stay for now.

If possible, OptionalToken shouldn't manage both tokens and errors. Errors could be addressed in a custom Exception class, and then it may be possible to remove OptionalToken and use a standard Optional<Token>.

3. BTW, it's perfectly good to refactor to NumberScanner, SymbolScanner, etc. Just keep them package scope and let them interact via package-visibility methods. NightScanner would still be the only one with public methods. Coupling is usually fine if a class evolves into multiple related classes. Usually the horror stories are across different packages, bidirectional coupling, or dependency.getThis().getThat()...