[jira] [Commented] (OFBIZ-10485) Refactor MapContext

["Taher Alkhateeb (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OFBIZ-10485?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16569128#comment-16569128 ] 

Taher Alkhateeb commented on OFBIZ-10485:
-----------------------------------------

Patches 4, 5, 6, 7 and 8 are all applied on trunk in r1837409. Check your editor settings because I found a tab character in one of the patches, and thank you for your work so far, the code definitely looks much cleaner.

I have a few questions comments for the upcoming patches:
 * I'm not sure replacing stackList with maps in patch 9 adds clarity. Maybe both names are vague and do not convey purpose. How about calling it contexts, or contextMaps, or something like that?
 * for multi-line comments, I recommend the /* */ comment style, maybe that would be a bit cleaner? Also, what do you mean by checking a key instead of null allows for overriding, it means override with a null instead of of skipping? If yes, aren't we changing the logic of the system and does that introduce any bugs or new behavior?
 * Nice work on the entryStream, I like it. Did you apply tests from your side to ensure no edge cases are broken after the replacement from the for-loops?
 * Why are we using a functional interface in patch #12? it feels a bit complicated, why not just apply 2 streams and be done with it?'

Again thank you for tolerating my questions, and I really appreciate your work.

> Refactor MapContext
> -------------------
>
>                 Key: OFBIZ-10485
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-10485
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: base
>            Reporter: Mathieu Lirzin
>            Assignee: Taher Alkhateeb
>            Priority: Minor
>             Fix For: Upcoming Branch
>
>         Attachments: OFBIZ-10485_0001-Remove-MapContext-dead-code.patch, OFBIZ-10485_0002-Add-missing-Override-in-MapContext.patch, OFBIZ-10485_0003-Use-the-Deque-interface-in-MapContext.patch, OFBIZ-10485_0004-Rewrite-MapContext-isEmpty.patch, OFBIZ-10485_0005-Rewrite-MapContext-containsKey.patch, OFBIZ-10485_0006-Rewrite-MapContext-keySet.patch, OFBIZ-10485_0007-Remove-MapContext.ListSet-class.patch, OFBIZ-10485_0008-Inline-MapContext-getMapContext.patch, OFBIZ-10485_0009-Rename-stackList-to-maps.patch, OFBIZ-10485_0010-Rework-comments.patch, OFBIZ-10485_0011-Add-entryStream.patch, OFBIZ-10485_0012-Add-withMapContainingKey.patch, OFBIZ-10485_0013-Rewrite-size.patch
>
>
> Following conversation [https://lists.apache.org/thread.htmlf1729ef5eafcf71adfbed0c3ea61dfb73225dff82abc4a57c3de8ed5@%3Cdev.ofbiz.apache.org%3E] on dev@ofbiz.apache.org.
> Here is a first batch of patches to clean things up. Those patches are meant to be applied in order.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)