ahora vamos a analizar código, el siguiente es un código extraído de un programador de
una empresa donde estuve, digamos que este programador se adueño del código que no
era propiedad suya, sin embargo debido a que su orgullo no le permitió preguntar como
era correcto, siempre ocasiono serios problemas, es mejor preguntar cuando uno este en duda,
o apoyarse de los demas programadores, siempre es mejor perder 2 o 3 minutos que
se traducen en horas ahorradas,la mejor filosofia es:
si pasan mas de 3o minutos y no encuentras una respuesta por ti mismo:
pregunta...Alguien conocerá la respuesta.
/*código de struts 1*/
/*si el código es mas de 100 lineas entonces los métodos no están optimizados*/
public ActionForward execute(ActionMapping mapping, ActionForm form,
HttpServletRequest request, HttpServletResponse response)
throws Exception {
/*este tipo de mensajes de log debe de ser removido de la version final, esto solo hace que la aplicaciea mas lenta y por lo tanto su uso no es recomendado */
log.debug("Entra a Asset");
HttpSession session = request.getSession();
/*este busca una session del atributo request y pregunta por el perfil, aquí la referencia deberestar dentro de los filtros de sesi no dentro del co, lo cual hace una mala practica de separacion */
Users user = (Users) session.getAttribute("activeUser");
String usrprofile = request.getParameter("usrprofile") != null ? request.getParameter("usrprofile") : "ADMIN";
if (!"USRMX".equals(usrprofile) && !"USRLC".equals(usrprofile) && !"ADMIN".equals(usrprofile) && !"USRSL".equals(usrprofile) && !"MEGAADMIN".equals(usrprofile)) {
response.sendError(425);
}
String userUpdate = "";
/*los cos de los lenguajes no deben manejarse dentro de la acciestos deben manejarse con el uso de locales*/
String lang = request.getParameter("lang") != null ? request.getParameter("lang") : "";
/*si los cos if y else no contienen ejecucion alguna entonces
la manera correcta de proceder es por medio de un if y un parametro negado,
este tipo de practica hace que el sistema haga mas saltos if-else,
lo cual se traduce en impacto al rendimiento */
if ("".equals(lang)) {
} else if (lang.length() == 3 && "_en".equals(lang)) {
} else {
response.sendError(425);
}
if (user == null) {
log.debug("sin sesion");
if (lang.equals("")) {
response.sendError(200);
} else {
response.sendError(201);
}
}
// Colocacion de variables en request
request.setAttribute("usrprofile", usrprofile);
request.setAttribute("lang", lang);
request.setAttribute("section", "admin.lc.catalog.materials.asset");
request.setAttribute("class", CLASS_NAME.toLowerCase());
String mappingForward = "";
AssetForm assetForm = (AssetForm) form;
// Obtiene la url
String requestedUrl = request.getRequestURL().toString();
String optionStr = requestedUrl.substring(
requestedUrl.lastIndexOf("/") + 1, requestedUrl.lastIndexOf("."));
log.debug("opcion Elegida ==> " + optionStr);
// Si es modo administrador
if (requestedUrl.indexOf("admin") > 0) {
// Devuelve la lista de resultados
if (optionStr.equals("assetList")) {
log.debug("entro en assetList");
ArrayList catalogList = getAssetList(request);
// Remueve variables de request
request.removeAttribute("catalogList");
request.removeAttribute("title");
// Coloca variables en request
request.setAttribute("catalogList", catalogList);
request.setAttribute("title", "list");
mappingForward = "success";
} // Muestra el formulario para agregar un asset
else if (optionStr.equals("assetFormAdd")) {
log.debug("entro en assetFormAdd");
ArrayList listAssettype = toolDAO.getAssetTypeList();
Ccprofile profile = (Ccprofile) session.getAttribute("activeProfile");
String activeProfile = profile.getName();
HashMap mapSP = (HashMap) request.getSession().getAttribute("mapSP");
String pos10 = (String) mapSP.get("POS-10");
ArrayList listDivision = null;
if (activeProfile.equalsIgnoreCase(pos10)) {
user = (Users) session.getAttribute("activeUser");
String idDiv = (String) user.getDivisionId().toString();
listDivision = toolDAO.getDivisionListByDiv(idDiv);
} else {
listDivision = toolDAO.getDivisionList();
}
// Remueve variables de request
request.removeAttribute("list_assettype");
request.removeAttribute("list_division");
request.removeAttribute("title");
// Coloca variables en request
request.setAttribute("list_assettype", listAssettype);
request.setAttribute("list_division", listDivision);
request.setAttribute("title", "formAdd");
mappingForward = "success";
} // Muestra el formulario con los datos para editar un registro
else if (optionStr.equals("assetFormEdit")) {
log.debug("entro en assetFormEdit");
ArrayList listAssettype = toolDAO.getAssetTypeList();
Ccprofile profile = (Ccprofile) session.getAttribute("activeProfile");
String activeProfile = profile.getName();
HashMap mapSP = (HashMap) request.getSession().getAttribute("mapSP");
String pos10 = (String) mapSP.get("POS-10");
ArrayList listDivision = null;
if (activeProfile.equalsIgnoreCase(pos10)) {
user = (Users) session.getAttribute("activeUser");
String idDiv = (String) user.getDivisionId().toString();
listDivision = toolDAO.getDivisionListByDiv(idDiv);
} else {
listDivision = toolDAO.getDivisionList();
}
request.setAttribute("list_assettype", listAssettype);
request.setAttribute("list_division", listDivision);
String id = request.getParameter("id");
if (id != null && !id.equals("")) {
Asset asset = getAssetById(request);
request.setAttribute("asset", asset);
}
// Remueve variables de request
request.removeAttribute("title");
// Coloca variables en request
request.setAttribute("title", "formEdit");
mappingForward = "success";
} // Agrega un registro a la tabla de asset
else if (optionStr.equals("assetAdd")) {
log.debug("entro en assetAdd: " + assetForm);
log.debug("Nombre esp: " + assetForm.getName_sp());
String res = "";
PrintWriter writer = response.getWriter();
/*el uso del metodo getfechaActualHoraActual no sigue la notaciamel-case correcta*/
userUpdate = user.getAccount() + " " + Utils.getfechaActualHoraActual();
log.debug("usuario ---> " + userUpdate);
assetForm.setUpdated(userUpdate);
// valida que no este duplicado el registro
/*estas validaciones van en el ActionForm y no en el Action*/
if (!isDuplicated(assetForm)) {
request.setAttribute("title", "add");
// Agrega el registro
res = addReccord(assetForm);
writer.print("ok");
writer.flush();
writer.close();
} else {
request.removeAttribute("title");
request.setAttribute("title", "formAdd");
writer.print("duplicated");
// res = "duplicatedRecord";
}
mappingForward = res;
} // Actualiza el registro en la base de datos
else if (optionStr.equals("assetEdit")) {
log.debug("entro en assetEdit: " + assetForm);
userUpdate = user.getAccount() + " " + Utils.getfechaActualHoraActual();
log.debug("usuario ---> " + userUpdate);
assetForm.setUpdated(userUpdate);
String update = updateAsset(assetForm);
request.setAttribute("title", "edit");
if (update.equals("success")) {
PrintWriter writer = response.getWriter();
writer.print("ok");
writer.flush();
writer.close();
}
mappingForward = "success";
} // Elimina un registro en la base de datos.
else if (optionStr.equals("assetDelete")) {
/*Este flujo if-else no ejecuta nada*/
if (request.getHeader("Referer") != null) {
if (request.getHeader("Referer").toUpperCase().indexOf("KO.COM") > -1 || request.getHeader("Referer").toUpperCase().indexOf("8888") > -1) {
System.out.println("referer ==> " + request.getHeader("Referer"));
String token = request.getParameter("nekoTlaiceps");
/*estos comentarios son indescriptivos y no se recomiendan, la recomendacion es que se indique que valores puede traer la variable y como debe ser tratada cada una de ellas*/
// valida que el token traiga algo
if (token != null && !token.equalsIgnoreCase("")) {
// obtiene el hdiv de session
String hdiv = (String) session.getAttribute("HDIV");
// valida que el token sea igual al hdiv de session
if (hdiv.equalsIgnoreCase(token)) {
log.debug("entro en assetDelete");
String id = request.getParameter("id");
if (id != null && !id.equals("")) {
boolean update = deleteAsset(request);
request.setAttribute("title", "edit");
if (update) {
mappingForward = "success";
} else {
mappingForward = "error";
}
}
} else {
mappingForward = "error";
}
} else {
mappingForward = "error";
}
token = "";
} else {
System.out.println("Error referer no contiene token");
response.sendError(425);
}
} else {
System.out.println("Referer es null ");
response.sendError(425);
}
}
} else { // Si es modo usuario
/*este código es innecesario, debido a que no ejecuta ninguna sentencia*/
if (optionStr.equals("News")) {
}
}
return mapping.findForward(mappingForward);
}
como pueden ver, la acción debió dividirse en varias acciones, permitiendo que los códigos sean mas faciles de entender, esta es una sola acción que pretende meter todo en uno, este tipo de antipatrones se llama God Object.
No hay comentarios:
Publicar un comentario