What design pattern to use to improve this Java program

Question

I am writing a program to parse an XML file to get a specific tag value called SerialNum, which is contained in the header tag. The file is constructed as follows:

• contains 1 heading and 1 body
• The header can contain many SerialNum tags. We need to extract the value of the last tag.

I used the Stax parser to get the SerialNum value, and I wrote this code:

public String findIdValue(HttpServletRequest request) { String serialNumberValue = null; if(request != null){ ServletInputStream servletInstream; try { servletInstream = request.getInputStream(); XMLInputFactory factory = XMLInputFactory.newInstance(); XMLStreamReader xmlStreamReader = factory.createXMLStreamReader(servletInstream); //begin parsing if we get <Header> //end parsing if we get <Header/> or </Header> int event = xmlStreamReader.getEventType(); boolean enableToParse = false; boolean gotSerialNumber = false; boolean parseComplete = false; while( (xmlStreamReader.hasNext()) && (!parseComplete) ){ switch(event) { case XMLStreamConstants.START_ELEMENT: if("Header".equals(xmlStreamReader.getLocalName())){ //tag is header, so begin parse enableToParse = true; }else if(("SerialNum".equals(xmlStreamReader.getLocalName())) && (enableToParse) ){ //tag is serialNum so enable to save the value of serial number gotSerialNumber = true; } break; case XMLStreamConstants.CHARACTERS: //get serial number and end the parsing if(gotSerialNumber){ //get wsa and end the parsing serialNumberValue = xmlStreamReader.getText(); gotSerialNumber = false; } break; case XMLStreamConstants.END_ELEMENT: //when we get </Header> end the parse //when we get </SerialNum> reinit flags //when we get </Header> end the parse even we don't get a serial number if("Header".equals(xmlStreamReader.getLocalName())){ parseComplete= true; }else if("SerialNum".equals(xmlStreamReader.getLocalName())){ //reinit flag when we get </SerialNum> tag gotSerialNumber = false; } break; default: break; } event = xmlStreamReader.next(); } } catch (final XMLStreamException e) { //catch block LOG.info("Got an XMLStreamException exception. " + e.getMessage()); } catch (final IOException e1) { //catch block LOG.info("Got an IOException exception. " + e1.getMessage()); } } return serialNumberValue; } 

This code retrieves the desired value, but the quality of the code is not very good: it is easy to read and maintain. It contains a switch case, and if else blocks the nested while loop. What design pattern is used to improve the quality of the code?

Answer 1

I don't think your code needs a design pattern. But clean code will be very nice.

I agree with Louis F. in the comments: "As a first step, you can try to extract the code for your different cases of switching to separate methods and give them meaningful names . "

I think your code has too many comments. This is code smell . Example:

 if("Header".equals(xmlStreamReader.getLocalName())){ //tag is header, so begin parse enableToParse = true; } 

How to remove this comment and explain it with code ?

 if(isTagHeader(xmlStreamReader)) { enableToParse = true; } 

Just some thoughts ... your code doesn't look awful. But I think that the main thing here is not in design.

If you are interested in going deeper, I highly recommend Robert C. Martin (Uncle Bob), The Clean Code.