[pedevel] Re: Suggestion for refactoring of PText
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: pedevel@xxxxxxxxxxxxx
- Date: Sat, 12 Apr 2008 21:40:48 +0200
On 2008-04-12 at 17:41:20 [+0200], Oliver Tappe <pedevel@xxxxxxxxxxxxxxx>
wrote:
> On 2008-04-12 at 02:02:49 [+0200], Ingo Weinhold <ingo_weinhold@xxxxxx>
> wrote:
> > On 2008-04-11 at 23:21:49 [+0200], Oliver Tappe <pedevel@xxxxxxxxxxxxxxx>
> > wrote:
> > >
> > > today I have thought about how we could separate the functionality
> > > contained
> > > in PText into several classes and here's what I have come up with:
> > >
> > > A - The Basic Text Model
> > >
> > > CTextBuffer (was: PTextBuffer)
> > > - exists once per file
> > > - implements a gap-in-the-middle buffer abstraction (allowing swift
> > > consecutive insertions at the same place)
> > > - supports inserting and deleting text
> > > - manages a list of listeners which will get informed about changes
> > > synchronously (by method call)
> >
> > Nice so far. The synchronous listeners are something that needs some
> > consideration, I guess. Since the object will be shared, it needs locking.
> > Usually one informs synchronous listeners with the lock being held (since
> > otherwise you get race conditions, when changes are made by several
> > threads
> > at almost the same time), which results in good deadlock potential, if the
> > listener requires locking itself. One can, of course, always use an
> > adapter
> > listener that sends out asynchronous messages to work around that problem.
>
> Ah, thanks for the hint. I suppose using asynchronous listeners would cause
> less headache concerning the communication between the listener and the
> corresponding view, too: with messages, the processing of the changes would
> be
> done by the view's (window) thread, so there would be no need to explicitly
> lock the view when reacting on changes to the model.
Yep, and, that was my main concern, you'll automatically get a consistent
locking order (BWindow -> CTextBuffer).
> > I believe for eXposer we used a different approach. The model would
> > associate
> > a queue with each listener in which it stored the changes pending for that
> > listener, and send out a notification message whenever it pushed a change
> > into an empty queue. When receiving the message the listener would
> > read-lock
> > the model and process the changes in the queue. Changing the model was
> > done
> > synchronously with the write lock being held.
>
> And the listener would empty the queue after having processed all items,
> which
> is safe because the read-lock protects the queue from being changed by the
> model and all other listeners deal with another (their) queue, right?
Yep, each queue totally belongs to its listener. Read-locking the model makes
sure that the model doesn't change (and thus push new queue items) while the
listener empties its queue. If the listener itself is multithreaded, it has
to use a separate lock, of course.
> I very much like the idea of having a queue for each listener as that would
> reduce the number of messages to handle in busy phases.
Absolutely. For a text editor that usually shouldn't be a problem, since the
user won't manage to cause more than a few model changes per second. With
scripting support things might look a little different, though.
> > With synchronous listeners this
> > could, of course, be implemented in the listener itself.
>
> Hm, just trying to understand: that would still have the increased risk of
> deadlock, right?
Sorry, my train of thought was a bit confusing. I guess summed up what I
wanted to say is that synchronous listening is fine as long as you use
asynchronous adapters for individual listeners.
Assuming CTextBufferListener is the synchronous listener interface that
CTextBuffer uses, then listeners that don't have locking problems and only
need to perform a quick task can still use it directly (i.e. synchronously).
Listeners that have to do more, but don't really need to synchronize with the
changes, can use a CTextBufferMessengerListener subclass, that sends a
message on every change. Listeners that do need to synchronize would use a
CTextBufferChangeQueueListener, that implements the change queue feature.
So CTextBuffer doesn't need to implement the fancy mechanism itself; it can
be enabled per listener. It would make sense, if CTextBuffer already wrapped
change events CTextBufferChangeEvent class instances, which would be
immutable and ref-counted, so that they can be shared by different change
queues (to save memory and CPU time in case the change is a big insert or
delete).
I hope that clears it up.
> [ ... ]
> > Mmh, this doesn't really convince me yet:
> >
> > * If CTextStructure interacts with CTextBuffer it can as well provide all
> > of
> > the features the former one has, namely also inserting and deleting of
> > text.
>
> Yes, it can, of course. It's just that I had this idea about keeping the
> functionalities as separate as possible and to make it possible to have a
> read-only controller (viewer), too. But I guess this doesn't make much sense
> for Pe, and these classes are not going to be used anywhere else anyway.
I agree. And if you really want to do that you would consequently start with
a read-only CTextBuffer interface. A CMutableTextBuffer class would add the
editing functionality. Consequently you'd also have a CTextStructure and a
CMutableTextStructure, and so on. But that will be quite a bit of additional
work for a feature that will likely never be needed.
[...]
> > As uninspired as it may sound, I would fold the whole hierarchy into a
> > single
> > class. Or maybe keep CTextStructure separate, but move the basic text
> > editing
> > functionality (insert, delete, replace) from CTextEditor to it.
> >
> > I just don't think inheritance offers many advantages -- actually I find
> > it
> > quite fuzzy what functionality to implement at what level.
>
> Well, I actually thought that the strict separation of functionalities makes
> that part rather clear, but if it has not come across that way, I'm
> obviously
> wrong :-/
No, your proposal made perfectly clear what functionality you would implement
at what level. I just saw no stringent reasons for assigning the features to
levels the way you proposed (e.g., as mentioned, searching could as well be
moved to any other level). And that's quite a good indication, that
inheritence is not the right tool for the job.
> > It would be easier
> > with aggregation: CTextStructure could provide a structural view on a
> > CTextBuffer. CTextSelection and CTextSearch would be orthogonal classes
> > working on a CTextStructure, and CTextEditor would integrate the whole
> > thing
> > and provide a nice interface for interested parties.
>
> Yep, aggregation-by-composition with CTextEditor (more or less) following
> the
> "facade" pattern. My next suggestion will be based on that - see the
> following
> mail.
Great! :-)
> > > C - Views
> > >
> > > PEditView (was: PText)
> > > - uses all functionality models mentioned above to implement the
> > > edit
> > > view
> > > - translates key/mouse events to appropriate method calls on the
> > > models
> > > - renders text for editing on screen
> > >
> > > PPrintView
> > > - sets up CTextStructure tas appropriate for printing (activates
> > > wrapping)
> > > - renders text for printing (less color, hidden invisibles)
> > Sounds good.
>
> With the asynchronous messaging model, these views would implement a
> specific
> interface that the functionality controllers would use to tell the view
> about
> the changes resulting from a change in the model (dirty lines). Each view
> could
> then react by redrawing if it is required (if any dirty lines are visible).
Yep.
> > > + split views (would be implemented by opening a second view on the
> > > same
> > > CTextBuffer
> >
> > You mean the hacked-in support for split view, I suppose. I.e. the feature
> > itself would look like before, right?
>
> Nope. During and after the refactoring, that feature will be gone
> altogether.
> We could then decide what kind of support we'd like to have for multiple
> views
> on the same document (several windows and/or multiple views in one window).
> Both should be rather simple, by then.
OK, fine by me. I suppose I already mentioned often enough that I would
happily trade the "split view" for a "multiple windows" feature. :-)
CU, Ingo
Other related posts:
