You learn something new every day. I never thought I’d post an auto-generated CVS log here, but it contains so many gems in so few lines of comments, so I had to. Keep in mind that the underlying code was written six years ago, in an era where everything had to be XML. If you don’t share my sense of humor, here are two tips: “EO” and “resubstitute the existing text”.

I’ve changed the name of the class, modification dates, and committers.

/*
* $Log: ObscuredEO.java,v $
* Revision 1.6 2005/04/17 11:16:37 mr_x
* reintroduced.
*
* Revision 1.4 2005/03/19 14:03:12 mr_x
* added type constants.
*
* Revision 1.3 2004/11/26 11:29:10 mr_x
* Added the function restartText(), this method can be used to resubstitute the existing text with a new text.
*
* Revision 1.2 2004/11/23 15:44:18 mr_x
* modified this class, so that it inherits from the XmlMessage class, like all other EO-classes in the message-package. It now overrides the method toXMLString to generate the complete tag plus its enclosed message.
*
* Revision 1.1 2004/10/14 14:59:17 mr_y
* First version
*


When I first saw this I knew it broke som kind of record. I’d say the code below manages to max the ratio accidental complexity / cyclomatic complexity. And yes, it does contain a bug too… And yes, iterator.remove() was available when this was conceived.
As always, I’ve renamed a few things to make the hide the source. “Entity” has a more descriptive name in the real code.

private void addEntityToSession(Entity ppto, HttpServletRequest request) {
     // Create an entity object to set as an attribute in the session
     EntityAdapter entity = new EntityAdapter(ppto);

     // Set the entity attribute in the session
     request.getSession().setAttribute("entity", entity);

     // Creates an array in which entities will be placed and displayed in a dropdownlist
     ArrayList historyArray = ((ArrayList) request.getSession().getAttribute("history"));

     if (historyArray != null) {

         // Removes the entity at position number 10 in historyArray
         if (historyArray.size() == 10) {
             historyArray.remove(9);
         }

         Collection tmpList = new ArrayList();

         // Looping through the historyArray and places any entity that are  equal to the
         // one the will be added
         for (Object aHistoryArray : historyArray) {
             if (((EntityAdapter) aHistoryArray).getId() == entity.getId()) {
                 tmpList.add(aHistoryArray);
             }
         }

         // Looping through the historyArray and removes entities that were added above in tmpList
         for (Object aTmpList : tmpList) {
             historyArray.remove(aTmpList);
         }
         historyArray.add(0, entities);
     }
}

At first glance this doesn’t seem that bad. Read again! As always, the code has been modified to hide its origin.

CREATE OR REPLACE PROCEDURE insert_file(
    I_FILENAME             VARCHAR2,
    I_CHECKSUM             VARCHAR2
)
AS
CURSOR c1 IS SELECT *
FROM filename
FOR UPDATE OF checksum;

L_CHECKSUM_COUNT NUMBER := 0;
BEGIN
    FOR rec IN c1 LOOP

        SELECT COUNT(*) INTO L_CHECKSUM_COUNT
        FROM filename
        WHERE checksum = I_CHECKSUM OR filename = I_FILENAME;

        IF L_CHECKSUM_COUNT > 0 THEN
            raise_application_error(-20120,'File already exists '|| I_FILENAME ||'.');
        END IF;

        INSERT INTO filename(id, filename, checksum)
                      VALUES(seq_id.nextval, I_FILENAME, I_CHECKSUM);
        EXIT;
    END LOOP;
END;

This requires no explanation:

/* Private log object */
private LogHelper logger = new LogHelper(this.getClass());

/* Text resources */
private static ResourceBundle bundle = ResourceBundle.getBundle("resources.Message");
private static String issueComment = bundle.getString("message.issue.comment");

// Default to 100
int batchSize = 100;

Jag blir så glad när ens kloka kolleger i branschen kommer på så tunga saker som nya designprinciper. Det är inte ofta!

I koden nedanför illustreras hur Struts har använts som språngbräda för att skapa en helt nytt sätt att designa interna interface på, nämligen design by similarity.

 För att problemet ska bli uppenbart måste dock sägas att íngen av metodernadisplayCustomerForm, processNewCustomer, processExistingCustomer eller searchSsnnågonsin använder parametrarna mapping, request och actionForward. Författaren tyckte dock att koden blev bättre om alla metoder “såg typ lika ut”, fastän de gör helt olika saker. Eller så var han kanske en ny överlagring på spåren…

 Metoden nedan innehåller också några andra bonusar för oss som gillar WTFs

 public ActionForward executeAction(ActionMapping mapping, ActionForm form, HttpServletRequest request,
        HttpServletResponse response, TransferObject to) throws Exception {

    if (customerForm.getOperationFlag().equals("1")) {
        actionForward = displayCustomerForm(customerForm, mapping, request, session, messages);
    } else if (customerForm.getSelectedAction() != null
        && customerForm.getSelectedAction().equals("join_us")) {
        actionForward = processNewCustomer(customerForm, mapping, request, session, messages, actionForward);
    } else if (customerForm.getSelectedAction() != null
        && customerForm.getSelectedAction().equals("already_joined")) {
        actionForward = processExistingCustomer(customerForm, mapping, request, actionForward, messages);
    } else if (customerForm.getSelectedAction() != null
        && customerForm.getSelectedAction().equals("findBySsn")) {
        actionForward = searchSsn(customerForm, mapping, request, actionForward, messages);
    } else {
        customerForm.setSelectedAction("join_us");
        customerForm.setDisplayMandateApprovedByPlayer(true);
        customerForm.setMandateApprovedByPlayer(true);
    }
    saveMessages(request, messages);
    return actionForward;
}

