Java ist auch eine Insel

Montag, Dezember 10, 2007

Schlechter Quellcode von NeuralBuild (NeuralLimits)

Über den Artikel http://entwickler.com/itr/news/psecom,neu,1,id,39010,nodeid,82.html bin ich auf einen Generator aufmerksam geworden, der automatisch DOAs und Transfer-Objekte erzeugt. Leider ist doch der generierte Quellcode wenig überzeugend. En Besipiel:

    public Integer insertCompany(Company criteriaCompany)
        throws NeuralBuildJavaSampleDAOInsertException {
        Integer keyVal = new Integer(0);
        con = connMgr.getConnection("neuralbuildjavasample");
        int setindex = 1;
        String vSQL = constructInsertPreparedStatement(criteriaCompany);
        try {
            PreparedStatement ps = con.prepareStatement(vSQL,
                    PreparedStatement.RETURN_GENERATED_KEYS);
            if (criteriaCompany.CompanyID_isSet) {
                ps.setInt(setindex, criteriaCompany.getCompanyID().intValue());
                setindex++;
            }
            if (criteriaCompany.CompanyName_isSet) {
                ps.setDate(setindex,
                    (java.sql.Date) criteriaCompany.getCompanyName());
                setindex++;
            }
            ps.executeUpdate();
            ResultSet rs = ps.getGeneratedKeys();
            if (rs.next()) {
                keyVal = Integer.valueOf(rs.getObject(1).toString());
            }
            rs.close();
            rs = null;
            ps.close();
        } catch (Exception e) {
            connMgr.freeConnection("neuralbuildjavasample", con);
            throw new NeuralBuildJavaSampleDAOInsertException("Insert Failed " +
                e.getMessage(), e);
        }
        connMgr.freeConnection("neuralbuildjavasample", con);
        return keyVal;
    }
  • new Integer(0) wird immer gebaut, auch wenn es später überschrieben wird.
  • finally fehlt: So ist das close() nicht garantiert. (Nicht sooo schlimm, da bei Freigabe der Connection auch ResultSet und XXXStatement freigegeben werden.)
  • Quellcode-Duplizierung bei connMgr.freeConnection("neuralbuildjavasample", con);. Das gehört auf jeden Fall in finally{}
  • Was soll den rs = null; bewirken? Den GC hilt das bei lokalen Variablen nun wirklich nicht.
  • catch (Exception e) muss wirklich sein? Ode doch catch (SQLException e)?
  • Wenn die Rückgabe ein Integer-Objekt ist, warum dann "Integer.valueOf(rs.getObject(1).toString());" schreiben? Das funktioniert zwar, aber wenn getObject() schon in Integer ist, muss man nicht erst in ein String und dann wieder zurück konvertieren.
  • Sollte man wirklich einfach nur 0 zurückliefern, wenn man keinen Schlüssel bekam?

Die anderen Methoden habe auch so ihre Macken, so mit der Namensgebung.

  • CachedRowSet Rs = null; Groß-/Kleinschreibung!
  • Gemische Groß-/Kleinschreibung ist Standard, nicht so bei setindex.
  • Unterstriche sind von Sun in der Namenskonvention nur bei Konstanten vorgesehen, aber nicht etwa bei CompanyID_isSet
  • Ist die Namensgebung so gut, wenn man liest
    ps.setDate(setindex, (java.sql.Date) criteriaCompany.getCompanyName() );
    Also setDate() auf ein Company-Name?
  • Die API-Doku sollte nicht schreiben: "Delete an object from the database." sondern laut Suns-JavaDoc-Vorgaben: "Deletes an object from the database."

AddThis Social Bookmark Button

posted by Christian Ullenboom at Montag, Dezember 10, 2007

2 Comments:

  • Ich hab hier eine Sache die mich schon länger plagt, und zwar beschwerst du dich über:
    "catch (Exception e) muss wirklich sein? Oder doch catch (SQLException e)?"


    Es ist zwar nicht ganz so sauber, aber ich find es oft ganz nett wenn man dadurch automatisch auch NullPointer Excpetions (unser ständiger Freund und Begleiter) und anderes unerwartetes auch abfängt.

    Hier kann ja eh ausser SQLExcpetion nicht viel passieren, aber oft stößt man dann auf konstrukte wie

    try{ ... }
    catch( SQLException se ){...}
    catch( NullPointerException ne ){ ...}
    catch( FileNotFoundException fnfe ){ ... }
    catch( IOException ioe ){ ... }
    catch( Übermüdungsexception üe ){ ... }

    Eclipse erzeugt das ja beispielsweise mit unglaublicher hingabe immer automatisch und mir stellts dann immer die Grausbirne auf und ich mach daraus ein
    try{}
    catch( Exception e ){ }

    Wie gesagt, es ist nicht sauber aber schöner zu lesen und in den meisten Fällen ist es vollkommen egal welcher Teil fehlschlägt (es sei denn irgendwas muss rückgängig gemacht werden).

    Wie praktiziert ihr anderen diese Multi-Exceptions?


    gruesse, hansi.

    By Anonymous kraxifax, at Dezember 10, 2007 3:18 PM  

  • Es gibt mindestes zwei Beispiele, in denen ich das auch zu einer catch ( Exception e ) zusammenfasse:

    * UIManager.setLookAndFeel
    * Reflection-Aufrufe

    Beide erzeugen mir so viele Ausnahmen, die für mich nur das Eine bedeuten: Klappt nicht! Also Fehler.

    Doch *grundsätzlich*, würde ich das nicht so machen, auch tendenziell nicht in einem Generator. RuntimeExceptions (NPE ist da das geringste Problem) finde ich eine ganz wertvolle Info, die ich ungern als checked SQL- oder sonstwas-Exeption vernudelt haben möchte. Wenn ich mir moderne Frameworks, wie Spring oder Java EE 5 anschaue, wird dort ausschließlich mit RuntimeException gearbeitet, und die fängt eine catch ( Exception e ) ebenfalls brutal ab. Wenn zum Beispiel das Framework aufgrund einer unchecked Exception eine Transaktion abbricht, wir diesen Fehler aber schon „weggefangen“ haben, ist das ein echtes Problem.

    Ich sehe für eine vernünftige Behandlung mehrere Möglichkeiten:

    * Warten auf Java 3000, da dort sicherlich so etwas wie catch ( IOException, SQLException e ) definiert wird.
    * Eine gemeinsame handle()-Funktion zu definieren, die man dann aus den catch-Handlern ansteuert.
    * Ein catch ( Exception e ) und dann ein instanceof Test auf die erwarteten Exceptions mit einer re-thow, wenn der Typ nicht bekannt ist.

    By Blogger Christian Ullenboom, at Dezember 13, 2007 10:20 AM  

Kommentar veröffentlichen

<< Home