Efficient parsing?

http://pastebin.java-gaming.org/224c73c2b59
This is my Compatiblity.shader file, and I’m combining the .vert and .frag shaders into one file. My current code for parsing is:


String shader=AssetHandler.readFile(path+".shader");
            String version=shader.substring(
                    (shader.indexOf("<VERSION>")==-1) ? 0 : shader.indexOf("<VERSION>")+9,
                    (shader.indexOf("</VERSION>")==-1) ? 0 : shader.indexOf("</VERSION>")).trim();
            vertShader=createShader(
                    ((version.isEmpty()) ? "" : ("#version "+version))
                    + shader.substring(shader.indexOf("<VERT>")+6, shader.indexOf("</VERT>")), GL_VERTEX_SHADER);
            fragShader=createShader(
                    ((version.isEmpty()) ? "" : ("#version "+version))
                    + shader.substring(shader.indexOf("<FRAG>")+6, shader.indexOf("</FRAG>")), GL_FRAGMENT_SHADER);

Is this ok? I feel like it could be a lot cleaner and nicer. I’m going to add a tag next.

If your going to be doing some serious parsing, might I suggest using JavaCC. I find it to be a wonderful tool that is as simple as it needs to be. If your unaware of it, you write a syntax for it and it will generate the source code for a java parser that will parse your input and perform any actions you add in.

Couple of suggestions you may want to consider:

  1. Encapsulate the code that extracts the text from between the XML tags into a helper method:
  • you can then write a unit-test to make sure that chunk works
  • the code that’s doing the actual parsing will then be a lot cleaner (and therefore less buggy / easier to maintain)
  • and you won’t be cut-and-pasting and hard-coding the end index when you add further functionality, further reducing chances of cock-ups :slight_smile:
  1. Avoid duplicating the indexOf() call - OK the performance of doing it twice is probably hardly game-breaking but it’s good practice.

  2. Is there a reason you’re not just throwing exceptions for the cases where the file is not valid (no tag)? You’ll have to check elsewhere that the vertex/fragment shader is potentially empty which (presumably) is a pretty fatal scenario, so you might as well handle that explicitly with an exception.

-stride