import java.sql.ResultSet; import java.util.List; import java.util.ArrayList; // etc etc. etc public class SomeServlet extends HttpServlet { // ALL THE FOLLOWING ARE CONSTANTA IN THE SERVLET. // THEIR VALUE DOES'T CHANGE DURING EXECUTION private static final String SELECT_DOCUMENT_QUERY = "SELECT \n" + " id,LineType, QtyTotal, ManufacturerPartNumber, Description, UnitCost,UnitPrice \n" + " FROM DocumentItems \n" + " WHERE DocID=%s order by linenumber "; private static final String HTML_HEADER_TABLE =""+ "" + ""+ ""; // These constants are the HTML Commbo for the given queries. // If they are initialized once at the beginning will increase // the servlets performace considerabily. private static final String UNIT_MEASURES_COMBO = getCombo( "Unit Measure", "UnitMeasKey", getListFrom( "select * from tciUnitMeasure where companyid like 'ENS' ") ); private static final String ITEM_CLASS_COMBO = getCombo( "Purchase Product Line", "PurchProdLine", getListFrom( "select * from timPurchProdLine where companyID = 'ENS' ") ); private static final String CLASS_KEY_COMBO = getCombo( "Item Class :", "itemclasskey", getListFrom( "select * from timItemClass where companyID = 'ENS' order by itemclassname desc") ); // Use a class to separete domain objects from presentation // This class becomes handy do pass data through methods. class Document { int lineType; int qty; String part; String desc; float cost; float price; String id; } // simple clas that holds a key/value pair class ComboPair { String key; String value; public ComboPair( String key, String value ) { this.key = key; this.value = value; } } /* * Finally the fixed code. * * Basically a method should do only one thig. If the method is doing a lot of things * several functionality at the same time, it should delegate the details of the subfunctionality * to another function. This way each function or method is easier to read/maintain/understand. * This method should read like this: * * For a given docId, * - query the database and display details of the document * - if the document is not present in mas? * - create a form to inser it * * differntiate each record with different style. */ private String getDetails(String doc){ // Close each result set before openning a new one. ResultSet rs = qw.DBquery( String.format( SELECT_DOCUMENT_QUERY, doc )); List documents = new ArrayList(); while(rs.next()){ documents.add( createDocumentFrom( rs )); } // Iterate through StringBuilder resultingTable = new StringBuilder( HTML_HEADER_TABLE ); boolean isEven = false;// starts as odd for( Document doc : documents ) { String clazz = getClassFor( doc , isEven ); isEven = !isEven; resultingTable.append(""+ "\n"+ "\n"+ "\n"+ "\n"+ "\n"); if( needsInsertForm( clazz ) ) { resultingTable.append( getInsertForm( document ) ); } resultingTable.append("
QtyPart NumDescriptionUnit CostUnit Price
"+doc.qty+""+doc.part+""+doc.desc+""+doc.cost+""+doc.price+"
"); } return table; } /** * This methods craates an instance of "Document". * Instead of mixing the fetch code with the render code * this method allows to separete the data from its presentation */ private Document createDocumentFrom( ResultSet rs ) { Document document = new Document(); document.lineType = rs.getInt("LineType"); document.qty = rs.getInt("QtyTotal"); document.part = rs.getString("ManufacturerPartNumber"); document.desc = rs.getString("Description"); document.cost = rs.getFloat("UnitCost"); document.price = rs.getFloat("UnitPrice"); document.id = rs.getString("id"); return document; } // Computes the correct css class for the given document. private static String getClassFor( Document document, boolean isEven ) { String clazz = "red"; switch( document.lineType ) { case 2: case 3: case 4: clazz ="yellow"; } if( document.qty == 0 ) { clazz = "yellow"; } ResultSet rs = mas.DBquery( String.format("select itemkey from timitem where itemid = '%s'", document.part); while (rs.next()) { clazz = isEven? "even" : "odd"; } rs.close(); return clazz; } // This is an utility method that reads bettern when used in the // main method. // if( needsInsertForm( clazzz ) ) { // is much clearer private static boolean needsInsertForm( String clazz ) { return "red".equals(clazz); } // Creates the inser form for the given document private static String getInsertForm( Document document ) { StringBuilder form = new StringBuilder(); form.append("\n"); //---------------------------------------------------------------------------------------------------------------------------- //get unit measure key form.append(UNIT_MEASURES_COMBO); //build ItemClass options from mas: Puchase ProductLine form.append(ITEM_CLASS_COMBO); //build item classkey options form.append( CLASS_KEY_COMBO); //---------------------------------------------------------------------------------------------------------------------------- form.append(""); form.append("
\n"); form.append("
\n"); form.append("\n"); form.append("\n"); form.append("\n"); form.append("\n"); form.append("
\n"; form.append("\n"; form.append(""); return form.toString(); } // Generates an html combo // is a bit messy, but does the job. In total this function is called only 3 times during // this servlet life. private static final String getCombo( String columnName, String comboName, List items ) { // Don't need to worry too much here about string concatenation, this is invoked only once. StringBuilder builder = new StringBuilder( "\n"+ " "+columnName+"\n"+ " \n"+ " \n"); return builder.toString(); } // Utlity method that builds a list of key=value objects from the given query. // This is used to query the catalogs. private static final List getListFrom( String query ) { ResultSet rs = mas.DBquery( query ); List list = new ArrayList(); try { while(rs.next()) { list.add( new ComboPair( rs.getString("UnitMeasKey"), rs.getString("UnitMeasID"))); } } finally { if( rs != null ) try { rs.close(); } catch( SQLException sqle ){} } return list; } }