В коде есть ряд улучшений.
Я постараюсь обратиться к некоторым из них, наиболее актуальным.
Наиболее актуально. Для этого следует использовать jsp / servlet / beans.
Но так как код не структурирован таким образом, я не буду объяснять дальше. Лучше объяснить улучшения сервлета как есть.
Метод делает слишком много вещей в одном месте. Чтобы сделать его лучше, необходимо несколько служебных методов, хотя это не улучшает производительность, безусловно, помогает поддерживать код и проясним, какова цель метода (позволяя модифицировать код для улучшений.
Некоторые части кода вообще не меняются. Они должны быть объявлены как константы. Например, есть три комбинации, которые никогда не меняются. Таким образом, они должны быть постоянными, поэтому не имеет значения, использует ли код строку «+», потому что они вызываются только один раз.
Открыть вложенные результаты. Перед открытием и использованием ResultSet предыдущий должен быть обработан и закрыт. Таким образом, соединения имеют возможность сэкономить ресурсы БД.
qw.DBquery Возможно, реальная реальная проблема в этом коде - это объект. Я не знаю, как выглядит код, но, вероятно, он не использует пул соединений и не закрывает соединение при каждом выполнении запроса. Это может потенциально означать, что для данного кода может быть открыто до 5 одновременных соединений. Пока нужен только один. В худшем случае в следующий раз (если соединения не закрыты) могут быть использованы еще 5 новых соединений, когда, вероятно, одно соединение может выполнять всю работу в течение нескольких часов. Мы никогда не узнаем.
Помимо замены конкатенации строк с использованием "+", я думаю, что использование ресурсов БД - это то, что снижает производительность этого сервлета.
Наконец, вот моя попытка сделать главный рефакторинг кода.
Надеюсь, это поможет лучше понять, как сделать Java-приложение. Я не скомпилировал его, поэтому он не сработает. Это делается только для того, чтобы показать, как можно исправить вышеперечисленные элементы.
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 =
"<table class=inner><thead><colgroup><col id='col1'>"+
"<col id='col2'><col id='col3'><col id='col4'><col id='col5'></colgroup>" +
"<tr class='enetBlue'><th>Qty</th><th>Part Num</th><th>Description</th>"+
"<th>Unit Cost</th><th>Unit Price</th></tr></thead><tbody>";
// 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") );
Тогда, хотя это определенно неправильный путь, сервлет может определить некоторые bean-компоненты, чтобы помочь ему выполнить работу, мы могли бы создать такие классы следующим образом:
// 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<Document> documents = new ArrayList<Document>();
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("<tr class='"+clazz+"'>"+
"<td>"+doc.qty+"</td>\n"+
"<td>"+doc.part+"</td>\n"+
"<td>"+doc.desc+"</td>\n"+
"<td>"+doc.cost+"</td>\n"+
"<td>"+doc.price+"</td></tr>\n");
if( needsInsertForm( clazz ) ) {
resultingTable.append( getInsertForm( document ) );
}
resultingTable.append("</tbody></table>");
}
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;
}
// Creates the inser form for the given document
private static String getInsertForm( Document document ) {
StringBuilder form = new StringBuilder();
form.append("<tr ><td colspan=5><table border=1><tr><td colspan=2>\n");
form.append("<form name=masinsert"+document.id+" method=get action=MASInsert>\n");
form.append("<input type=hidden name=\"partnumber"+document.id+"\" value=\""+document.part+"\">\n");
form.append("<input type=hidden name=\"itemdescription"+document.id+"\" value=\""+document.desc+"\">\n");
form.append("<input type=hidden name=\"itemcost"+document.id+"\" value=\""+document.cost+"\">\n");
form.append("<input type=hidden name=\"itemlistprice"+document.id+"\" value=\""+document.price+"\">\n");
form.append("</td><tr>\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("<tr><td colspan=2><input id='m"+document.id+"' type=\"button\" onclick=\"masinsert('"+ document.id +"')\" value=\"Add to MAS\"></td></tr>");
form.append("</table>\n";
form.append("</form>\n";
form.append("</td></tr>");
return form.toString();
}
// 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);
}
Есть два метода, которые здесь не включены. Они все еще грязные, но они помогают создавать константы. Таким образом, вместо создания этих строк каждый раз, они просто создаются один раз в течение жизни сервлета.
Полный код здесь http://pastebin.com/f2ad1510d
Надеюсь, это поможет.