[GSoC 2012][Collaboration] ScDocFunc and Collaboration

[Eike Rathke]

Hi Matúš,

On Tuesday, 2012-07-31 19:09:45 +0200, Matúš Kukan wrote:

> > To be able to
> > call the same methods from the view regardless of whether we have direct
> > or collaborative input we need ScDocFuncDirect and ScDocFuncSend,
> > otherwise we'd have to distinguish between direct and collab in each
> > method.
> 
> Right, it would be about adding one 'if' into each method.

And then call another method anyway (or have all code in the if block)
for collaboration.. to me a derived ScDocFuncSend looks cleaner.


> > For the receiver we need ScDocFuncRecv to process the command and send
> > it as input to ScDocFuncDirect. The functionality of ScDocFuncRecv
> > theoretically could be implemented anywhere, but deriving it from
> > ScDocFunc gives access to ScDocShell (currently available as member in
> > ScDocFuncRecv also, but see abstract base class below), and IMHO
> > deriving it clarifies the relation between ScDocFunc* classes.
> 
> But currently ScDocFuncRecv is not deriving from anything

Hum yes, sorry I mixed that up and got carried away..

> and there is just ~one method,
> which is possible to move into ScDocFuncSend.
> See attached diff as illustration for what I have in mind.

Sure, would work. Just why do it so and call methods of the base class
instead of having the little ScDocFuncRecv helper class with
a ScDocFuncDirect pointer?

Btw, we should not end up with yet another
if(string==...){}else if(string==...){}else ...
chain for the commands that looks ugly and slows down things again, I'd
like more a hashmap resolving to IDs that can be switch-cased on.


> > Chaining ScDocFuncRecv into ScDocFuncSend in real collab mode is only
> > needed for ScDocFuncSend::SetCollaboration() so could be passed there
> > instead, but in dev/demo mode it is also needed in
> > ScDocFuncSend::SendMessage() to emulate the broadcast, having it always
> > as member IMHO is fine.
> 
> I was ignoring demo mode for now, I believe it can be solved later.

I think it's good to keep an eye on it as it will come handy when
working on the send/recv commands.


> >> Also ScDocFuncDirect is just ScDocFunc with own costructor, not sure what for.
> >
> > Ideally we'll have ScDocFunc as an abstract base class later, do not
> > construct the derived classes with an extra ScDocShell and have
> > ScDocFuncDirect implement the then abstract methods for which
> > implementation currently still lives at ScDocFunc. Just ignore that
> > little indirection for now.
> 
> Sorry if that's obvious but I don't understand why ScDocFunc should be abstract.
> I've never understood the comment in sendfunc.cxx:
> // FIXME: really ScDocFunc should be an abstract base, so
> // we don't need the rDocSh hack/pointer
> ScDocFuncSend::ScDocFuncSend( ScDocShell& rDocSh, ScDocFuncRecv *pDirect )
>         : ScDocFunc( rDocSh ),
> 
> How does that depend on whether ScDocFunc is abstract ( that means it
> has at least one pure virtual method I guess ?

yes

> ) or not ?

IMHO whether we need to pass rDocSh or not is not the point, but the
benefit of an abstract base class would be that a pure virtual method
forces the implementation of a derived class to implement exactly that
signature. Adding a parameter to a method of just one of the classes and
forgetting the other is not possible, it would need to be added to
ScDocFunc and both ScDocFuncDirect and ScDocFuncSend derivatives would
have to implement it. It helps the developer.


> > I actually see no benefit in having one ScDocFunc, why do you think it
> > would be nicer?
> 
> Hmm, because there would be only one but that may be also (ideo)logically bad.
> Depends where you want to distinguish between collaboration modes,
> what ScDocFunc means ...

ScDocFunc so far was the glue between view and document or API and
document, actually much of its functionality previously was in
ScViewFunc before it was extracted to work also with API calls. To me
having a derived ScDocFuncSend that implements virtual methods for the
collaboration case sounds logical, I think that in fact the receiving
end should act much like if it was called via API.

  Eike

-- 
LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20120801/551bd174/attachment.pgp>