Refactoring är inte alltid så lätt…

Jag har stött på ett litet problem; Jag sitter och skall göra en liten refactoring av en klass och funderar på hur man bäst skall gå tillväga. Här är min main-kod i min klass:


try {
 initialize();
 if (this.orderId.equals("")) {
  createErrorResponseOrderNumberIsMissing();
  outputResponse.println(jsonReturn.toJSON());
  return;
 }
 this.orderDocument = allOrders.getDocumentByKey(this.orderId);
 if (this.orderDocument == null) {
  createErrorResponseOrderNotFound();
 } else {
  processOrderDocument();
  createResponseMessage();
 }
 outputResponse.println(jsonReturn.toJSON());
} catch(Exception e) {
.... exception handling snipped out of this example... not of interest right now! ...
}

Mitt problem med koden är att jag inte gillar att ha två olika exit-points i koden; du har en om ordernummer saknas och en annan för alla andra resultat. Jag skulle vilja ha endast en exitpoint – den som finns precis före catch-satsen. Men hur löser jag att det finns två villkor som måste uppfyllas varav det andra inte kan kontrolleras såvida inte en lookup utförs, där argumentet för lookupen baseras på det första värdet i villkoret?

Jag vill ju inte utföra lookupen om ordernummer inte finns angiven – då kommer jag att få en massa följdfel som inte är önskvärt. Jag skulle önska att lookupen hellre returnerade en exception om man anger ett sträng utan innehåll som argument. Det gör den. Det blir då NullPointerException. Jag skulle kunna fånga upp detta fel och hantera detta, men eftersom det kan bli NullPointerException var somhelst i koden så har jag ingen koll att just denna NPE handlar om att ordernummer saknas.

Hur löste jag detta?

Det var tanken på att hantera ett exception som ledde mig åt rätt håll. Eftersom lookupen utförs av en 3:e parts produkt som jag inte kan modifiera så beslöt jag mig för att ändra min klass ”ReturnMessage” – vilket är den lokala variabeln jsonReturn som sätts i initialize() till objektet ReturnMessage;

this.jsonReturn = new ReturnMessage();

Jag beslutade mig för att den inte fick initieras utan argument. Konstruktorn utan argument togs bort så att man måste ange ReturnMessage(String id) istället. I konstruktorn kan jag sedan slänga en egen (ny) exception; OrderIdMissingException.

Nästa förändring som var tvunget för att detta skulle fungera är att ändra hur man initierar jsonReturn. Detta lades till i initialize()-metoden:

this.jsonReturn = new ReturnMessage(getOrderId());

(Och nej – jag kan inte lägga in exception-kontrollen i ”getOrderId”-metoden, för denna används på fler ställen där det skall vara fullt legalt att använda en tom sträng och inget exception får då genereras!)

I main-funktionen kunde jag sedan lägga till catch-sats för detta exception och skapa felmeddelandet där istället:


catch(OrderIdMissingException exception) {
 createErrorResponseOrderNumberIsMissing();
}

Jag flyttade också outputResponse.println(jsonReturn.toJSON()); till finally-satsen. Så här blev koden till slut:


try {
 initialize();
  this.docOrder = allOrders.getDocumentByKey(jsonReturn.orderId);
  if (this.docOrder == null) {
   createErrorResponseOrderNotFound();
  } else {
   processOrderDocument();
   createResponseMessage();
  }
 } catch(OrderIdNotSuppliedException missingOrderIdException) {
  createErrorResponseOrderNumberIsMissing();
 } catch(Exception e) {
... exclueded exceptionhandling from example...
 } finally {
  outputResponse.println(jsonReturn.toJSON());
 }

Eller är det någon som har förslag på en bättre lösning?

Annonser

Kommentera

Fyll i dina uppgifter nedan eller klicka på en ikon för att logga in:

WordPress.com Logo

Du kommenterar med ditt WordPress.com-konto. Logga ut / Ändra )

Twitter-bild

Du kommenterar med ditt Twitter-konto. Logga ut / Ändra )

Facebook-foto

Du kommenterar med ditt Facebook-konto. Logga ut / Ändra )

Google+ photo

Du kommenterar med ditt Google+-konto. Logga ut / Ändra )

Ansluter till %s

%d bloggare gillar detta: