EVA-1190 Report studies browsable since given date#70
EVA-1190 Report studies browsable since given date#70jmmut merged 1 commit intoEBIvariation:developfrom
Conversation
| * @see #groupAndCount | ||
| * @see #projectAndFlatten | ||
| */ | ||
| public List<VariantStudySummary> findByFromDate(Date fromDate) { |
There was a problem hiding this comment.
is "findByFromDate" a valid spring data idiom? if not I suggest "findFromDate"
There was a problem hiding this comment.
oh, even better: "findSinceDate". "from date" suggests studies from that date only (to me). specially in the request parameter. /studies/list?fromDate=2018-04-15 doesn't appear to mean the same as /studies/list?sinceDate=2018-04-15. a third alternative would be "newerThan" but that may be too speech-mimicking.
There was a problem hiding this comment.
I don't like a lot "findByFromDate", but I don't think that "fromDate" is wrong. I would say that the method name for retrieving the studies in a particular date is "findByDate", not "from date".
Other names? findFromDate, findSinceDate... if we should use "findBy" because we are doing it in all the methods, I would say "findByDateGreaterOrEqualThan" ... but I think is very verbose
There was a problem hiding this comment.
I like "findByFromDate", I think it is indicating it will retrieve results FROM the given date TO any date after that (as TO is not specified)
There was a problem hiding this comment.
To be honest I used the "findBy" because that is the idiom in Spring repositories. Even though this is a service I did it by inertia. The "from date" is as @andresfsilva says, as normally if we were to represent the interval the arguments would most probably be called fromDate and toDate.
If you guys prefer a completely different name please discuss and change the name yourselves while I'm travelling, otherwise I will merge it after arrival because there are already 2 approvals.
There was a problem hiding this comment.
I read a bit and it seems "from" is used indeed for ranges. this naming issue has already spent too much time. I'm merging and deploying so that EBIvariation/eva-ws#89 can run travis.
| * @see #groupAndCount | ||
| * @see #projectAndFlatten | ||
| */ | ||
| public List<VariantStudySummary> findByFromDate(Date fromDate) { |
There was a problem hiding this comment.
I like "findByFromDate", I think it is indicating it will retrieve results FROM the given date TO any date after that (as TO is not specified)
4 participants
Added query of
VariantStudySummaryobjects by creation date, test dataset modified accordingly.