On Thu, Jul 10, 2014 at 12:03 PM, Luc Maisonobe <l...@spaceroots.org> wrote:
> Hi Venkat, > > Le 09/07/2014 18:56, venkatesha murthy a écrit : > > Could some one respond please > > > I'll have a look at this probably on Sunday. > We are all volunteer, so there may be some delay for action. > Hi Luc, Thanks much for the same. venkat > > best regards, > Luc > > > > > > > On Sat, Jul 5, 2014 at 8:06 PM, venkatesha m > <ts_v_mur...@yahoo.com.invalid> > > wrote: > > > >> > >> > >> > >> > >> On Saturday, 28 June 2014 12:08 AM, venkatesha murthy < > >> venkateshamurth...@gmail.com> wrote: > >> > >> > >> > >> On Wed, Jun 25, 2014 at 12:54 PM, Luc Maisonobe <l...@spaceroots.org> > >> wrote: > >> > >>> Hi Venkat, > >>> > >>> Le 25/06/2014 06:21, venkatesha murthy a écrit : > >>>> The Percentile actually uses KthSelector logic and is dependent on > only > >>>> KthSelector > >>>> however the variability part is pivoting strategy. > >>>> > >>>> Given that both KthSelector and Pivoting are independent we could make > >>> them > >>>> as utility classes or may be functions with in MathUtils with exposed > >>>> interfaces. > >>> > >>> +1 > >>> > >>>> > >>>> Heres my opinion: > >>>> > >>>> First, Move Both PivotingStrategy and KthSelector to utils package as > >>> they > >>>> can be general purposed > >>> > >>> +1 > >>> > >>>> > >>>> Secondly, make PivotingStrategy enum implement an interface > >>>> PivotingStrategyInterface which allows > >>>> random generator to be set and with other necessary methods. This is > >> to > >>>> make way for some one who is interested to make a different seed for > >>> random > >>>> or for a different pivoting strategy itself. > >>> > >>> Take care that the random generator cannot be set in the enum itself as > >>> there is only one instance and it would mean we make an enum mutable, > >>> which is really something chilling to me. > >>> > >>> Do we really need this to be an enum? Couldn't we have only an > interface > >>> and three regular classes implementations so people can set up their > own > >>> private RandomPivotingStrategy without fearing other parts of the code > >>> would change it? > >>> > >>>> > >>>> Next, make the KthSelector accept a PivotingStrategyInterface rather > >> than > >>>> enum > >>> > >>> +1 > >>> > >>>> > >>>> Next, make Percentile accept a constructed KthSelector and allow it > >> flow > >>>> through evaluate and estimate method instead of flowing > >> PivotingStrategy > >>>> through estimate method. > >>> > >>> +1. I understand that in this case you want to replace the > >>> withPivotingStrategy by withKthSelector, which is a good thing as it is > >>> a more user friendly level of customization. > >>> > >>> Luc > >>> > >>>> > >>>> What do you think > >> Please let know for any other information/clarification needed. > >>>> > >>>> thanks > >>>> venkat. > >>>> > >>> > >> > >> As per this discussion ; i have attached todays patch. > >> pl let know > >> > >> Please let know if this patch i submitted as per above discussion is > > fine? > > > > > >> thanks > >> venkat > >> > >> > >>> > >>> --------------------------------------------------------------------- > >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > >>> For additional commands, e-mail: dev-h...@commons.apache.org > >>> > >>> > >> > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
