When Good Practices Go Bad

I'm we were all nagged about similar things when we were younger: wear your coat outside, elbows off the table, and use constants instead of string literals. It would seem though that not every one (James Watson's colleague especially) doesn't quite get the gist of these "best practices" we were all taught ...

private static final String 
  SELECT_MAPS = 
    SELECT + DOC_TYPE + COMMA + PARTNER + COMMA + PRIORITY + COMMA + ERROR + COMMA + 
    ERROR_DETAIL + COMMA + ACCEPT_REJECT + COMMA + PROCESS_CODE + COMMA + 
    RESULT_STATUS_CHANGE + FROM + ERROR_MAPS + WHERE,
  WHERE_PARTNER_SETTINGS = 
    DOC_TYPE + EQUALS + PARAMETER + AND + PARTNER + EQUALS + PARAMETER,
  NULL_PARTNER_SETTINGS = 
    DOC_TYPE + NULL + AND + PARTNER + NULL,
  WHERE_STATUS_CHANGE_DEPENDENCY_PREFIX = 
    " NOT EXISTS " + OPEN_PARENTHESIS + SELECT + "'X'" + FROM + STATUS_CHANGE_DEPENDENCY + 
    WHERE + MAP_ID + EQUALS + ERROR_MAPS + "." + MAP_ID + AND + STATUS_CHANGE + " NOT IN " 
    + OPEN_PARENTHESIS,
  WHERE_STATUS_CHANGE_DEPENDENCY_SUFFIX = 
    CLOSE_PARENTHESIS + CLOSE_PARENTHESIS,
  ORDER_BY = " ORDER BY 1, 2, 3";

Those of us from the Java or .NET world have also been taught to use StringBuffer/StringBuilders instead of concatenating strings together (myStr3 = myStr1 + myStr2). Leave it to Mr. Best Practices to find a way to actually make this technique less efficient ...

String sql
  = new StringBuffer(SELECT_MAPS)
    .append(ERROR_CODE)
    .append((errorNumber == null) ? NULL : (EQUALS + errorNumber))
    .append(AND)
    .append(ORDER_STATUS)
    .append((orderStatusCode == null) ? NULL : (EQUALS + orderStatusCode))
    .append(AND)
    .append(STATUS_CHANGE)
    .append((statusChangeTypeCode == null) ? NULL : (EQUALS + statusChangeTypeCode))
    .append(AND)
    .append(WHERE_PARTNER_SETTINGS)
    .append((statusChangeTypeCodesStr == null)
      ? ""
      : new StringBuffer(AND)
          .append(WHERE_STATUS_CHANGE_DEPENDENCY_PREFIX)
          .append(statusChangeTypeCodesStr)
          .append(WHERE_STATUS_CHANGE_DEPENDENCY_SUFFIX)
          .toString())
    .append(enableGenericMapping
      ? new StringBuffer(UNION)
        .append(SELECT_MAPS)
        .append(ERROR_CODE)
        .append((errorNumber == null) ? NULL : (EQUALS + errorNumber))
        .append(AND)
        .append(ORDER_STATUS)
        .append((orderStatusCode == null) ? NULL : (EQUALS + orderStatusCode))
        .append(AND)
        .append(STATUS_CHANGE)
        .append((statusChangeTypeCode == null) ? NULL : (EQUALS + statusChangeTypeCode))
        .append(AND)
        .append(NULL_PARTNER_SETTINGS)
        .append((statusChangeTypeCodesStr == null)
            ? ""
            : new StringBuffer(AND)
            .append(WHERE_STATUS_CHANGE_DEPENDENCY_PREFIX)
                .append(statusChangeTypeCodesStr)
                .append(WHERE_STATUS_CHANGE_DEPENDENCY_SUFFIX)
                .toString())
          .toString()
      : "")
    .append(ORDER_BY).toString();

Had he gone with the earlier technique, the compiler would have automatically concatenated the constants and literals, instead of having to do them at run time ...

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!