Yes, that sounds fine. Henning, we voted against friend for a number of reasons, I think the most important one is referencing to testing code in the core.
On 23/8/2010 3:26 PM, Kostka Bořivoj wrote: > Itamar, > > I propose to just redeclare 6 IndexWriter methods from private to protected > (not public). Then I can create IndexWriter4Test subclass, > which declares same methods public and just calls protected methods from > IndexWriter. > > For me this seems better than keeping them private, copying their body, > redeclare members used in them from private > to protected and make SegmentInfo class exported. > > Is this acceptable? > > Borek > > >> -----Original Message----- >> From: Itamar Syn-Hershko [mailto:ita...@code972.com] >> Sent: Sunday, August 22, 2010 1:16 PM >> To: clucene-developers@lists.sourceforge.net >> Subject: Re: [CLucene-dev] IndexWriter's private methods for tests >> >> Well, how many functions and variables that are marked private those >> functions try to access? >> >> We can either have the ~8 or so functions public, or upgrade the other >> functions from private to protected - which actually makes quite a lot >> of sense. >> >> Itamar. >> >> On 21/8/2010 10:43 PM, Kostka Bořivoj wrote: >> >>> Itamar, >>> >>> I tried to solve this by subclassing, but it is probably impossible. Eg. >>> method >>> >> getNumBufferedDocuments() uses >> >>> docWriter, which is declared private in IndexWriter, so I cannot access it. >>> I can't see any solution except declaring testing methods public (or at >>> least >>> >> protected) in original IndexWriter >> >>> Borek >>> >>> >>> >>>> -----Original Message----- >>>> From: Kostka Bořivoj [mailto:kos...@tovek.cz] >>>> Sent: Tuesday, August 17, 2010 3:39 PM >>>> To: clucene-developers@lists.sourceforge.net >>>> Subject: Re: [CLucene-dev] IndexWriter's private methods for tests >>>> >>>> OK, I understand your idea now. The problem is only segmentsInfos, so I >>>> will >>>> >> change it >> >>>> to exported (it can be changed in the future, if we decide so). >>>> >>>> If we hit more/many exporting issues we could consider special DLL build >>>> with >>>> >> more >> >>>> exports for testing purposes rather than link statically. >>>> Anyway, tt will be best, if we have both static libs and dll build for >>>> cl_test. >>>> >>>> Borek >>>> >>>> >>>> >>>>> -----Original Message----- >>>>> From: Itamar Syn-Hershko [mailto:ita...@code972.com] >>>>> Sent: Tuesday, August 17, 2010 1:34 PM >>>>> To: clucene-developers@lists.sourceforge.net >>>>> Subject: Re: [CLucene-dev] IndexWriter's private methods for tests >>>>> >>>>> We definitely need to port all tests. >>>>> >>>>> What I meant with subclassing is to remove the functions that aren't >>>>> used currently, and have a IndexWriterForTests class in cl_test which >>>>> will implement those functions instead. From what I recall none of these >>>>> functions are used anywhere, so removing them from the core isn't an >>>>> issue. If any of them is indeed used, well, since it is used for things >>>>> other than testing we do need it there (and as such it should be made >>>>> protected / public, based on where it is being called from in tests). >>>>> >>>>> If you'll hit many exporting issues with cl_test you can make cl_test >>>>> link statically to cl_core (and also cl_shared). That is better than >>>>> exporting new stuff that is better left internal. >>>>> >>>>> Itamar. >>>>> >>>>> On 17/8/2010 11:35 AM, Kostka Bořivoj wrote: >>>>> >>>>> >>>>>> Itamar, >>>>>> >>>>>> Perhaps I'm wrong, but I think subclassing isn't (easy) solution: >>>>>> >>>>>> Related IndexWriter's methods are private, not protected, so I cannot >>>>>> call them >>>>>> >>>>>> >>>> even >>>> >>>> >>>>> from subclass. >>>>> >>>>> >>>>>> I also cannot copy the code, as segmentsInfos methods are used inside, >>>>>> and >>>>>> >>>>>> >>>>> segmentInfos class is not exported so I'm not able to link. >>>>> >>>>> >>>>>> Maybe protected will be good trade-off(?) >>>>>> >>>>>> If methods remain private, I have to remove a lot of checks from tests. >>>>>> I can do >>>>>> >> that, >> >>>>>> >>>>> of course, but I'm not sure if such test are still usefull. >>>>> >>>>> >>>>>> And the last point - if methods remain private, we should simply remove >>>>>> them as >>>>>> >> a >> >>>>>> >>>>> whole, because it is just a chunk of dead code. >>>>> >>>>> >>>>>> Borek >>>>>> >>>>>> >>>>>> >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Itamar Syn-Hershko [mailto:ita...@code972.com] >>>>>>> Sent: Monday, August 16, 2010 10:47 PM >>>>>>> To: clucene-developers@lists.sourceforge.net >>>>>>> Subject: Re: [CLucene-dev] IndexWriter's private methods for tests >>>>>>> >>>>>>> Borek, >>>>>>> >>>>>>> I had answered this in Jun 24th [1][2]. We pretty much agreed >>>>>>> "friend"ing won't do much good. >>>>>>> >>>>>>> I'd still prefer exposing this code only in the test suite and not in >>>>>>> core, since it is only being used there - this can be achieved easily by >>>>>>> subclassing. No reason to have them in core if all they are used for is >>>>>>> tests, and I wouldn't want to add more compile switches... >>>>>>> >>>>>>> Itamar. >>>>>>> >>>>>>> [1] >>>>>>> http://permalink.gmane.org/gmane.comp.jakarta.lucene.clucene.devel/3484 >>>>>>> [2] The whole discussion: >>>>>>> http://comments.gmane.org/gmane.comp.jakarta.lucene.clucene.devel/3472 >>>>>>> >>>>>>> On 16/8/2010 4:00 PM, Kostka Bořivoj wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> In tests I'm porting following set of methods from IndexWriter is used: >>>>>>>> >>>>>>>> //for test purposes >>>>>>>> int32_t getDocCount(int32_t i); >>>>>>>> int32_t getNumBufferedDocuments(); >>>>>>>> int32_t getSegmentCount(); >>>>>>>> int32_t getBufferedDeleteTermsSize(); >>>>>>>> int32_t getNumBufferedDeleteTerms(); >>>>>>>> SegmentInfo* newestSegment(); >>>>>>>> >>>>>>>> The problem is all methods are marked private. In JLucene methods are >>>>>>>> >> without >> >>>>>>>> >>>>>>>> >>>>>>> public/private definition, >>>>>>> >>>>>>> >>>>>>> >>>>>>>> so they are accessible from classes in same package (and test are in >>>>>>>> same >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> package). >>>>>>> >>>>>>> >>>>>>> >>>>>>>> Such thing is not possible in C++, so I think methods should be >>>>>>>> changed to >>>>>>>> >>>>>>>> >>>> public. >>>> >>>> >>>>>>>> >>>>>>> Otherwise tests cannot >>>>>>> >>>>>>> >>>>>>> >>>>>>>> be ported. >>>>>>>> >>>>>>>> Any other opinion? >>>>>>>> >>>>>>>> Borek >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> ------------------------------------------------------------------------------ >>>>>>>> This SF.net email is sponsored by >>>>>>>> >>>>>>>> Make an app they can't live without >>>>>>>> Enter the BlackBerry Developer Challenge >>>>>>>> http://p.sf.net/sfu/RIM-dev2dev >>>>>>>> _______________________________________________ >>>>>>>> CLucene-developers mailing list >>>>>>>> CLucene-developers@lists.sourceforge.net >>>>>>>> https://lists.sourceforge.net/lists/listinfo/clucene-developers >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> ------------------------------------------------------------------------------ >>>>>>> This SF.net email is sponsored by >>>>>>> >>>>>>> Make an app they can't live without >>>>>>> Enter the BlackBerry Developer Challenge >>>>>>> http://p.sf.net/sfu/RIM-dev2dev >>>>>>> _______________________________________________ >>>>>>> CLucene-developers mailing list >>>>>>> CLucene-developers@lists.sourceforge.net >>>>>>> https://lists.sourceforge.net/lists/listinfo/clucene-developers >>>>>>> >>>>>>> >>>>>>> >>>>>> ------------------------------------------------------------------------------ >>>>>> This SF.net email is sponsored by >>>>>> >>>>>> Make an app they can't live without >>>>>> Enter the BlackBerry Developer Challenge >>>>>> http://p.sf.net/sfu/RIM-dev2dev >>>>>> _______________________________________________ >>>>>> CLucene-developers mailing list >>>>>> CLucene-developers@lists.sourceforge.net >>>>>> https://lists.sourceforge.net/lists/listinfo/clucene-developers >>>>>> >>>>>> >>>>>> >>>>> ------------------------------------------------------------------------------ >>>>> This SF.net email is sponsored by >>>>> >>>>> Make an app they can't live without >>>>> Enter the BlackBerry Developer Challenge >>>>> http://p.sf.net/sfu/RIM-dev2dev >>>>> _______________________________________________ >>>>> CLucene-developers mailing list >>>>> CLucene-developers@lists.sourceforge.net >>>>> https://lists.sourceforge.net/lists/listinfo/clucene-developers >>>>> >>>>> >>>> ------------------------------------------------------------------------------ >>>> This SF.net email is sponsored by >>>> >>>> Make an app they can't live without >>>> Enter the BlackBerry Developer Challenge >>>> http://p.sf.net/sfu/RIM-dev2dev >>>> _______________________________________________ >>>> CLucene-developers mailing list >>>> CLucene-developers@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/clucene-developers >>>> >>>> >>> ------------------------------------------------------------------------------ >>> This SF.net email is sponsored by >>> >>> Make an app they can't live without >>> Enter the BlackBerry Developer Challenge >>> http://p.sf.net/sfu/RIM-dev2dev >>> _______________________________________________ >>> CLucene-developers mailing list >>> CLucene-developers@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/clucene-developers >>> >>> >> ------------------------------------------------------------------------------ >> This SF.net email is sponsored by >> >> Make an app they can't live without >> Enter the BlackBerry Developer Challenge >> http://p.sf.net/sfu/RIM-dev2dev >> _______________________________________________ >> CLucene-developers mailing list >> CLucene-developers@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/clucene-developers >> > ------------------------------------------------------------------------------ > This SF.net email is sponsored by > > Make an app they can't live without > Enter the BlackBerry Developer Challenge > http://p.sf.net/sfu/RIM-dev2dev > _______________________________________________ > CLucene-developers mailing list > CLucene-developers@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/clucene-developers > ------------------------------------------------------------------------------ This SF.net email is sponsored by Make an app they can't live without Enter the BlackBerry Developer Challenge http://p.sf.net/sfu/RIM-dev2dev _______________________________________________ CLucene-developers mailing list CLucene-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/clucene-developers