Att styra programflödet är inte alltid lätt. De flesta är väl överens om att det är jättedåligt att använda exceptions för att styra flödet, men författaren av denna snutt håller nog inte med, och har dessutom tagit nästa steg genom att styra med serverns context och status på rollbacken: 

try {
    // ... Hundratals rader kod
} catch (ThingException pe) {
    logger.info("Could not create customer for this thing with thing_id: " + thing.getThingId() + ", " + pe.getMessage());
    noOfImportedInvalidThings++;
    if (ctx.getRollbackOnly()) {
        throw new BatchJobException("Transaction was rolled back in sub routine for thing_id: " + thing.getThingId(), pe);
    }
}

Har bytt ut några variabelnamn för att skydda den skyldige. “ThingException” och “thing” heter något annat i verkligneten.

Letade på nätet efter lite info om taggar i Subversion. Hittade bland annat detta guldkorn i en Subversion HOWTO. Om länken blir trasig vill jag inte förlora detta fina stycke, så jag citerar också:

“There are no differences between branches and tags in Subversion. In fact, Subversion doesn’t even know the concept of branches or tags: everything is a file or a directory for it.

A branch or a tag is nothing more than a copy of your files under a different path. It is an O(1) operation in time and disk space, so there’s no harm copying everything.”

Ibland är det otroligt lärorikt att läsa andras kod för att lära sig objektorientering. En del medlemsnamn i skissen nedan är ändrade, dock ej klassnamnen! 

Det är inte alltid lätt att skriva i en blogg som inte går ut på att trivialt sammanfatta eller kommentera någonting man har läst. Därför behöver även en blogg lite semester. Ny termin – nya möjligheter. Vi mjukstartar med en snippet jag har sparat för just ett sådant tillfälle. Har så klart ändrat lite på variabel- och metodnamnen… 

Kommentar överflödig!

public void testFindPlayerIdForSomething() {
    final Long expectedPlayer = (long) 12345;
    SomeServiceImpl someService = new SomeServiceImpl();
    someService.setSomeDao(new SomeDaoImpl() {
        public List<Long> findPlayerIdForSomething(Obfuscated ticket) {
            List<Long> results = new ArrayList<Long>();
                results.add(ticket);
                return results;
        }
    });
    Long result = someService.findPlayerIdForSomething(null).get(0);
    assertEquals(expectedPlayer, result);
}

Det jag träffade på igår är egentligen mer än en ren WTF. Om man programmerar så här, kan man lätt göra sig oumbärlig på en arbetsplats. Med fina ord kan vi kalla följande konstruktion Temporal coupling via global variables in persistent storage (för att göra en claim for fame). En annan rubrik skulle kunna vara “Work protection^2″

Tänk en PL/SQL-procedur på ca 500 rader som tar 5 in-parmetarar, anropar ytterligare några funktioner och procedurer, och vars egentliga flöde och happy path inte är helt självklara.
CREATE OR REPLACE PROCEDURE do_a_lot 
(
L_A IN NUMBER,
L_B IN NUMBER,
L_C IN DATE,
L_D IN VARCHAR2,
L_E IN DATE
) 
AS
-- Nåt tiotal lokala variabler
BEGIN 
-- 50 Rader kod
SELECT INTO a_very_imporant_flag COUNT(*) FROM 
(
  -- Relativt enkel select på något 10-tal rader innehållande
  WHERE table_a.attribute=table_x.attribute
  AND table_a.anotherattribute=L_A
  AND table_a.attribute=table_b.attribute
  ...
)
-- Sedan igen:
SELECT INTO a_very_imporant_flag COUNT(*) FROM 
(
  -- Relativt enkel select på något 10-tal rader innehållande
  WHERE table_a.attribute=table_y.attribute
  AND table_a.anotherattribute=L_A
  AND table_a.attribute=table_b.attribute
  ...
)
-- 200 snarlika rader
-- 200 rader affärskritisk logik som beror på resultat från ovan
END;


Jag har strukit under två rader ovan. Dessa innehåller den dolda parametern! Det man nämligen måste veta för att ta rätt väg genom proceduren är table_b.attribute per default är NULL, vilket är helt fel. Istället ska man sätta det värdet innan man anropar proceduren, så att det förväntade resultatet uppnås.

Det här kanske blev lite rörigt förklarat, men sammanfattnigsvis:

Den temporala kopplingen ligger i att man måste anropa någonting som sätter table_b.attribute till icke-NULL, som också är den hemliga globala variabeln.

Sätter man detta i system är man behövd för alltid.