Sorry, again, I'm very busy at the moment so I haven't read all this in detail.
But a quick comment about an calls to
Globals.curEditor().getLayerManager().getActiveLayer().getDiagram()
I would say that this is unsafe anywhere except in a Mode class or an
Action class where those classes are used correctly (in my view).
A Mode represents the mode the diagram editor is in while the user is
interacting with it (e.g. ModeCreate for creating and placeing a new
item, ModeSelect for selecting Figs). As the user is interacting only
with a specific diagram then the current diagram will be known to be
safe at that point.
Actions represent the user selecting some menu item or button etc and
within the execute method it would be expected that the current
diagram is known at the start of the execute (but some functionality
of the action may actually be changing that value).
If anything else requires that information it should be passed that as
an argument from the Mode or Action.
I refer above to 'correctly' used Action/Classes. I have seen places
where in non-GUI we do the likes of
ActionMyAction ma = new ActionMyAction();
ma.execute();
In my view this is improper usage. If there is code in an Action or
Mode that requires being reused outside the Mode/Action then it should
be moved elsewhere in the model and instead reused by the Mode/Action.
Regards
Bob.
2008/11/11 Tom Morris <[EMAIL PROTECTED]>:
> On Mon, Nov 10, 2008 at 4:13 PM, Michiel van der Wulp <[EMAIL PROTECTED]>
> wrote:
>
>>> Since we usually want the Diagram associated with the active window
>>> and we can use GEF to follow
>>> Globals.curEditor().getLayerManager().getActiveLayer().getDiagram() to
>>> get this we can break the dependency on the ProjectManager.
>>
>> So, the ArgoUML application has a singleton ("Globals") that keeps track of
>> the current editor and hence the current diagram. Hmmm...
>
> "Globals" is actually a GEF class, but yes. For things that know the
> current Layer (which can be gotten from Figs as well), there's no need
> to refer to the singleton. All our Layers are LayerPerspectives,
> which know the Diagram.
>
>> We can as well ask the current diagram what the current project is then...
>
> That wasn't in the public API, but I just increased its visibility, so yes.
>
>> One reason [to keep Project.activeDiagram] may be when you close the
>> application
>> (I.E. all projects at once)
>> and select "save all". Then we would need to know the last selected diagram
>> per project. BTW: We now do save the last selected diagram. Project B will
>> need to be saved with diagram B3 selected.
>>
>> And:
>> Suppose we have a multi-user & multi-project version of ArgoUML, so that one
>> user is working on the current diagram in project A, and the other user is
>> working on the current diagram of project B - so we may need a current
>> diagram per project per user.
>
> Restoring the editing context is useful, but the way that Eclipse does
> this is that it keeps track of what windows were open as part of the
> editing session. If you open/close a project independent of the
> session, no information is saved. If the current ArgoUML model is
> maintained it will probably need to be extended to support multiple
> open diagram windows, so activeDiagram becomes an ordered list of open
> diagrams.
>
> What I've done for now is deprecated all non-persistence uses of
> getActiveDiagram.
>
>> We may need to keep the Project.getActiveDiagram(), but should get rid of
>> ProjectManager.getManager().getCurrentProject() . Should we not deprecate
>> this now?
>
> I guess I can take the same approach and deprecate it except for some
> small number of uses (there are probably at least one or two valid
> uses of it). Just to put the scale of the problem in perspective
> though, there are 290 (!) uses of this in the set of projects that I
> have open in my current workspace (includes ArgoEclipse and modules),
> so so this is a non-trivial problem.
>
>>> On a related note, the constructors for four FigEdges are using this
>>> idiom to set their owning GEF Layer which seems hugely backwards to
>>> me. Anyone know why this is being done in the constructor instead of
>>> by the PGML parser? (The comments seem to imply that it's only
>>> required because of some special condition during load). The four
>>> Figs are FigAssociation, FigAssociationEnd, FigDependency, and
>>> FigGeneralization.
>>>
>>
>> See issue 3494 - unfinished because of architecture uncertainty.
>
> I had a look, but I don't understand how it applies. I agree that
> Figs should belong to a Layer (LayerPerspective in ArgoUML's case),
> but why isn't the PGML parser setting this up? Presumably it already
> created the Diagram & Layer, so it knows where the Fig goes. As a
> matter of fact, thinking about this a little more, with the old style
> loading algorithm where the currentProject wasn't updated until the
> entire loading process was complete doing
>
>
> setLayer(ProjectManager.getManager().getCurrentProject().getActiveDiagram().getLayer())
>
> is guaranteed to put the Fig in the wrong Diagram.
>
> I tried commenting out the setLayer code for these four cases and it
> didn't seem to introduce any problems. I'm wondering if the reason
> for this code has gone away (if there ever was one).
>
> Tom
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
