Stats/misc

[PapaBravo]

split functionality in Resource and ResponseBuilder

[thomaskipar]

AlcoholLevelResponse:

• always use { } for in if/else/while... statements
• you could do the whole math in one hql statement as well (date/time arithmetics)
• comment: // else FORBIDDEN --> what is forbidden here and why?
• add mehtod comments! What if user does not exist, for instance? Test this in your unit tests as well!
• I think you implementation is wrong (or am I missing something)? : oldConcentration = curConcentration + oldConcentration * (curDate.getTime() - oldDate.getTime()) / 1000 / 60 / 60 * alcoholBreakDown; --> old date is the date of the first drawing. However, shouldn't that be the current time instead?

TestAlcoholLevelResponseBuilder:

• "level 1-3 per mille" seems to be a quite wide range. Just calc the value yourself an test for it (with little epsilon)
• test for 0 per mille -> a user that did not draw at all and a user that did not draw for quite some time (maybe within the x hours range?)

AchievementResponseBuilder:

• see above (comments, tests for invalid input)

Please rename methods:

• build* to retrieve*
• rename classes as well... actually I do not know how for now ;-)

Will you adapt existing code later on of for that pull request as well? --> try to not to do so many stuff for one pull request in future, please.

[PapaBravo]

AlcoholLevelResponseBuilder

• I have no idea how to do that math on hql only as it is basically rescursively defined. Grouping into (custom) intervals on hql would result in huge queries (that have to be build on runtime in more or less the same way as I do it now).
• comments follow shortly. This was more to for you to confirm the new structure.
• calculations are right IMO. will check again. Will add tests as you proposed.

On the renaming: Is Factory better then Builder?

I will adapt more code on this pull request yes. I tried to complete one task with one pull request.

[thomaskipar]

Have a look at http://docs.jboss.org/hibernate/orm/3.3/reference/en/html/queryhql.html, 14.10 Expressions. eg. current_time()/date(), "-", etc. could be used to perform the whole calculation at once (i guess). However, this is low prio... so only change it if your in the mood (and you have the time) :-)

[thomaskipar]

No second review performed yet.