Hi all, IMHO it is a “Bad practice” (e.g. see “Law of Demeter” [1]) which is probably also used by others ;) However I agree if such method chaining is used in combination with “Fluent Interfaces/Builder/..” but not for “adding elements to a collection”. But I also respect other opinions ;o)
Based on the fact that a change of this methods behaviour would break already existing services I think like Ramesh that we should/could create a new “addEntity(..)” method and keep the “getEntities()” like it is. Perhaps we could add a “@Deprecated” to the “get” but it must more like a “@DeprecatedBehaviour” because only the behaviour will be changed and not the methods signature. Best Regards, Michael [1]: https://en.wikipedia.org/wiki/Law_of_Demeter <https://en.wikipedia.org/wiki/Law_of_Demeter> > On 25 Jan 2016, at 08:41, Francesco Chicchiriccò <[email protected]> wrote: > > On 24/01/2016 12:40, mibo wrote: >> Hi all, >> >> During work on olingo-832 I realized that the „getEntities“ method in the >> „org.apache.olingo.commons.api.data.EntityCollection“ return the list of >> entities directly (instead of an unmodifiable List). >> >> IMHO the best practice for List handling is to return only unmodifiable >> Lists and offer a method like „addEntity(e:Entity)“ for modification to >> ensure the encapsulation of the data (entities) within the collection object. > > Well, this being considered a "bad practice" is quite controversial, IMHO - > just for an out-of-the-box example, this is the standard way how collections > are managed via JAXB. > >> As suggested modification the „getEntites“ should return a >> „Collections.unmodifiableList(entities);“ and the EntityCollection gets a >> new method „public void addEntity(Entity e) { entities.add(e); }“. >> >> I know it is a little bit late to change the API that way, especially >> because we used the „bad practice“ of „getEntities().add(..)“ a lot in the >> Oligo code and tutorial. >> >> Nevertheless I wanted to start a (short) discussion on the dev mailing list >> what do you think about this and if we should change the code or not. > > As said above, I don't believe this is a required change: intentionally, the > actual list is final and there is not setEntities() method, so I don't see > which bad practice could derive from this. The getEntities() method is not > returning an array, but a final collection that cannot be replaced. > > Regards. > > -- > Francesco Chicchiriccò > > Tirasa - Open Source Excellence > http://www.tirasa.net/ > > Involved at The Apache Software Foundation: > member, Syncope PMC chair, Cocoon PMC, Olingo PMC, CXF committer > http://home.apache.org/~ilgrosso/ > >
smime.p7s
Description: S/MIME cryptographic signature
