0

I am a computer science university student working on my first 'big' project outside of class. I'm attempting to read through large text files (2,000 - 3,000 lines of text), line by line with buffered reader. When a keyword from a list of enums is located, I want it to send the current line from buffered reader to its appropriate method to be handled appropriatley.

I have a solution, but I have a feeling in my gut that there is a much better way to handle this situation. Any suggestions or feedback would be greatly appreciated.

Current Solution I am looping through the the list of enums, then checking if the current enum's toString return is in the current line from buffered reader using the String.contains method. If the enum is located, the enum is used in a switch statement for the appropriate method call. (I have 13 total cases just wanted to keep the code sample short).

try (BufferedReader reader = new BufferedReader(new FileReader(inputFile.getAbsoluteFile()))){

    while ((currentLine = reader.readLine()) != null) {
        
        for (GameFileKeys gameKey : GameFileKeys.values()) {
            
            if (currentLine.contains(gameKey.toString())) {
                switch (gameKey) {
                    case SEAT -> seatAndPlayerAssignment(currentTableArr, currentLine);
                    case ANTE -> playerJoinLate(currentLine);
                }
            }
        }
    }
}

Previous Solution Originally, I had a nasty list of if statements checking if the current line contained one of the keywords and then handled it appropriatley. Clearly that is far from optimal, but my gut tells me that my current solution is also less than optimal.


try (BufferedReader reader = new BufferedReader(new FileReader(inputFile.getAbsoluteFile()))){

    while ((currentLine = reader.readLine()) != null) {
        
        if(currentLine.contains(GameFileKey.SEAT){
            seatAndPlayerAssignment(currentTableArr, currentLine);
        }
        else if(currentLine.contains(GameFileKey.ANTE){
            playerJoinLate(currentLine);           
        }
    }
}

Enum Class In case you need this, or have any general feedback for how I'm implementing my enums.

public enum GameFileKeys {
    ANTE("posts ante"),
    SEAT("Seat ");

    private final String gameKey;

    GameFileKeys(String str) {
        this.gameKey = str;
    }

    @Override
    public String toString() {
        return gameKey;
    }
}
  • If you want to be efficient you could construct a regex expression that contains all the ENUM and does a single check on the line rather than a `for` loof that loops each line for every ENUM. See here for how that might be done [Java regular expression OR operator](https://stackoverflow.com/questions/2031805/java-regular-expression-or-operator) – sorifiend Jul 19 '22 at 04:13
  • If all of the methods to be called on the line had the same signature, I'd suggest putting that on the enum, but it looks like they don't. – tgdavies Jul 19 '22 at 04:15
  • Thanks for the response! I looked into regex expressions but it seems that would just return a boolean. Do you have any suggestions for if the regex evaluates to true? In particular, how would I gather which one of my ENUM values matched? – Another Joe Jul 19 '22 at 04:17
  • Could one line have multiple matches (from Enum). From your current implementation it seems you are only interested in first match? – Tintin Jul 19 '22 at 04:19
  • For the regex idea it would just help you work out quickly if a line contained one of you ENUM or not. By doing that you only do heavy processing on the lines that require it, rather than on all lines. Unless you are running mission-critical code or serving thousands of simultaneous connections then your current option works just fine, and there is no reason to make it more efficient. – sorifiend Jul 19 '22 at 04:21
  • @Tintin No, each line will only have exactly one ENUM match. These are generated text files with a predictable output. – Another Joe Jul 19 '22 at 04:22
  • @sorifiend I understand what you are saying now, thank you for all your assistance! – Another Joe Jul 19 '22 at 04:23
  • Further reading on `switch` vs `if/else` chains here: [if else vs switch performance in java](https://stackoverflow.com/questions/33051931/if-else-vs-switch-performance-in-java) – sorifiend Jul 19 '22 at 04:26
  • @sorifiend Thank you again, this is a great explination. – Another Joe Jul 19 '22 at 04:32

1 Answers1

0

I cannot improve over the core of your code: the looping on values() of the enum, performing a String#contains for each enum object’s string, and using a switch. I can make a few minor suggestions.

I suggest you not override the toString method on your enum. The Object#toString method is generally best used only for debugging and logging, not logic or presentation.

Your string passed to constructor of the enum is likely similar to the idea of a display name commonly seen in such enums. The formal enum name (all caps) is used internally within Java, while the display name is used for display to the user or exchanged with external systems. See the Month and DayOfWeek enums as examples offering a getDisplayName method.

Also, an enum should be named in the singular. This avoids confusion with any collections of the enum’s objects.

By the way, looks like you have a stray SPACE in your second enum's argument.

At first I thought it would help to have a list of all the display names, and a map of display name to enum object. However, in the end neither is needed for your purpose. I kept those as they might prove interesting.

public enum GameFileKey
{
    ANTE( "posts ante" ),
    SEAT( "Seat" );

    private String displayName = null;

    private static final List < String > allDisplayNames = Arrays.stream( GameFileKey.values() ).map( GameFileKey :: getDisplayName ).toList();
    private static final Map < String, GameFileKey > mapOfDisplayNameToGameFileKey = Arrays.stream( GameFileKey.values() ).collect( Collectors.toUnmodifiableMap( GameFileKey :: getDisplayName , Function.identity() ) );

    GameFileKey ( String str ) { this.displayName = str; }

    public String getDisplayName ( ) { return this.displayName; }

    public static GameFileKey forDisplayName ( final String displayName )
    {
        return
                Objects.requireNonNull(
                        GameFileKey.mapOfDisplayNameToGameFileKey.get( displayName ) ,
                        "None of the " + GameFileKey.class.getCanonicalName() + " enum objects has a display name of: " + displayName + ". Message # 4dcefee2-4aa2-48cf-bf66-9a4bde02ac37." );
    }

    public static List < String > allDisplayNames ( ) { return GameFileKey.allDisplayNames; }
}

You can use a stream of the lines of your file being processed. Just FYI, not necessarily better than your code.

public class Demo
{
    public static void main ( String[] args )
    {
        Demo app = new Demo();
        app.demo();
    }

    private void demo ( )
    {
        try
        {
            Path path = Demo.getFilePathToRead();
            Stream < String > lines = Files.lines( path );
            lines.forEach(
                    line -> {
                        for ( GameFileKey gameKey : GameFileKey.values() )
                        {
                            if ( line.contains( gameKey.getDisplayName() ) )
                            {
                                switch ( gameKey )
                                {
                                    case SEAT -> this.seatAndPlayerAssignment( line );
                                    case ANTE -> this.playerJoinLate( line );
                                }
                            }
                        }
                    }
            );
        }
        catch ( IOException e )
        {
            throw new RuntimeException( e );
        }
    }

    private void playerJoinLate ( String line )
    {
        System.out.println( "line = " + line );
    }

    private void seatAndPlayerAssignment ( String line )
    {
        System.out.println( "line = " + line );
    }

    public static Path getFilePathToRead ( ) throws IOException
    {
        Path tempFile = Files.createTempFile( "bogus" , ".txt" );
        Files.write( tempFile , "apple\nSeat\norange\nposts ante\n".getBytes() );
        return tempFile;
    }
}

When run:

line = Seat

line = posts ante

Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154