CodeSOD: In House Refactoring

Today's submitter works for a company that has a history of outsourcing pretty much all development, and to the lowest bidder, at that. This resulted in some terrible code, complete with 30,000 line classes, so the company decided to bring development back in house and clean up the code.

After three years of hard work, the in-house team was proud to announce their new, more maintainable, version of the code. Unfortunately, they didn't have the best sense of what maintainable looked like, so their Java code had methods with signatures like this:

public String createNewActivity( String node_id, String problem_id, String act_spec, String act_reason, String eff_date, String person_id, String synopsis, String commit_date, int elapsed_time, String comment, boolean customer_viewable, String actionPlan, String apFolderLocation, String apLogging, String areasAffected, String artifactLocation, String assembliesFileChanged, Date assignmentDate, String baseResults, String baseVersionRecreated, Date completionDate, String coordinator, String crystalReportsFiles, String customerBusinessImpact, String customerName, String customization, String custFolderResults, String databaseTrace, Date dateTimeOfDeployment, String dbBkupLocationName, String dbName, String dbResults, String defectFixesInUpdate, String defectId, String downtimeRequired, String environment, String environmentLocation, String errorMessage, String escalationsName, String escalationHostingName, String escalationType, String escalationUpdate, String eventLogs, String fiddler, String filestoreRefreshNeeded, String forecastDate, String hostingName, String hostNumber, String iisLogs, String instructions, String memoryDump, String operationMode, String overallRisk, String packageDeployedInCodeTest, String perfmonLogs, String preferredDowntime, String priority, String pseRequestSubtype, String pseRequestType, String qaAssignTo, String qaResults, String qaStatus, String receiveNotifications, String recreatedOnBase, String recreatedWCustApFolder, String recreatedWCustDbBackup, String recreationSteps, String requestReason, String requestType, String services, String servicesNeedReset, String sourceDatasourceName, String sourceHostNumber, String sqlStatementIncluded, String status, String statusUpdates, String summaryDescription, String summaryScope, String supportServer, String targetDatasourceName, String targetHostNumber, String testCasesDocumentUrl, String testCasesExecutionActual, String testCasesExecutionEstimate, String testServerName, String testServerType, String ticketReference, String userCreatedFilesNeeded, String webex, String whoCreatedPackage, String workaround) throws Exception

This method calls into a database, and while it at least uses a prepared statement to do it, the statement still ends up looking pretty silly:

 preparedStatement = con.prepareStatement("insert into ACT_TT ( " + " S_ACTIVITY_ID " + ",ACTION_PLAN " + ",AP_FOLDER_LOCATION " + ",AP_LOGGING " + ",AREAS_AFFECTED " + ",ARTIFACT_LOCATION " + ",ASSEMBLIES_FILE_CHANGED " + ",ASSIGNMENT_DATE " + ",BASE_RESULTS " + ",BASE_VERSION_RECREATED " + ",CASE " + ",COMMENTS " + ",COMPLETION_DATE " + ",COORDINATOR " + ",CRYSTAL_REPORTS_FILES " + ",CUSTOMER_BUSINESS_IMPACT " + ",CUSTOMER_NAME " + ",CUSTOMIZATION " + ",CUST_FOLDER_RESULTS " + ",DATABASE_TRACE " + ",DATE_TIME_DEPLOYED " + ",DB_BKUP_LOCATION_NAME " + ",DB_NAME " + ",DB_RESULTS " + ",DEFECT_FIXES_IN_UPDATE " + ",DEFECT_ID " + ",DOWNTIME_REQUIRED " + ",ENVIRONMENT " + ",ENVIRONMENT_LOCATION " + ",ERROR_MESSAGE " + ",ESCALATIONS_NAME " + ",ESCALATION_HOSTING_NAME " + ",ESCALATION_TYPE " + ",ESCALATION_UPDATE " + ",EVENT_LOGS " + ",FIDDLER " + ",FILESTORE_REFRESH_NEEDED " + ",FORECAST_DATE " + ",HOSTING_NAME " + ",HOST_NUMBER " + ",IIS_LOGS " + ",INSTRUCTIONS " + ",MEMORY_DUMP " + ",OPERATION_MODE " + ",OVERALL_RISK " + ",PACKAGE_DEPLOYED_IN_CODE_TEST " + ",PERFMON_LOGS " + ",PREFERRED_DOWNTIME " + ",PRIORITY " + ",PSE_REQUEST_SUBTYPE " + ",PSE_REQUEST_TYPE " + ",QA_ASSIGN_TO " + ",QA_RESULTS " + ",QA_STATUS " + ",RECEIVE_NOTIFICATIONS " + ",RECREATED_ON_BASE " + ",RECREATED_W_CUST_AP_FOLDER " + ",RECREATED_W_CUST_DB_BACKUP " + ",RECREATION_STEPS " + ",REQUEST_REASON " + ",REQUEST_TYPE " + ",SERVICES " + ",SERVICE_NEED_RESET " + ",SOURCE_DATASOURCE_NAME " + ",SOURCE_HOST_NUMBER " + ",SQL_STATEMENT_INCLUDED " + ",STATUS " + ",STATUS_UPDATES " + ",SUMMARY_DESCRIPTION " + ",SUMMARY_SCOPE " + ",SUPPORT_SERVER " + ",TARGET_DATASOURCE_NAME " + ",TARGET_HOST_NUMBER " + ",TEST_CASES_DOCUMENT_URL " + ",TEST_CASES_EXECUTION_ACTUAL " + ",TEST_CASES_EXECUTION_ESTIMATE " + ",TEST_SERVER_NAME " + ",TEST_SERVER_TYPE " + ",TICKET_REFERENCE " + ",USER_CREATED_FILES_NEEDED " + ",WEBEX " + ",WHO_CREATED_PACKAGE " + ",WORKAROUND " + ") VALUES (" + " ?,?,?,?,?," + " ?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?," + " ?,?,?,?,?)");

And if you're wondering about the data types of those fields, we can tell from a short selection from the section where they set the values, this is almost entirely stringly typed:

 preparedStatement.setString(10, baseVersionRecreated); preparedStatement.setString(11, problem_id); preparedStatement.setString(12, comment); preparedStatement.setTimestamp(13, getAsTimestamp(completionDate)); preparedStatement.setString(14, coordinator); preparedStatement.setString(15, crystalReportsFiles);

I can only guess that, at some point, the costs of maintaining this version will spiral out of control, and someone will have the bright idea to outsource it to the lowest bidder. And the cycle continues.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!