I would suggest to consider usability over operator developer convenience. With String[] as parameter, BeanUtils will automatically convert a comma separated string into an array. That does not work with List<String>.
Someone could also explore the use of additional BeanUtils converters for common types that we would like to offer better configuration is support for. This same discussion came up a while ago with time duration/unit properties. Thomas On Thu, May 12, 2016 at 9:17 PM, Bhupesh Chawda <[email protected]> wrote: > If this List is large enough, the user might have to create a large number > of properties, one for each element. > In this case however, it should be small enough and I think we can go ahead > with a List. > > ~Bhupesh > > On Thu, May 12, 2016 at 8:36 PM, Yogi Devendra <[email protected]> > wrote: > > > IMO from engineering perspective, List<String> is more elegant than comma > > separated strings and parsing it later. > > > > From user experience perspective, it is important to have consistent way > > across all operators. Some operators having comma separated string others > > having List<String> is confusing. > > > > Since, this discussion is more generic, not specific to a particular > > operator we should discuss this as a separate email thread. Some > guideline > > document describing best practices would be even better. > > > > > > ~ Yogi > > > > On 13 May 2016 at 06:36, Munagala Ramanath <[email protected]> wrote: > > > > > Philosophical musing: In software, functionality trumps (pun *not* > > > intended) elegance. > > > > > > Ram > > > > > > On Thu, May 12, 2016 at 6:02 PM, Bhupesh Chawda < > [email protected] > > > > > > wrote: > > > > > > > I thought that this might not be considered clean w.r.t. the user > > > > experience while configuring the operator. > > > > However, if it is acceptable then I'll go ahead with this. > > > > > > > > Thanks for pointing it out! > > > > > > > > ~Bhupesh > > > > > > > > On Thu, May 12, 2016 at 5:52 PM, Thomas Weise < > [email protected]> > > > > wrote: > > > > > > > > > Thanks! Looks a bit cumbersome though... > > > > > > > > > > > > > > > On Thu, May 12, 2016 at 5:47 PM, Munagala Ramanath < > > > [email protected]> > > > > > wrote: > > > > > > > > > > > An example of doing *exactly* that is shown here: > > > > > > http://docs.datatorrent.com/application_packages/ > > > > > > in the section entitled "Operator properties" > > > > > > > > > > > > Ram > > > > > > > > > > > > On Thu, May 12, 2016 at 5:44 PM, Bhupesh Chawda < > > > > [email protected] > > > > > > > > > > > > wrote: > > > > > > > > > > > > > AFAIK, there is no clean way to set a List property from an XML > > > file. > > > > > > > > > > > > > > ~Bhupesh > > > > > > > > > > > > > > On Thu, May 12, 2016 at 5:41 PM, Munagala Ramanath < > > > > > [email protected]> > > > > > > > wrote: > > > > > > > > > > > > > > > What was the concern about "configuring the List elements > from > > > the > > > > > XML > > > > > > > > property file" > > > > > > > > and what issue might it create ? > > > > > > > > > > > > > > > > Ram > > > > > > > > > > > > > > > > On Thu, May 12, 2016 at 5:38 PM, Bhupesh Chawda < > > > > > > [email protected] > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi All, > > > > > > > > > > > > > > > > > > I'll reopen the pull request again after addressing review > > > > > comments. > > > > > > > > > > > > > > > > > > - There was a concern from Thomas regarding configuring > > the > > > > List > > > > > > > > > elements from the XML property file. I think this might > > > create > > > > > an > > > > > > > > > issue. I > > > > > > > > > propose that in that case we create String getters and > > > setters > > > > > > > instead > > > > > > > > > of > > > > > > > > > list. The signatures will stay the same but we'll change > > the > > > > > type > > > > > > of > > > > > > > > the > > > > > > > > > fields to String to make them consistent: > > > > > > > > > - String topics > > > > > > > > > - public String getTopics() > > > > > > > > > - public void setTopics(String topics) > > > > > > > > > - I'll rename the branch name as suggested by Gaurav on > > the > > > > pull > > > > > > > > > request. > > > > > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > > > > > > > > ~Bhupesh > > > > > > > > > > > > > > > > > > On Wed, May 11, 2016 at 8:43 AM, Munagala Ramanath < > > > > > > > [email protected]> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Yes, making the accessor types match the field will avoid > > > > > > confusion, > > > > > > > so > > > > > > > > > +1. > > > > > > > > > > > > > > > > > > > > We can add accessors like getTopicItem() and > setTopicItem() > > > for > > > > > the > > > > > > > > > > individual > > > > > > > > > > elements if necessary. > > > > > > > > > > > > > > > > > > > > Ram > > > > > > > > > > > > > > > > > > > > On Wed, May 11, 2016 at 12:08 AM, Bhupesh Chawda < > > > > > > > > > [email protected]> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > Hi All, > > > > > > > > > > > > > > > > > > > > > > The fields in Kafka Input Operator in Malhar kafka > > project > > > > are > > > > > of > > > > > > > > type > > > > > > > > > > > String array. However the getters and setters are not > > > > > consistent > > > > > > > and > > > > > > > > > > have a > > > > > > > > > > > return type and parameters of String type respectively. > > > > > > > > > > > This may cause issues in down stream systems and hence > > must > > > > be > > > > > > > > fixed. I > > > > > > > > > > > have created a JIRA to track this - > > > > > > > > > > > https://issues.apache.org/jira/browse/APEXMALHAR-2084 > > > > > > > > > > > > > > > > > > > > > > I am proposing that the types of these fields be made > > > > > > List<String> > > > > > > > > > > instead > > > > > > > > > > > of String[ ]. Additionally also make the getters and > > > setters > > > > > > accept > > > > > > > > > > > parameters and return data of the same type. > > > > > > > > > > > > > > > > > > > > > > Please let me know your thoughts on the same. > > > > > > > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > ~Bhupesh > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
