Hi Niklas, On Fri, 2007-07-20 at 12:33 +0200, Niklas Nebel wrote: > Kohei Yoshida wrote: > > I'd like get your input on the new UNO API that I have introduced as > > part of implementing the multi-string filter. The relevant issue page > > is here: > > > > http://qa.openoffice.org/issues/show_bug.cgi?id=77677 > > > > Currently, Calc only has one type of sheet filter, which is represented > > by com.sun.star.sheet.TableFilterField. And it is read from & written > > to the document model all at once via getFilterFields and > > setFilterFields in the XSheetFilterDescriptor interface. Multiple > > instances of this data are contained in css::uno::Sequence. > > > > Because I needed to allow reading and writing of filters of different > > types, which the current API doesn't allow, I created a new set of API > > just for this. Let me summarize the new API below. > > TableFilterField and TableFilterFieldNormal would then be two structs > for basically the same thing.
Yes. I don't like it either. What I initially tried to do was to modify the existing TableFilterField by moving its first two data members to a new base struct TableFilterFieldBase, and leave the other four in. But that flaged offapi's API compatibility check. See discussion on this thread: http://udk.openoffice.org/servlets/ReadMsg?list=dev&msgNo=3862 What we could do alternatively is to totally scratch the base struct, and keep all filter structs on their own structs, without having them derived from the common base. This way we could re-use the existing TableFilterField. I had tried that in my draft implementation once. But, one issue with this approach was that, it made it harder to store all filter fields of different types in the same container in C++. For Java and Python it's probably not much of a concern because everything is a child class of 'Object' (coicidentally that holds true in both languages). But for C++, it does not apply, unless we use void*, shared_ptr<void> etc, which I would rather avoid using. > Also, First/Next-style iteration is > usually done through a createEnumeration method, where it's clear that > each call returns a new object with separate enumeration state. Why > don't we keep the new interface similar to the old one, with get/set for > a sequence of fields, > either with a new struct containing all field > options, This was my first approach, and it would be okay just to handle the multi-string filter extension. But this would make API a little ugly, since the multi-string filter only needs three data: Field, Connection, and StringSet, and everything else gets ignored. Also, this would not be safe for future addition of new filter types either. > or with an any containing the old or new struct? This is doable. But we still need to know the filter type before unpacking the data from Any so that we can use the correct struct instance to unpack it to. So, I still think that we should have some means to explicitly specify the filter type even when using Any type. I can think of two ways to do this. 1) Introduce an additional struct to hold data type as well as the data itself: struct NameNotYetDetermined { TableFilterFieldType Type; uno::Any Data; }; and store that in uno::Sequence<NameNotYetDetermined>. Or 2) Use the following API: void addTableFilterField( [in] uno::Any aData, [in] TableFilterFieldType eType ); I personally prefer the second approach. This is in line with what Noel was suggesting for reading the field data. > I also don't think we should integrate this without having UI for it. > That would result in "hidden state", where a filter behaves in an > unexpected way and the user can't see why. This is certainly true. But, if we integrate it in the current state, such "hidden state" would only occur when the user imports a document from Excel 2007 and that document contains multi-string filter data. Even then, using Calc's autofilter, Standard or Advanced filters would fall back to the old behavior. So, I would say the casualty for introducing such "hidden state" is minimal. Another reason I want to integrate it now and the UI later, is that, in the OOXML importer code, I'm currently having to maintain two versions of autofilter import code, one for this API change, and one without it. Integrating this bit now would allow me to only maintain one version of that code, which would make my job a little easier. --Kohei --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
