Return to Answer

I suggest you follow everything what Simon said in his answer. I would like to point out some additional issues:

• Don't ever concatenate strings to build an SQL query (unless you have meticulously checked that there is no possibility of injection attacks, e.g. by properly escaping everything). While it is probably safe in this case, it is a massive code smell. Use parameterized queries instead.

• SELECT * and then extracting those columns you are interested in is totally unecessary. Only ask for what you need.

• When using a C-style for loop over an index `i', you will usually want to specify the upper bound directly and not introduce an extra variable. Instead of

   int length = list.size();
   for (int i = 0; i < length; i++) {
  

  just do for (int i = 0; i < list.size; i++). Of couse this shouldn't be done for more complex bounds (because performance), but here introducing variables is just obfuscation. It also avoids useless names like:

   int length = list.size();
   int size = list.size();
  

• Talking about names: list isn't a good name for a List<...> – I can see from the type what it is. Would timeIntervals be a better name?

• Your three loops can be mostly joined into a single loop, once you view the timeIntervals list as a stack: On every step, you pop the previous element off from the stack, compare the previous and the current element, and if they are sufficiently different, push the both onto the stack.

  The next step is to see that you don't need a whole stack and only the previous interval, and the sum. As an extra, this saves you some memory :-)

• IIRC, the Calendar API shouldn't be used. You might want to look into JodaTime.

• Please don't rely on the ordering of rows from an SQL query. If you need some order, specify an ORDER BY clause in the SQL.

• The 15-Minute extra shouldn't be hardcoded – take it as a parameter to your function and ultimately from a configuration file. Business rules don't belong into compiled code.

I suggest you follow everything what Simon said in his answer. I would like to point out some additional issues:

• Don't ever concatenate strings to build an SQL query (unless you have meticulously checked that there is no possibility of injection attacks, e.g. by properly escaping everything). While it is probably safe in this case, it is a massive code smell. Use parameterized queries instead.

• SELECT * and then extracting those columns you are interested in is totally unecessary. Only ask for what you need.

• When using a C-style for loop over an index `i', you will usually want to specify the upper bound directly and not introduce an extra variable. Instead of

   int length = list.size();
   for (int i = 0; i < length; i++) {
  

  just do for (int i = 0; i < list.size; i++). Of couse this shouldn't be done for more complex bounds (because performance), but here introducing variables is just obfuscation. It also avoids useless names like:

   int length = list.size();
   int size = list.size();
  

• Talking about names: list isn't a good name for a List<...> – I can see from the type what it is. Would timeIntervals be a better name?

• Your three loops can be mostly joined into a single loop, once you view the timeIntervals list as a stack: On every step, you pop the previous element off from the stack, compare the previous and the current element, and if they are sufficiently different, push the both onto the stack.

  The next step is to see that you don't need a whole stack and only the previous interval, and the sum. As an extra, this saves you some memory :-)

• IIRC, the Calendar API shouldn't be used. You might want to look into JodaTime.

• Please don't rely on the ordering of rows from an SQL query. If you need some order, specify an ORDER BY clause in the SQL.

• The 15-Minute extra shouldn't be hardcoded – take it as a parameter to your function and ultimately from a configuration file. Business rules don't belong into compiled code.