XmlControl class

Question

I've written this code that connects to a .xml file at a given url, parses it and outputs the values of some specific elements to the console.

I want to write reusable and appropriate code accordingly to OOP standards, but I'm not sure where to start. How to improve this code?

public class XmlControl {   

    public static void main(String[] args) throws IOException, ParserConfigurationException, SAXException {

        try {
        URL xmlUrl = new URL("http://localhost/file.xml");
        URLConnection connection = xmlUrl.openConnection();

        DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
        DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
        Document doc = dBuilder.parse(connection.getInputStream());

        doc.getDocumentElement().normalize();
        NodeList nList = doc.getElementsByTagName("Start");
        NodeList kList = doc.getElementsByTagName("Article");



        for (int temp = 0; temp < nList.getLength(); temp++) {

        Node nNode = nList.item(temp);



        if (nNode.getNodeType() == Node.ELEMENT_NODE) {

            Element eElement = (Element) nNode;

                        for(int count = 0; count < eElement.getElementsByTagName("Article").getLength(); count++) {

                            Node kNode = kList.item(count);
                            Element kElement = (Element) kNode;

                            String title = eElement.getElementsByTagName("Title").item(count).getTextContent();

                            if(!"".equals(title)) {

                                System.out.println( title );                

                            }


                        }

        }
    }


        } catch (IOException | ParserConfigurationException | SAXException | DOMException e) { // Exception e
    // e.printStackTrace();
            System.out.println("Check your connection");
    }
    }

}

Answer 1

public class XmlControl {   

    public static void main(String[] args)

The first problem with regards to OOP here, is that the entirety of your code is written in static context, in the program's entry point - you're not creating any objects.

The entry point (the static void main method) shouldn't contain any of the application's logic. Instead, it should create the objects that do.

I'd expect the entry point to be in a class like this:

public class MyApplication {   

    public static void main(String[] args)

And then you would have an XmlControl class with a constructor that takes the class' dependencies as parameters; this will help move some of the responsibilities out of the method, which is doing too many things.

Then, implement a separate method for each thing you need to do - a method that does one thing would only ever need to change if it needs to be doing something else! By breaking down the problem into small steps, you're also making the responsibilities clearer:

public class XmlControl {

    private final DocumentBuilderFactory builderFactory;

    public XmlControl(DocumentBuilderFactory builderFactory) {
        this.builderFactory = builderFactory;
    }

    public void parseDocument(URL url) { // todo: use a meaningful name
        URLConnection connection = getUrlConnection(url);
        if (connection == null) {
            // invalid url?
            return;
        }

        Document document = getNormalizedDocument(connection);
        if (document == null) {
            // invalid response?
            return;
        }

        // todo: loop through elements
    }

    private URLConnection getUrlConnection(URL url) {
        try {
            return url.openConnection();
        }
        catch(...) {
            // log/output connection exception
            return null;
        }
    }

    private Document getNormalizedDocument(URLConnection connection) {
        try {
            DocumentBuilder builder = builderFactory.newDocumentBuilder();
            Document result = builder.parse(connection.getInputStream());
            result.getDocumentElement().normalize();
        }
        catch(...) {
            // log/output exception
            return null;
        }
    }
}

The actual loop body would belong in its own method, in order to respect the level of abstraction of parseDocument: good code that is easy to read and to maintain, is written in methods that have a consistent level of abstraction: the implementation details of exactly how the loop operates would clash with the nice abstractions of getNormalizedDocument and getUrlConnection - to be consistent you'd need to implement that loop logic inside something like a walkDocumentNodes method.

Good code that's easy to read and to follow, also makes use of consistent indentation. Whenever you see this:

}
}

Or this:

                    }

    }

It's a big red flag saying that something has gone terribly wrong.

Anyway, back into void main, you'd end up with something pretty simple - like this:

public class MyApplication {   

    public static void main(String[] args) {
        XmlControl control = new XmlControl(new DocumentBuilderFactory());
        URL xmlUrl = new URL("http://localhost/file.xml");
        control.parseDocument(xmlUrl);
    }
}

Notice how the XmlControl class doesn't need to care about the actual URL being processed.