[haiku-appserver] Re: invalidation again

Hi,

Stephan Assmus wrote:
> Ok, so what is RootLayer::fUpdateReg (for example)?

        Leftover. :)

> I must say I don't like the design with the "current layer". I think 
> that the layers and the data they carry is munched together with the 
> managing of multiple layers. This is the confusing part. So inside an 
> instance of layer, there is code concerning that layer (instance), and 
> then there seems to be code as if Layer was a static class. You set 
> some member variable of one layer instance, to manage another layer. So 
> it is completely confusing, which layer we're actually updating or 
> changing. You don't see this from the code, it just "happens" to work 
> if the client code knows what it is doing. This is not necessarily good 
> code, the correct usage should be enforced by the design, this would 
> make it all much more usable. And I think most of the bugs come from 
> client code not actually doing all the necessary things in order for 
> the mess in Layer to "magically" work correctly.

        Believe me I can understand your frustration. I know that piece of code 
is not easy to understand. I wrote that code with only one objective in 
mind: speed. If that I sacrificed the beautifulness of C++: the OOP design.
        I have tried, and succeeded to shoot more than one rabbit at a time. In 
the same loop I managed to calculate the visible regions, the  regions 
which should be redrawn, to resize and move all child layers, to create 
a list of regions who should be blit on screen _no matter_ the resize 
directions of a layer. (Note that for center and right aligned view, R5 
invalidates the whole view's visible area. I only invalidate what really 
needs to be redrawn).

> Ok, I don't like to "preach" or anything, so please don't take the 
> following that way. Take it as someone stating his experience, you may 
> agree or not agree. So here it goes:
> 
> In my experience, this is a good aproach to object oriented 
> programming: Usually, a class contains data and functions to manipulate 
> that data. Most of the times, there are multiple bits of data, which 
> depend on each other in some way. Certain combinations of the data 
> values can be considered "valid", other as "invalid". Now, the trick is 
> to enforce the "valid" combinations of data inside the class. So that 
> client code which uses the public methods of a class *can not* bring 
> the object into an "invalid" state.
> As soon as you design your code so that you depend on the client code 
> to keep the data valid, you have a number of problems:
>       * You open up a lot of possibilities for bugs
>       * You make the code hard to understand, because you don't know 
> which part of the client code might mess with something and cause an 
> invalid state, you cannot enforce "code paths".

        Yes, I very much agree. I have read books about C++ and how you should 
program it so that the code should be clean, easy to understand and 
portable. However in my (not that vast) experience and form some books 
that I read, to achieve performance you have to do some sacrifices. 
That's what I did in this case.

        That I did not do with the window manager code. If you have a look 
there you will see it's very beautiful written (IMO). :-)

> I know, you have all this "but it needs to be fast" stuff in mind.

        Yup.

> But I 
> think the current app_server design is completely overdoing this, for 
> _no_ good reason.

        If you are talking about this piece of code only, then I agree. If you 
talk about the general app_server design, I _certainly_ do not agree!

> I think a lot of the problems can be contributed to 
> this fact. So while you work on something, please try to make the extra 
> effort to refactor things. It will save so much time in the end.

        I did that, I will do that.
        You have no idea how time it took me to put all those pieces in a 
single loop. I did took and extra effort to refactor things for that code.


bye,
Adi.

Other related posts: