Github user kaspersorensen commented on the issue:

    https://github.com/apache/metamodel/pull/148
  
    Ah yes you're right. That should give a good boost in performance in deed!
    
    Functionally the code looks good to me. I am not 100% sure about what I 
think about the JoinHelper class. It's only made up of static methods which 
means that all it's methods could just as well be directly part of 
MetaModelHelper. And MetaModelHelper is IMO already a kind of a junkdrawer of 
static methods, so I don't think I want another :-) On the other hand, 
something that would maybe be nice is if you made the JoinHelper class 
non-static so that you would instantiate it like this:
    ```
    DataSetJoiner joiner = new DataSetJoiner(fromItemWithJoin);
    DataSet dataSet = joiner.join(dataSets);
    ```
    (or something quite similar to this)
    
    I think then we would have a class that's quite nicely built and scoped 
such that it cannot be used out of context (statically). So that would be my 
suggestion for a more pretty design :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to