patternjavaModerate

Sonar remarks make SQL unreadable?

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

sqlmakeremarkssonarunreadable

Problem

I ran the following code through Sonar, a static analysis tool.

Original file:

```
public interface LegRepository extends CustomRepository {

final String FIND_START_LEG_BY_TRANSPORTMISSION_ID = "SELECT ll.* "
+ "FROM T_Movement_Leg ll "
+ "INNER JOIN "
+ "(SELECT l.TransportMission_SID tptId, min(l.legorder) as legorder "
+ "FROM T_Movement_Leg l where l.TransportMission_SID = ?1 and l.is_Canceled = 'false' and l.IN_USE = 'true' "
+ "GROUP BY l.TransportMission_SID) groupedl "
+ "ON ll.TransportMission_SID = groupedl.tptId "
+ "AND ll.legorder = groupedl.legorder";

final String FIND_LAST_LEG_BY_TRANSPORTMISSION_ID = "SELECT ll.* "
+ "FROM T_Movement_Leg ll "
+ "INNER JOIN "
+ "(SELECT l.TransportMission_SID tptId, max(l.LegEffective_EndDate) as datetime "
+ "FROM T_Movement_Leg l where l.TransportMission_SID = ?1 and l.is_Canceled = 'false' and l.IN_USE = 'true' "
+ "GROUP BY l.TransportMission_SID) groupedl "
+ "ON ll.TransportMission_SID = groupedl.tptId "
+ "AND ll.LegEffective_EndDate = groupedl.datetime";

final String FIND_PREVIOUS_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = "SELECT ll.* "
+ "FROM T_Movement_Leg ll "
+ "INNER JOIN "
+ "(SELECT l.TransportMission_SID tptId, max(l.legorder) as legorder "
+ "FROM T_Movement_Leg l where l.TransportMission_SID = ?1 and l.is_Canceled = 'false' and l.IN_USE = 'true' and l.legorder =?2";

final String FIND_NEXT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = "SELECT ll.* "
+ "FROM T_Movement_Leg ll "
+ "INNER JOIN "
+ "(SELECT l.TransportMission_SID tptId, min(l.legorder) as legorder "
+ "FROM T_Movement_Leg l where l.TransportMission_SID = ?1 and l.is_Canceled = 'false' and l.IN_USE = 'true' and l.legorder > ?2 "
+ "GROUP BY l.Trans

Solution

First things first, I assume you are already convinced that using literal SQL query strings is the way to go in this project/class, and that there are no comparable solutions from using an established database access library.

If you prioritize readability of literal SQL query strings, then @200_success's advice is solid. However, looking at the content of your query strings, I think you may want to consider approaching refactoring not from a Sonar remarks or even a 'literacy' perspective, but a pattern-identification perspective.

Your refactored code is not fully refactored actually, since you were still using a case mixture of keywords e.g. and/AND. Taking things to the extreme (I like living on the edge, sometimes), I managed to 'condense' your literal Strings into the following fragments:

// extra line breaks added for CodeReview.SE presentation
String SELECT_ALL_LL = "SELECT ll.* ";
String FROM = "FROM T_Movement_Leg @@ WHERE @@.TransportMission_SID = ?1 ";
String SELECT_INNER_JOIN = SELECT_ALL_LL + "FROM T_Movement_Leg ll " 
                + "INNER JOIN (SELECT l.TransportMission_SID tptId, %s %s ";
String BASE_CONDITION = FROM.replaceAll("@@", "l") 
                + "AND l.is_Canceled = 'false' AND l.IN_USE = 'true' ";
String ON_TRANSPORTMISSION_ID = "GROUP BY l.TransportMission_SID) groupedl ON " 
                + "ll.TransportMission_SID = groupedl.tptId AND ll.%s = groupedl.%s";
String ON_TRANSPORTMISSION_ID_AND_LEGORDER = 
                String.format(ON_TRANSPORTMISSION_ID, "legorder", "legorder");
String CURRENT_LEG = SELECT_ALL_LL + FROM.replaceAll("@@", "ll") 
                + "AND ll.legorder %s ?2";

And the final definitions of your query strings:

// extra line breaks added for CodeReview.SE presentation
String FIND_START_LEG_BY_TRANSPORTMISSION_ID = 
        String.format(SELECT_INNER_JOIN, "min(l.legorder)", "legorder")
        + BASE_CONDITION
        + ON_TRANSPORTMISSION_ID_AND_LEGORDER;

String FIND_LAST_LEG_BY_TRANSPORTMISSION_ID = 
        String.format(SELECT_INNER_JOIN, "max(l.LegEffective_EndDate)", "datetime")
        + BASE_CONDITION
        + String.format(ON_TRANSPORTMISSION_ID, "LegEffective_EndDate", "datetime");

String FIND_PREVIOUS_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = 
        String.format(SELECT_INNER_JOIN, "max(l.legorder)", "legorder")
        + BASE_CONDITION + "AND l.legorder  ?2 "
        + ON_TRANSPORTMISSION_ID_AND_LEGORDER;

String FIND_CURRENT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = 
        String.format(CURRENT_LEG, "=");

String FIND_CURRENT_AND_FOLLOWING_LEGS_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = 
        String.format(CURRENT_LEG, ">=");

String FIND_LAST_ORDER_NUMBER_BY_TRANSPORTMISSION_ID = 
        "SELECT max(l.legorder) " + BASE_CONDITION;

The patterns I identified are:

Finding the start/last/previous/next legs require an inner join on a table that has a fixed BASE_CONDITION with an optional supplementary condition ("AND l.legorder ?2 "), and both tables are joined on very similar clauses.

Finding the following legs in addition to the current leg only differs from the latter by a conditional operator, namely >= vs. =.

Finding the last order number is pretty much a standalone query, though it shares the same BASE_CONDITION.

These patterns hint that you may want to consider using a builder-like pattern to construct your query strings, which also helps to solve one point mentioned in @200_success's answer:

Furthermore, it would be a tragedy to expose these SQL fragments in your interface.

A 'query builder' can be further extended to validate parts of the query during its construction, in addition to hiding away the query fragments while exposing (arguably) readable method names for interfacing. Here is a sample, simple implementation for your queries, based on the extensive refactoring done above:

```
public static final class QueryBuilder {
// extra line breaks added for CodeReview.SE presentation
private static final String SELECT_ALL_LL = "SELECT ll.* ";
private static final String FROM = "FROM T_Movement_Leg @@ WHERE "
+ "@@.TransportMission_SID = ?1 ";
private static final String SELECT_INNER_JOIN = SELECT_ALL_LL
+ "FROM T_Movement_Leg ll INNER JOIN (SELECT "
+ "l.TransportMission_SID tptId, %s %s ";
private static final String BASE_CONDITION = FROM.replaceAll("@@", "l") + " AND "
+ "l.is_Canceled = 'false' AND l.IN_USE = 'true' ";
private static final String ON_TPT_ID = "GROUP BY l.TransportMission_SID) groupedl "
+ "ON ll.TransportMission_SID = groupedl.tptId "
+ "AND ll.%s = groupedl.%s";
private static final String ON_TRANSPORTMISSION_ID_AND_LEGORDER =
String.format(ON_TPT_ID, "legorder", "legorder

Code Snippets

// extra line breaks added for CodeReview.SE presentation
String SELECT_ALL_LL = "SELECT ll.* ";
String FROM = "FROM T_Movement_Leg @@ WHERE @@.TransportMission_SID = ?1 ";
String SELECT_INNER_JOIN = SELECT_ALL_LL + "FROM T_Movement_Leg ll " 
                + "INNER JOIN (SELECT l.TransportMission_SID tptId, %s %s ";
String BASE_CONDITION = FROM.replaceAll("@@", "l") 
                + "AND l.is_Canceled = 'false' AND l.IN_USE = 'true' ";
String ON_TRANSPORTMISSION_ID = "GROUP BY l.TransportMission_SID) groupedl ON " 
                + "ll.TransportMission_SID = groupedl.tptId AND ll.%s = groupedl.%s";
String ON_TRANSPORTMISSION_ID_AND_LEGORDER = 
                String.format(ON_TRANSPORTMISSION_ID, "legorder", "legorder");
String CURRENT_LEG = SELECT_ALL_LL + FROM.replaceAll("@@", "ll") 
                + "AND ll.legorder %s ?2";

// extra line breaks added for CodeReview.SE presentation
String FIND_START_LEG_BY_TRANSPORTMISSION_ID = 
        String.format(SELECT_INNER_JOIN, "min(l.legorder)", "legorder")
        + BASE_CONDITION
        + ON_TRANSPORTMISSION_ID_AND_LEGORDER;

String FIND_LAST_LEG_BY_TRANSPORTMISSION_ID = 
        String.format(SELECT_INNER_JOIN, "max(l.LegEffective_EndDate)", "datetime")
        + BASE_CONDITION
        + String.format(ON_TRANSPORTMISSION_ID, "LegEffective_EndDate", "datetime");

String FIND_PREVIOUS_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = 
        String.format(SELECT_INNER_JOIN, "max(l.legorder)", "legorder")
        + BASE_CONDITION + "AND l.legorder < ?2 "
        + ON_TRANSPORTMISSION_ID_AND_LEGORDER;

String FIND_NEXT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = 
        String.format(SELECT_INNER_JOIN, "min(l.legorder)", "legorder")
        + BASE_CONDITION + "AND l.legorder > ?2 "
        + ON_TRANSPORTMISSION_ID_AND_LEGORDER;

String FIND_CURRENT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = 
        String.format(CURRENT_LEG, "=");

String FIND_CURRENT_AND_FOLLOWING_LEGS_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = 
        String.format(CURRENT_LEG, ">=");

String FIND_LAST_ORDER_NUMBER_BY_TRANSPORTMISSION_ID = 
        "SELECT max(l.legorder) " + BASE_CONDITION;

public static final class QueryBuilder {
    // extra line breaks added for CodeReview.SE presentation
    private static final String SELECT_ALL_LL = "SELECT ll.* ";
    private static final String FROM = "FROM T_Movement_Leg @@ WHERE " 
                                    + "@@.TransportMission_SID = ?1 ";
    private static final String SELECT_INNER_JOIN = SELECT_ALL_LL 
                                    + "FROM T_Movement_Leg ll INNER JOIN (SELECT " 
                                    + "l.TransportMission_SID tptId, %s %s ";
    private static final String BASE_CONDITION = FROM.replaceAll("@@", "l") + " AND "
                                    + "l.is_Canceled = 'false' AND l.IN_USE = 'true' ";
    private static final String ON_TPT_ID = "GROUP BY l.TransportMission_SID) groupedl "
                                    + "ON ll.TransportMission_SID = groupedl.tptId "
                                    + "AND ll.%s = groupedl.%s";
    private static final String ON_TRANSPORTMISSION_ID_AND_LEGORDER = 
                                    String.format(ON_TPT_ID, "legorder", "legorder");
    private static final String CURR_LEG = SELECT_ALL_LL + FROM.replaceAll("@@", "ll")
                                    + "AND ll.legorder %s ?2";

    private final StringBuilder builder = new StringBuilder();

    private QueryBuilder() {
        // empty
    }

    public static QueryBuilder selectWithInnerJoin(final String supplementaryColumn,
            final String columnAlias) {
        final QueryBuilder qBuilder = new QueryBuilder();
        qBuilder.builder
                .append(String.format(SELECT_INNER_JOIN, supplementaryColumn, 
                                        columnAlias))
                .append(BASE_CONDITION);
        return qBuilder;
    }

    public QueryBuilder andLegOrderCondition(final String condition) {
        builder.append(" AND l.legorder ").append(condition).append(" ?2 ");
        return this;
    }

    public String onDefaultColumns() {
        return builder.append(ON_TRANSPORTMISSION_ID_AND_LEGORDER).toString();
    }

    public String onDefaultColumnAnd(final String leftColumn, 
            final String rightColumn) {
        return builder.append(String.format(ON_TPT_ID, 
                    "LegEffective_EndDate", "datetime")).toString();
    }

    public static String selectWithLegOrderCondition(final String condition) {
        return String.format(CURR_LEG, condition);
    }

    public static String selectLastLegOrder() {
        return "SELECT max(l.legorder) " + BASE_CONDITION;
    }

    @Override
    public String toString() {
        return builder.toString();
    }
}

// extra line breaks added for CodeReview.SE presentation
String FIND_START_LEG_BY_TRANSPORTMISSION_ID = QueryBuilder
        .selectWithInnerJoin("min(l.legorder)", "legorder")
        .onDefaultColumns();

String FIND_LAST_LEG_BY_TRANSPORTMISSION_ID = QueryBuilder
        .selectWithInnerJoin("max(l.LegEffective_EndDate)", "datetime")
        .onDefaultColumnAnd("LegEffective_EndDate", "datetime");

String FIND_PREVIOUS_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = QueryBuilder
        .selectWithInnerJoin("max(l.legorder)", "legorder")
        .andLegOrderCondition("<").onDefaultColumns();

String FIND_NEXT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = QueryBuilder
        .selectWithInnerJoin("min(l.legorder)", "legorder")
        .andLegOrderCondition(">").onDefaultColumns();

String FIND_CURRENT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = 
        QueryBuilder.selectWithLegOrderCondition("=");

String FIND_CURRENT_AND_FOLLOWING_LEGS_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = 
        QueryBuilder.selectWithLegOrderCondition(">=");

String FIND_LAST_ORDER_NUMBER_BY_TRANSPORTMISSION_ID = 
        QueryBuilder.selectLastLegOrder();

Context

StackExchange Code Review Q#83174, answer score: 13

Revisions (0)

No revisions yet.