I ran the following code through Sonar, a static analysis tool.Original file:public interface LegRepository extends CustomRepository<Leg, Long> {    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             + GROUP BY l.TransportMission_SID) groupedl             + ON ll.TransportMission_SID = groupedl.tptId             + AND ll.legorder = groupedl.legorder;    final String FIND_CURRENT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = SELECT l.*             + FROM T_Movement_Leg l             + WHERE l.TransportMission_SID = ?1 AND l.legorder=?2;    final String FIND_CURRENT_AND_FOLLOWING_LEGS_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = SELECT l.*             + FROM T_Movement_Leg l             + WHERE l.TransportMission_SID = ?1 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.TransportMission_SID) groupedl             + ON ll.TransportMission_SID = groupedl.tptId             + AND ll.legorder = groupedl.legorder;    final String FIND_LAST_ORDER_NUMBER_BY_TRANSPORTMISSION_ID = SELECT max(l.legorder)             + FROM T_Movement_Leg l             + WHERE l.TransportMission_SID = ?1 and l.is_Canceled = 'false' and l.IN_USE = 'true';    @Query(value = FIND_START_LEG_BY_TRANSPORTMISSION_ID, nativeQuery = true)    Leg findStartLeg(Long tptMissionId);    @Query(value = FIND_LAST_LEG_BY_TRANSPORTMISSION_ID, nativeQuery = true)    Leg findEndLeg(Long tptMissionId);    @Query(value = FIND_PREVIOUS_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER, nativeQuery = true)    Leg findPreviousLeg(Long tptMissionId, Integer legOrder);    @Query(value = FIND_NEXT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER, nativeQuery = true)    Leg findNextLeg(Long tptMissionId, Integer legOrder);    @Query(value = FIND_LAST_ORDER_NUMBER_BY_TRANSPORTMISSION_ID, nativeQuery = true)    Integer findLastOrderNumberByTransportMission(Long tptMissionId);    @Query(value = FIND_CURRENT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER, nativeQuery = true)    Leg findCurrentLeg(Long tptMissionId, Integer legOrder);    @Query(value = FIND_CURRENT_AND_FOLLOWING_LEGS_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER, nativeQuery = true)    List<Leg> findCurrentAndFollowingLegs(long tptMissionId, Integer legOrder);    List<Leg> findByTransportMissionOrderByLegorderAsc(TransportMission transportMission);    List<Leg> findByTransportMissionAndCanceledOrderByLegorderAsc(TransportMission transportMission, Boolean canceled);}In my opinion, it is perfectly readable.  However, Sonar complains:The final is absolute cause it's an interface.Some literal is used more then 2 times.So after refactoring I get this class with 100% compliance of Sonar:public interface LegRepository extends CustomRepository<Leg, Long> {    String FROM_MOVEMENT_LEG = FROM T_Movement_Leg ll ;    String INNER_JOIN = INNER JOIN ;    String GROUP_BY = GROUP BY l.TransportMission_SID) groupedl ;    String ON_TRANSPORTMISSION_ID_AND_LEGORDER = ON ll.TransportMission_SID = groupedl.tptId AND ll.legorder = groupedl.legorder;    String SELECT_ALL_LL = SELECT ll.* ;    String FIND_START_LEG_BY_TRANSPORTMISSION_ID = SELECT_ALL_LL            + FROM_MOVEMENT_LEG            + 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            + ON_TRANSPORTMISSION_ID_AND_LEGORDER;    String FIND_LAST_LEG_BY_TRANSPORTMISSION_ID = SELECT_ALL_LL            + FROM_MOVEMENT_LEG            + 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            + ON ll.TransportMission_SID = groupedl.tptId             + AND ll.LegEffective_EndDate = groupedl.datetime;    String FIND_PREVIOUS_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = SELECT_ALL_LL            + FROM_MOVEMENT_LEG            + 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             + GROUP_BY            + ON_TRANSPORTMISSION_ID_AND_LEGORDER;    String FIND_CURRENT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = SELECT_ALL_LL            + FROM_MOVEMENT_LEG            + WHERE ll.TransportMission_SID = ?1 AND ll.legorder=?2;    String FIND_CURRENT_AND_FOLLOWING_LEGS_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = SELECT_ALL_LL            + FROM T_Movement_Leg ll             + WHERE ll.TransportMission_SID = ?1 AND ll.legorder>=?2;    String FIND_NEXT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER = SELECT_ALL_LL            + FROM_MOVEMENT_LEG            + 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            + ON_TRANSPORTMISSION_ID_AND_LEGORDER;    String FIND_LAST_ORDER_NUMBER_BY_TRANSPORTMISSION_ID = SELECT max(ll.legorder)             + FROM T_Movement_Leg ll             + WHERE ll.TransportMission_SID = ?1 and ll.is_Canceled = 'false' and ll.IN_USE = 'true';    @Query(value = FIND_START_LEG_BY_TRANSPORTMISSION_ID, nativeQuery = true)    Leg findStartLeg(Long tptMissionId);    @Query(value = FIND_LAST_LEG_BY_TRANSPORTMISSION_ID, nativeQuery = true)    Leg findEndLeg(Long tptMissionId);    @Query(value = FIND_PREVIOUS_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER, nativeQuery = true)    Leg findPreviousLeg(Long tptMissionId, Integer legOrder);    @Query(value = FIND_NEXT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER, nativeQuery = true)    Leg findNextLeg(Long tptMissionId, Integer legOrder);    @Query(value = FIND_LAST_ORDER_NUMBER_BY_TRANSPORTMISSION_ID, nativeQuery = true)    Integer findLastOrderNumberByTransportMission(Long tptMissionId);    @Query(value = FIND_CURRENT_LEG_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER, nativeQuery = true)    Leg findCurrentLeg(Long tptMissionId, Integer legOrder);    @Query(value = FIND_CURRENT_AND_FOLLOWING_LEGS_BY_TRANSPORTMISSION_ID_AND_LEG_ORDER, nativeQuery = true)    List<Leg> findCurrentAndFollowingLegs(long tptMissionId, Integer legOrder);    List<Leg> findByTransportMissionOrderByLegorderAsc(TransportMission transportMission);    List<Leg> findByTransportMissionAndCanceledOrderByLegorderAsc(TransportMission transportMission, Boolean canceled);}The final tag I can understand, however I find it better to read static final then final alone, just to make clear to someone who read's the code.  It's indeed obsolete but when you don't see it's an interface you are confused.I can live with this, because it's actually normal.What I have a problem with is actually the second part.I have to make more fields because I repeat some parts of the query's.Yes that's true, but how can you read now the query easy?It's because I want the query readable, I split it in parts.By splitting it, I need now to insert other fields where you can't read directly what's behind it.What do you think about this?
Sonar remarks make SQL unreadable?
java;sql;comparative review;constants
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 presentationString 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 presentationString 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;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 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);    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();    }}Generating the final query strings then becomes:// extra line breaks added for CodeReview.SE presentationString 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();In SummaryIt is sometimes OK to hard-code literal query strings, especially when prioritizing readability of the raw queries in code, when the queries themselves are simple enough, or when there are no distinctive patterns. However, when these literal strings become too boilerplate-like, it might be worthwhile to either invest time in using a database access library, or constructing the queries with the aid of the builder approach so that common parts of the queries remain standardized.Is the sample QueryBuilder really a good way to keep reusability higher? I don't think there's a definitive answer to that... Always evaluate the pros and cons.