Quale modello di progettazione da utilizzare per migliorare questo programma Java

Question

Scrivo un programma per analizzare un file XML per ottenere un valore di tag specifico denominato SerialNum contenuto in un tag Header. Il file è costruito come di seguito:

• Contiene 1 intestazione e 1 Corpo
• l'intestazione può contiene molti tag SerialNum. Abbiamo bisogno di estrarre il valore dell'ultimo tag.

ho usato il parser Stax per ottenere il valore SerialNum, e ho scritto questo codice:

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; 
} 

Questo codice estratto il valore necessario, ma la qualità del codice non è molto buona: non è facile leggi e mantieni. Contiene un caso di commutazione e se altrimenti blocchi annidati in un ciclo while. Quale modello di progettazione utilizzare per migliorare la qualità del codice?

Answer 1

Non penso che il tuo codice abbia bisogno di un modello di progettazione. Ma un codice più pulito sarebbe davvero bello.

Sono d'accordo con il Louis F. suggerito nei commenti: "Come primo passo, si potrebbe provare a estrarre il codice dei diversi casi di switch in metodi separati e dare loro nomi significativi".

Penso che il tuo codice contenga troppi commenti. Questo è un code smell. Solo un esempio:

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

Che dire di rimuovere quel commento e explain it with code?

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

Solo alcuni pensieri ... il tuo codice non sembra terribile. Ma penso che il punto principale qui non riguardi i modelli di design.

Se sei interessato ad approfondire la questione consiglio vivamente il libro "Codice pulito" di Rober C. Martin (Zio Bob).