Lecture de lib/engine/action/

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Lecture de lib/engine/action/

Julien PUYDT
Hi,

I finally found some time for ekiga yesterday evening and this morning.
It turns out my chat_opal_v314 branch was dead : it didn't rebase
cleanly and anyway it was not working and I don't remember anything
about it. I'll just start from scratch.

I saw my loudmouth code didn't compile anymore because of a new action
system : this morning I tried to understand it... so I have questions
(and critics...) :

(1) An action name has a global meaning... but one can have one "call"
action per presentity, so how does one know which one the global one is
about? The last which was clicked and declared it provided it?

(2) An action has two signals : enabled and disabled... which means it
can get back and forth to both states. But how is it related to the
lifetime management of objects? We don't want a dead object to be called
through an action, or a would-be-dead object to be kept alive because an
action somewhere still has a callback on it...

(3) I really think it is very bad that the base class Ekiga::Action has
enable/disable methods : the loudmouth presentity knows if it can be
called or not, nobody has to dictate whether or not this is possible!
The only thing a UI can ask is to trigger an enabled action, it doesn't
get to decide more than that! The UI's job is be pretty : let the
serious code handle the serious work!

(4) I really don't get what this private on_activated is about...

(5) Ekiga::ActionStore is too stupid : how can one put an UI on that?!
There's no signal to know when an action is
added/removed/enabled/disabled... for the last two I can get around that
by putting callbacks directly on the child actions (but then I have more
connections to manage), but for the first two? The Ekiga::RefLister api
is better in that respect : it was meant for dynamic situations.

(6) The fact that a base class isn't purely virtual is annoying... In
the rest of the engine, a base class Ekiga::Foo is purely virtual for
100% of cases, and an Ekiga::FooImpl is provided with an implementation
to cover 99% of use cases. Don't get in the way of the last percent!

(7) The difference between Ekiga::Actor and Ekiga::ActionProvider eludes
me...

(8) Ekiga::Actor has a friend on GActorMenu : that is a huge design
flaw, as that means the base code isn't GUI-independent anymore!

It's hard to get back :-(

Snark
_______________________________________________
ekiga-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/ekiga-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: Lecture de lib/engine/action/

Damien Sandras
Hi,

Not much time to enter a lengthy discussion right now.

My generic answer would be that "it just works", which is already a good starting point.

(3) I really think it is very bad that the base class Ekiga::Action has 
enable/disable methods : the loudmouth presentity knows if it can be 
called or not, nobody has to dictate whether or not this is possible! 

You are right, I will provide a fix.

(5) Ekiga::ActionStore is too stupid : how can one put an UI on that?! 
There's no signal to know when an action is 
added/removed/enabled/disabled... for the last two I can get around that 
by putting callbacks directly on the child actions (but then I have more 
connections to manage), but for the first two? The Ekiga::RefLister api 
is better in that respect : it was meant for dynamic situations.

You don't have to play with the ActionStore, it is an internal variable. Just play with the Actor code.

(6) The fact that a base class isn't purely virtual is annoying... In 
the rest of the engine, a base class Ekiga::Foo is purely virtual for 
100% of cases, and an Ekiga::FooImpl is provided with an implementation 
to cover 99% of use cases. Don't get in the way of the last percent!


Yes, and I tend to disagree with that design which is too restrictive. I have already discussed this design with many people
whose job is to program and design object-oriented code and they all told me that it was really a weird design. However, it
just works, so that's ok for me now.

(7) The difference between Ekiga::Actor and Ekiga::ActionProvider eludes 
me...

iirc an ActionProvider can provide Actions to Actors.

(8) Ekiga::Actor has a friend on GActorMenu : that is a huge design 
flaw, as that means the base code isn't GUI-independent anymore!

Indeed, that needs to be changed and it is in my personal TODO.

--
Damien SANDRAS

Ekiga Project
http://www.ekiga.org

_______________________________________________
ekiga-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/ekiga-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: Lecture de lib/engine/action/

Julien PUYDT

Le 05/01/2015 11:46, Damien Sandras a écrit :

>> (6) The fact that a base class isn't purely virtual is annoying... In
>> the rest of the engine, a base class Ekiga::Foo is purely virtual for
>> 100% of cases, and an Ekiga::FooImpl is provided with an implementation
>> to cover 99% of use cases. Don't get in the way of the last percent!
>
> Yes, and I tend to disagree with that design which is too restrictive. I
> have already discussed this design with many people
> whose job is to program and design object-oriented code and they all
> told me that it was really a weird design. However, it
> just works, so that's ok for me now.
>

