Optimization of an OSM-file parser

I need to code a parser that can parse OSM-files and I was wondering how to optimize it in terms of performance and readability. I have here two versions of basically the same parser. One homemade and the other using java for own XML-parser. Can someone help me?

Thanks a million in advance.

//HOMEMADE PARSER

public class Parser implements IParser {

    private final String fileName;
    private final List<Double> boundingBox = new ArrayList<>();
    private final HashMap<Long, OsmNode> osmNodeMap = new HashMap<>();
    private final HashMap<Long, OsmWay> osmWayMap = new HashMap<>();
    private final HashMap<Long, OsmRelation> osmRelationMap = new HashMap<>();

    public Parser(String filename) {
        this.fileName = filename;
    }

    @Override
    public void parse() {
        try {
            InputStream is = Parser.class.getResourceAsStream("/data/" + fileName);
            BufferedReader br = new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8));

            String line;

            while ((line = br.readLine()) != null) {
                //System.out.println(line);
                line=line.trim();
                if (line.startsWith("<bounds")) {
                    double minlat = getAttributeDouble(line, "minlat");
                    double minlon = getAttributeDouble(line, "minlon");
                    double maxlat = getAttributeDouble(line, "maxlat");
                    double maxlon = getAttributeDouble(line, "maxlon");

                    boundingBox.add(minlat);
                    boundingBox.add(minlon);
                    boundingBox.add(maxlat);
                    boundingBox.add(maxlon);
                }

                if (line.startsWith("<node")) {
                    long id = getAttributeLong(line, "id");
                    double lat = getAttributeDouble(line, "lat");
                    double lon = getAttributeDouble(line, "lon");

                    OsmNode osmNode = new OsmNode(id, lat, lon);

                    osmNodeMap.put(id, osmNode);
                }

                if (line.startsWith("<way")) {
                    long id = getAttributeLong(line, "id");
                    String subline;
                    List<OsmNode> nodes = new ArrayList<>();
                    HashMap<String, String> tags = new HashMap<>();

                    while ((subline = br.readLine()) != null) {
                        subline=subline.trim();

                        if (subline.startsWith("</way>")) {
                            break;
                        }

                        if (subline.startsWith("<nd")) {
                            long subid = getAttributeLong(subline, "ref");
                            OsmNode osmNode = osmNodeMap.get(subid);
                            nodes.add(osmNode);
                        } else if (subline.startsWith("<tag")) {
                            String k = getAttribute(subline, "k");
                            String v = getAttribute(subline, "v");
                            tags.put(k, v);
                        }

                    }

                    OsmWay osmWay = new OsmWay(id, nodes, tags);
                    osmWayMap.put(id, osmWay);
                }

                if (line.startsWith("<relation")) {
                    long id = getAttributeLong(line, "id");

                    OsmRelation currentRelation = osmRelationMap.get(id);
                    if (currentRelation == null) {
                        currentRelation = new OsmRelation(id, new ArrayList<>(), new HashMap<>());
                        osmRelationMap.put(id, currentRelation);
                    }

                    String subline;
                    ArrayList<Member> members = new ArrayList<>();
                    HashMap<String, String> tags = new HashMap<>();

                    while ((subline = br.readLine()) != null) {
                        subline=subline.trim();

                        if (subline.startsWith("</relation>")) {
                            break;
                        }

                        if (subline.startsWith("<member")) {

                            String type = getAttribute(subline, "type");
                            long refId = getAttributeLong(subline, "ref");
                            String role = getAttribute(subline, "role");

                            switch (type) {

                                case "node":
                                    OsmNode node = osmNodeMap.get(refId);
                                    if (node != null) {
                                        members.add(new Member(node, role));
                                    }
                                    break;

                                case "way":
                                    OsmWay way = osmWayMap.get(refId);
                                    if (way != null) {
                                        members.add(new Member(way, role));
                                    }
                                    break;

                                case "relation":
                                    OsmRelation refRel = osmRelationMap.get(refId);
                                    if (refRel == null) {
                                        refRel = new OsmRelation(refId, new ArrayList<>(), new HashMap<>());
                                        osmRelationMap.put(refId, refRel);
                                    }
                                    members.add(new Member(refRel, role));
                                    break;

                                default:
                                    break;
                            }
                        }

                        if (subline.startsWith("<tag")) {
                            String k = getAttribute(subline, "k");
                            String v = getAttribute(subline, "v");
                            tags.put(k, v);
                        }
                    }
                    currentRelation.setMembers(members);
                    currentRelation.setTags(tags);

                }
            }
        } catch (IOException e) {
            System.out.println(e.getMessage());
        }
    }

    public String getAttribute(String s, String key) {
        String pattern = key + "=\"";
        int start = s.indexOf(pattern);
        if (start == -1) {
            return null;
        }
        int valueStart = start + pattern.length();
        int valueEnd = s.indexOf('"', valueStart);
        return s.substring(valueStart, valueEnd);
    }

    public double getAttributeDouble(String s, String key) {
        String val = getAttribute(s, key);
        if (val == null) {
            return Double.NaN;
        }
        return Double.parseDouble(val);
    }

    public long getAttributeLong(String s, String key) {
        String val = getAttribute(s, key);
        if (val == null) {
            return 0L;
        }
        return Long.parseLong(val);
    }

    @Override
    public List<Double> getBoundingBox() {
        return boundingBox;
    }

    @Override
    public HashMap<Long, OsmNode> getOsmNodeMap() {
        return osmNodeMap;
    }

    @Override
    public HashMap<Long, OsmWay> getOsmWayMap() {
        return osmWayMap;
    }

    @Override
    public HashMap<Long, OsmRelation> getOsmRelationMap() {
        return osmRelationMap;
    }
}

