Dirty KoCreateShapeStrategy

[Jonathan Schmidt-Dominé - Developer]

Hi KOffice-devs! (especially thomas and zagge)

When I look at the KoCreateShapeStrategy-constructor I see really dirty code:
A temporary shape is created to determine the outline. That's relly bad:
-Shape-creation could have heavy side-effects. Imagine a shape-plugin to
display Maxima-/Cantor-sessions. (e.g. TeXmacs supports such a feature)
Starting such a session would be a really heavy side-effect and it would be
done twice.
-Or imagine an automatically generated table of figures: A picture would be
inserted and removed: a lot of duplicated work to update the table.
-Such plugins could workaround it by checking if it's just a temporary, but
that would be new ugly code and that's not the task of a plugin.
-So KoCreateShapeStrategy should workaround it.
Why not create a shape, hide it, show it, when it is placed, and delete it if
the creation is aborted?

I have already discussed with ingwa and CyrilleB in IRC, but they've not
written the class.

Jonathan/The User

------------------------
Automatisch eingefügte Signatur:
Es lebe die Freiheit!
Stoppt den Gebrauch proprietärer Software!
Operating System: GNU/Linux
Kernel: Linux 2.6.31-ARCH
Distribution: Arch Linux
Qt: 4.5.1
KDE: 4.3.71 (KDE 4.3.71 (KDE 4.4 >= 20091007))
KMail: 1.12.90
http://gnu.org/
http://kde.org/
http://windows7sins.org/
_______________________________________________
koffice-devel mailing list
koffice-devel@...
https://mail.kde.org/mailman/listinfo/koffice-devel

On Sunday 1. November 2009 22.27.34 Jonathan Schmidt-Dominé -
Developer wrote:
> Hi KOffice-devs! (especially thomas and zagge)
>
> When I look at the KoCreateShapeStrategy-constructor I see really dirty
> code: A temporary shape is created to determine the outline. That's relly
> bad: -Shape-creation could have heavy side-effects. Imagine a
> shape-plugin to display Maxima-/Cantor-sessions. (e.g. TeXmacs supports
> such a feature) Starting such a session would be a really heavy
> side-effect and it would be done twice.

If the constructor is taking a lot of CPU time, I'd say the shape is
misdesigned.

> -Or imagine an automatically generated table of figures: A picture would
> be inserted and removed: a lot of duplicated work to update the table.

The shape is never added to a document so no table of figures ever learns
about this shape. This is not an issue.

> -Such plugins could workaround it by checking if it's just a temporary,
> but that would be new ugly code and that's not the task of a plugin.

The shape should never actually be added to a document and thus no
workarounds are needed.

I don't think the problem you indicate is really a problem.
--
Thomas Zander
_______________________________________________
koffice-devel mailing list
koffice-devel@...
https://mail.kde.org/mailman/listinfo/koffice-devel

[Jonathan Schmidt-Dominé - Developer]

Hi!

> If the constructor is taking a lot of CPU time, I'd say the shape is
> misdesigned.
At the moment I've actually no problem with it, I just saw it when debugging a
shape and I think for some useful shapes it would be really bad.
The CPU time of the constructor could be short, but it could be possible that
it invokes other threads.

> The shape is never added to a document so no table of figures ever learns
> about this shape. This is not an issue.
And how should the shape recognize if it was added to the document or just
added to get the outline?
> > -Such plugins could workaround it by checking if it's just a temporary,
> > but that would be new ugly code and that's not the task of a plugin.
>
> The shape should never actually be added to a document and thus no
> workarounds are needed.
It has to use a workaround. (checking the parent, constructing when init is
invoked or something like that)
> I don't think the problem you indicate is really a problem.
Well, a pro for the double-creation is: At the first creation the shape could
leave the heavy stuff out using extra-checkings. With single-creation it would
do the stuff, too, when the shape is not actually added. (cancelled action)
But on the other hand the init-method could be used and with a single-shape
you could just save CPU-time.

Jonathan

------------------------

Automatisch eingefügte Signatur:
Es lebe die Freiheit!
Stoppt den Gebrauch proprietärer Software!
Operating System: GNU/Linux
Kernel: Linux 2.6.31-ARCH
Distribution: Arch Linux
Qt: 4.5.1
KDE: 4.3.71 (KDE 4.3.71 (KDE 4.4 >= 20091007))
KMail: 1.12.90
http://gnu.org/
http://kde.org/
http://windows7sins.org/

_______________________________________________
koffice-devel mailing list
koffice-devel@...
https://mail.kde.org/mailman/listinfo/koffice-devel

[Jonathan Schmidt-Dominé - Developer]

Hi!

I just want to summarize my impression:
-The code is certainly dirty (it does not reflect the semantic structure)
-The code is not as slow as I thaught, because heavy constructing stuff should
be done in activate()
-In my point of view it would not be a problem not to delete the shape, that
would be cleaner and maybe a bit faster (it should nearly always imply extra
CPU-time to allocate and create an object twice)

Jonathan

------------------------

Automatisch eingefügte Signatur:
Es lebe die Freiheit!
Stoppt den Gebrauch proprietärer Software!
Operating System: GNU/Linux
Kernel: Linux 2.6.31-ARCH
Distribution: Arch Linux
Qt: 4.5.1
KDE: 4.3.71 (KDE 4.3.71 (KDE 4.4 >= 20091007))
KMail: 1.12.90
http://gnu.org/
http://kde.org/
http://windows7sins.org/

_______________________________________________
koffice-devel mailing list
koffice-devel@...
https://mail.kde.org/mailman/listinfo/koffice-devel

Hello Jonathan,

> -In my point of view it would not be a problem not to delete the shape,
>  that would be cleaner and maybe a bit faster (it should nearly always
>  imply extra CPU-time to allocate and create an object twice)

You are right, this should be easily fixable. One thing you need to make sure
is that the shape is deleted when the insertion is canceled by the user. If
you want to give it a try a patch is welcome.

Thorsten
_______________________________________________
koffice-devel mailing list
koffice-devel@...
https://mail.kde.org/mailman/listinfo/koffice-devel