Justin, the app-schema part looks fine. Please go ahead and commit. On 27/06/11 23:02, Justin Deoliveira wrote: > Issue updated with a new patch that does the rename of > JoiningQuery.getJoins() to JoniningQuery.getQueryJoins(). If you guys could > review that would be great. Thanks. > > -Justin > > On Fri, Jun 24, 2011 at 12:34 AM, Ben > Caradoc-Davies<[email protected]> wrote: > +1. Let me know when the patch is ready and I'll test it. > > > On 24/06/11 10:18, Niels wrote: > > Yeah, getQueryJoins() seems good. > > Cheers > > On 23/06/11 21:49, Justin Deoliveira wrote: > Thanks Niels, > > That is unfortunate that you will be moving on. Your work in geotools has > been very impressive. > > I think a method rename will work. I will update the patch to include a > method rename in app-schema. Any preference about a name? How about > getJoins() -> joins() or getJoins() -> getQueryJoins()? > > On Wed, Jun 22, 2011 at 9:57 PM, > Niels<[email protected]><mailto:[email protected]<mailto:[email protected]>> > wrote: > > Yes, this is true. Eventually I would like to see app-schema joining be using > the new geotools joining API, but I won't be around for that anymore. I think > it will take more to do the migration though than just merging, for example > because of the use of filters rather than two expressions for joining. In the > meantime a temporary solution would need to be found so nothing is broken, > simply renaming the method or something. > > > On 23/06/11 11:46, Justin Deoliveira wrote: > I just realized that the proposed api will actually cause a complication > failure in app-schema, due to JoiningQuery which subclasses query and adds > getJoins() but returns a different class called Join. > > app-schema folks: what are your thoughts? the two classes are significantly > different. Any chance of merging them? Or perhaps making the one in > app-schema a subclass of the new Join class? > > On Wed, Jun 22, 2011 at 9:34 AM, Ian > Turton<[email protected]<mailto:[email protected]><mailto:[email protected]<mailto:[email protected]>>> > wrote: > On 22 June 2011 11:23, Justin > Deoliveira<[email protected]<mailto:[email protected]><mailto:[email protected]<mailto:[email protected]>>> > wrote: > Hi Ian, > > On Wed, Jun 22, 2011 at 9:11 AM, Ian > Turton<[email protected]<mailto:[email protected]><mailto:[email protected]<mailto:[email protected]>>> > wrote: > > > It looks good to me so +1. > > My small niggle is the use of > > query.getJoins().add(join1); > > where I might prefer: > > query.addJoin(join1); > > which might be easier for users to spot in the API. > > I don't have a strong preference but I tend to stay away from doing that in > apis and instead just make the collection available. Reasoning being that > you are start to have to turn the containing class (in this case Query) into > a collection type, give it a getNumberJoins() method, removeJoin(), etc... > which imo pollutes the api. > > > That is a good point - but may be an addJoin() convenience method and > getJoins for everything else. I suspect most users will be just adding > a join to their query. > > > If you do stick with getJoins() is there are there good reasons to > make it return a list over a collection? I can't work out if it is > important to force an ordering to joins or not. > > I do think ordering is important, if for anything to determine what order > the attributes will be in the resulting feature type. It would seem strange > to do: > query.getJoins().add(new Join("type1")) > query.getJoins().add(new Join("type2")) > But then have the feature type return them in the order "type2", "type1". So > I guess my answer would be "any reason not to make it a list"? :) > > That is exactly the reason that was hoovering in the back of my mind > that I couldn't pull forward. A list is fine - I just wanted to check. > > Ian > > > > -- > Justin Deoliveira > OpenGeo - http://opengeo.org > Enterprise support for open source geospatial. > > > > -- > Niels Charlier > > Software Engineer > CSIRO Earth Science and Resource Engineering > Phone: +61 8 6436 8914 > > Australian Resources Research Centre > 26 Dick Perry Avenue, Kensington WA 6151 > > ------------------------------------------------------------------------------ > Simplify data backup and recovery for your virtual environment with vRanger. > Installation's a snap, and flexible recovery options mean your data is safe, > secure and there when you need it. Data protection magic? > Nope - It's vRanger. Get your free trial download today. > http://p.sf.net/sfu/quest-sfdev2dev > _______________________________________________ > Geotools-devel mailing list > [email protected]<mailto:[email protected]><mailto:[email protected]<mailto:[email protected]>> > > https://lists.sourceforge.net/lists/listinfo/geotools-devel > > > > > -- > Justin Deoliveira > OpenGeo - http://opengeo.org > Enterprise support for open source geospatial. > > > > -- > Niels Charlier > > Software Engineer > CSIRO Earth Science and Resource Engineering > Phone: +61 8 6436 8914 > > Australian Resources Research Centre > 26 Dick Perry Avenue, Kensington WA 6151 > > > -- > Ben Caradoc-Davies<[email protected]> > Software Engineering Team Leader > > CSIRO Earth Science and Resource Engineering > Australian Resources Research Centre > > > > -- > Justin Deoliveira > OpenGeo - http://opengeo.org > Enterprise support for open source geospatial. > >
-- Ben Caradoc-Davies <[email protected]> Software Engineering Team Leader CSIRO Earth Science and Resource Engineering Australian Resources Research Centre ------------------------------------------------------------------------------ All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