//JAVAS OWN XML-PARSER

public class ParserStax implements IParser {

    private final String fileName;

    private final List<Double> boundingBox = new ArrayList<>();
    private final HashMap<Long, OsmNode> osmNodeMap = new HashMap<>();
    private final HashMap<Long, OsmWay> osmWayMap = new HashMap<>();
    private final HashMap<Long, OsmRelation> osmRelationMap = new HashMap<>();

    public ParserStax(String filename) {
        this.fileName = filename;
    }

    @Override
    public void parse() {
        try (InputStream is = ParserStax.class.getResourceAsStream("/data/" + fileName)) {
            if (is == null) {
                throw new IllegalArgumentException("Resource not found: /data/" + fileName);
            }

            XMLInputFactory factory = XMLInputFactory.newInstance();

            safeConfigure(factory);

            XMLStreamReader r = factory.createXMLStreamReader(is);

            Long currentWayId = null;
            List<OsmNode> currentWayNodes = null;
            HashMap<String, String> currentWayTags = null;

            Long currentRelId = null;
            ArrayList<Member> currentRelMembers = null;
            HashMap<String, String> currentRelTags = null;

            while (r.hasNext()) {
                int event = r.next();

                if (event == XMLStreamConstants.START_ELEMENT) {
                    String name = r.getLocalName();

                    switch (name) {
                        case "bounds" -> {
                            double minlat = getAttrDouble(r, "minlat");
                            double minlon = getAttrDouble(r, "minlon");
                            double maxlat = getAttrDouble(r, "maxlat");
                            double maxlon = getAttrDouble(r, "maxlon");

                            boundingBox.add(minlat);
                            boundingBox.add(minlon);
                            boundingBox.add(maxlat);
                            boundingBox.add(maxlon);
                        }

                        case "node" -> {
                            long id = getAttrLong(r, "id");
                            double lat = getAttrDouble(r, "lat");
                            double lon = getAttrDouble(r, "lon");
                            osmNodeMap.put(id, new OsmNode(id, lat, lon));
                        }

                        case "way" -> {
                            currentWayId = getAttrLong(r, "id");
                            currentWayNodes = new ArrayList<>();
                            currentWayTags = new HashMap<>();
                        }

                        case "nd" -> {
                            if (currentWayId != null) {
                                long ref = getAttrLong(r, "ref");
                                OsmNode n = osmNodeMap.get(ref);
                                if (n != null) currentWayNodes.add(n);
                            }
                        }

                        case "relation" -> {
                            currentRelId = getAttrLong(r, "id");

                            OsmRelation currentRelation = osmRelationMap.get(currentRelId);
                            if (currentRelation == null) {
                                currentRelation = new OsmRelation(currentRelId, new ArrayList<>(), new HashMap<>());
                                osmRelationMap.put(currentRelId, currentRelation);
                            }

                            currentRelMembers = new ArrayList<>();
                            currentRelTags = new HashMap<>();
                        }

                        case "member" -> {
                            if (currentRelId != null) {
                                String type = getAttr(r, "type");
                                long refId = getAttrLong(r, "ref");
                                String role = getAttr(r, "role");

                                switch (type) {
                                    case "node" -> {
                                        OsmNode node = osmNodeMap.get(refId);
                                        if (node != null) currentRelMembers.add(new Member(node, role));
                                    }
                                    case "way" -> {
                                        OsmWay way = osmWayMap.get(refId);
                                        if (way != null) currentRelMembers.add(new Member(way, role));
                                    }
                                    case "relation" -> {
                                        OsmRelation refRel = osmRelationMap.get(refId);
                                        if (refRel == null) {
                                            refRel = new OsmRelation(refId, new ArrayList<>(), new HashMap<>());
                                            osmRelationMap.put(refId, refRel);
                                        }
                                        currentRelMembers.add(new Member(refRel, role));
                                    }
                                    default -> {
                                    }
                                }
                            }
                        }

                        case "tag" -> {
                            String k = getAttr(r, "k");
                            String v = getAttr(r, "v");

                            if (currentWayId != null) {
                                currentWayTags.put(k, v);
                            } else if (currentRelId != null) {
                                currentRelTags.put(k, v);
                            } else {
                            }
                        }

                        default -> {
                        }
                    }
                }

                if (event == XMLStreamConstants.END_ELEMENT) {
                    String name = r.getLocalName();

                    switch (name) {
                        case "way" -> {
                            if (currentWayId != null) {
                                OsmWay way = new OsmWay(currentWayId, currentWayNodes, currentWayTags);
                                osmWayMap.put(currentWayId, way);

                                currentWayId = null;
                                currentWayNodes = null;
                                currentWayTags = null;
                            }
                        }

                        case "relation" -> {
                            if (currentRelId != null) {
                                OsmRelation rel = osmRelationMap.get(currentRelId);
                                if (rel == null) {
                                    rel = new OsmRelation(currentRelId, new ArrayList<>(), new HashMap<>());
                                    osmRelationMap.put(currentRelId, rel);
                                }

                                rel.setMembers(currentRelMembers);
                                rel.setTags(currentRelTags);

                                currentRelId = null;
                                currentRelMembers = null;
                                currentRelTags = null;
                            }
                        }

                        default -> {
                        }
                    }
                }
            }

            r.close();

        } catch (Exception e) {
            System.out.println(e.getMessage());
        }
    }