Uh... they never heard of separating the interface from the
implementation? They never found it useful to quarantine code behind an
interface so deps don't spread to the whole code base? They never used
an interface to make polymorphism work?

I don't mean the idea of an action system is bad, it's just that I'm not
convinced the current framework is sound. Notice that the current
LiveObject framework with the MenuBuilder is already making it possible
to export actions in a very dynamic way, so it has good things. It does
lack centralization... I think the answer to my question (1) would help
me understand more precisely what you want.

Snark
_______________________________________________
ekiga-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/ekiga-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: Lecture de lib/engine/action/

Damien Sandras
Le lundi 05 janvier 2015 à 20:35 +0100, Julien Puydt a écrit :
Le 05/01/2015 11:46, Damien Sandras a écrit :
>> (6) The fact that a base class isn't purely virtual is annoying... In
>> the rest of the engine, a base class Ekiga::Foo is purely virtual for
>> 100% of cases, and an Ekiga::FooImpl is provided with an implementation
>> to cover 99% of use cases. Don't get in the way of the last percent!
>
> Yes, and I tend to disagree with that design which is too restrictive. I
> have already discussed this design with many people
> whose job is to program and design object-oriented code and they all
> told me that it was really a weird design. However, it
> just works, so that's ok for me now.
>

Uh... they never heard of separating the interface from the 
implementation? They never found it useful to quarantine code behind an 
interface so deps don't spread to the whole code base? They never used 
an interface to make polymorphism work?


No, they tend to disagree on other parts of the design. That is, frequently, overly complex for
what we try to achieve, or overdesigned with too much abstraction.

Separating some parts of the implementation from the interface is useful when the code is templatized
as it makes the compilation lighter than having the template directly in the base class.

There are also other flaws to the approach like the fact that the interfaces are sometimes too basic and
leave the job to the specific implementation. In some cases, there is only one method in the interface.
That does not make much sense.

I don't mean the idea of an action system is bad, it's just that I'm not 
convinced the current framework is sound. Notice that the current 
LiveObject framework with the MenuBuilder is already making it possible 
to export actions in a very dynamic way, so it has good things. It does 
lack centralization... I think the answer to my question (1) would help 
me understand more precisely what you want.

MenuBuilder was tied to the concept of menu. We are now architecting things behind Actions,
not Menus.

I propose to keep the current Action code intact and to work on the parts of Ekiga that really need
work.

As a second step, and after the current release, I would like to work on the improvements and simplifications
of the engine.

To make it clear : I won't discuss further about the engine before we release 5.0. And I won't accept any big change
to the current code. We need to make progress and concentrate on what matters.
--
Damien SANDRAS

Ekiga Project
http://www.ekiga.org

_______________________________________________
ekiga-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/ekiga-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: Lecture de lib/engine/action/

Damien Sandras
In reply to this post by Julien PUYDT
Le lundi 05 janvier 2015 à 20:35 +0100, Julien Puydt a écrit :

lack centralization... I think the answer to my question (1) would help 
me understand more precisely what you want.
"(1) An action name has a global meaning... but one can have one "call" action per presentity, so how does one know which one the global one is about? The last which was clicked and declared it provided it?"

There are two different things:
1) The Ekiga Engine abstraction: An Actor provides Actions. Actions are local to the given Actor. They are not global and there is no central "store". This is very similar to what the LiveObject was doing. A bit later, we could turn the LiveObject into an Actor and get rid of MenuBuilder. A presentity is thus an Actor, and each presentity declares its own Actions.

2) The GLIB implementation (GActorMenu): You simply construct a GActorMenu by constructing it with an Actor as argument. The GActorMenu then registers the actions into the GLIB GUI abstraction system where they are global (or local to your specific window).

This is very simple to use and has one big advantage : it is automagic, and you can use GUI actions anywhere you want if you know they exist (like "Call", "Transfer", ...).

Yes, we finally have a Call button users can click when a Contact or a Presentity is selected without the need to right-click on the Contact/Presentity and select "Call" in the MenuBuilderGtk. That's one of the many improvements !
--
Damien SANDRAS

Ekiga Project
http://www.ekiga.org

_______________________________________________
ekiga-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/ekiga-devel-list