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?
String
s? \$\endgroup\$