    private static void safeConfigure(XMLInputFactory factory) {
        try {
            factory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
        } catch (IllegalArgumentException ignored) {}
        try {
            factory.setProperty("javax.xml.stream.isSupportingExternalEntities", false);
        } catch (IllegalArgumentException ignored) {}
    }

    private static String getAttr(XMLStreamReader r, String name) {
        return r.getAttributeValue(null, name);
    }

    private static long getAttrLong(XMLStreamReader r, String name) {
        String v = getAttr(r, name);
        return (v == null) ? 0L : Long.parseLong(v);
    }

    private static double getAttrDouble(XMLStreamReader r, String name) {
        String v = getAttr(r, name);
        return (v == null) ? Double.NaN : Double.parseDouble(v);
    }

    @Override
    public List<Double> getBoundingBox() {
        return boundingBox;
    }

    @Override
    public HashMap<Long, OsmNode> getOsmNodeMap() {
        return osmNodeMap;
    }

    @Override
    public HashMap<Long, OsmWay> getOsmWayMap() {
        return osmWayMap;
    }

    @Override
    public HashMap<Long, OsmRelation> getOsmRelationMap() {
        return osmRelationMap;
    }
}

is there a good reason to not use existing library?

6 Likes

(re not using an existing library) it might be an “XML homework” question rather than a “how do I solve an OSM problem” one.

I’ve certainly used OSM for this sort of thing in the past - it’s a well defined format that hasn’t changed since basically forever, but the actual keys and values are extremely variable. Test data is as big or as little as you want and freely available.

If there is one good thing about LLMs - it is that “do my homework for me” cheating requests moved there. I guess some may still ask humans?

Though asking specific questions is fine, also in case of homework. Though “please rewrite my code for me” is hardly it. https://codereview.stackexchange.com/ may or may not work for that, at least partially (check their rules first).

I have the following comments.

  1. Do not use XML, use PBF instead.
  2. If you must use XML, never use a home-made XML parser as it will run into problems with a ton of special-case things (e.g. XML entities, character set issues, escaping). The approach with the Java XML library is the only sensible one (if you must use Java).
  3. When doing anything with OSM data that is more than just a homework, you cannot use an approach like yours that simply stores all data in memory; you need a streaming process. I believe the “Stax” parser will already do the right thing and not attempt to read the whole file at once, but your code then stores everything in memory which is not something that will work on a planet scale.
7 Likes

Alas, it wasn’t even a homework question. OP has just edited a spam link into the question - I removed it.

Edit: And the mods have suspended the user.

7 Likes