Tobias Laufkötter commented on OFBIZ-9549:

Hi Jacques,

thank you for the review.

# I opted for the {{return}} instead of {{else}} to reduce the code complexity. 
There is nothing to do after the {{else}}, so there is no reason to introduce a 
new block and an additional level of indentation, which makes the code harder 
to read. Testing for the "easy" cases at the beginning and returning right 
after is a popular and (in my opinion) preferable code style. Now that I think 
about it again, I'd also like to return from the second {{if}}-block.
# I included this, because the called function doesn't like a forward only 
result set. I now realize, that the alternative doesn't like it either and that 
the thrown exception is handled properly. I'll just take it out.
# I'll apply your formatting.
# I'll take it back in.
# Actually, I'm not quite sure what would be preferrable in this case. The 
general use case in these methods is to get the results and filter or display 
them. In each case the resulting list would be iterated and not modified during 
the iteration. An {{ArrayList}} allocates less memory, since we know the size 
of the resulting list and don't need additions beyond the initial size. So 
performance wise, this should be the way to go. For an empty list, the 
{{LinkedList}} actually leaves a smaller memory footprint, so I would chose 
that one for the empty list use cases. I would leave this open to discussion, 
since I'm not an expert and need to trust what StackOverflow tells me.

> Refactor EntityListIterator
> ---------------------------
>                 Key: OFBIZ-9549
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-9549
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>    Affects Versions: Trunk
>            Reporter: Tobias Laufkötter
>            Assignee: Jacques Le Roux
>            Priority: Minor
>         Attachments: OFBIZ-9549.patch
> The EntityListIterator contains duplicate code and has need for refactoring 
> in general.

This message was sent by Atlassian JIRA

Reply via email to