Tuesday 28 April 2009

I fucked-up Seaside again

I have made it again: I fucked-up my entire Seaside application! Rien ne vas plus!

This block has much of a diary, because I'm reporting my daily experiences with the Seaside web server library.

One of the main problems, aside from the many others, is that the logic and the control flow is so extremely scattered over different classes and very inconsistent terminology so that it is extremely hard to track down errors.

I had to make a couple of changes related to the special types of requests. As a consequence major features don't work anymore. This alone would not be a problem if it wasn't so very extremely difficult to follow the flow of actions that Seaside performs.

These problems are essentially caused by these characteristics of Seaside:

1) Very scattered responsibilities
Several different classes perform actions directly on instance variables of others, in this case of the WARequest (see also my post "Proposal 1: WARequest"). This makes it very difficult to track down what happens where and why.

Because of this alone Seaside is not really object-oriented, as the law of encapsulation is regularly violated.

2) Inconsistent terminology
Especially class and method names are inconsistent and often misleading. This makes it even more difficult to understand what is going on where.

3) Debugging is extremely difficult
And it's also very time-consuming, because Seaside always accesses the current session via signalling. This can be seen as a nice technical feature but it makes the developer's work very inefficient. More about this soon in a separate post.

I am not yet sure about the following statement, because I'm still (after several weeks) in the phase of learning Seaside, but I have the strong feeling that there are simpler, more concise and probably also more efficient ways of accessing the current session.

If I had known the full scope of these problems when the decision for Seaside was made, we would not have decided to use Seaside! It's a nightmare!

Don't expect censored language from me. I am not US American and I detest
self-censoring called "political correctness". In Europe most people still speak out the truth!

----------------------------------------------------------------------------------------
Update: Problem solved!

Here is the solution: What caused yesterday's confusion was written by myself and therefore, of course, my own mistake.

However, this was still an indirect consequence from Seaside's distributed responsibilities regarding the handling of request types. To avoid this, I have added a couple of methods to class WARequest, because this class should be the only one to know about the internals and types of its data. When writing these additional methods I had a misconception about the meanings and priorities of key fields, which are spread all over other classes in the original Seaside code.

Because I consider this situation very important, I am giving a practical example in a separate post.

8 comments:

  1. If you're making changes directly in WARequest, you've created a huge mess for yourself. Why aren't you creating components and working with those?

    ReplyDelete
  2. No Jarober, that is certainly not correct - not in general (see Proposal 1) and even less for us. More tonight in an update here and probably in one or two more posts.

    Spreading WARequest responsibilities over other classes is one of the worst (and foolish) design mistakes in Seaside. This is very low "object-orientability", i.e. no encapsulation of knowledge and behaviour.

    ReplyDelete
  3. It sounds to me like your proposal would end up creating what's known as a "god object". Having created more than one of those myself (I have one in BottomFeeder), I can tell you that they aren't a good answer...

    ReplyDelete
  4. Jarober,

    The term "god object" is new to me so I cannot comment that. I certainly don't want some kind of central library in one class.

    If you look at my proposals 1 and 10, I think the arguments are clear and should be convincing.

    By the way, the same is true for a couple of other classes whose implementation details were launched to consumer methods in other classes.

    All I want is the encapsulation of data and code in one class ("object"), which includes behaviour, implementation details, keys etc.

    Today in Seaside this is widely spread over many classes.

    ReplyDelete
  5. It's widely spread because the responsibility is widely spread. In general, a class should do one thing. A "god class" is one that does many supposedly related things. The problem with such classes is that they get too big, have too many responsibilities, and get referenced all across a system because of this. The end result: refactoring is made much more difficult, and changes to the "god object" are harder, because any change impacts so many other behaviors.

    ReplyDelete
  6. Well, Jarober,

    I would prefer to stick to this concrete example, because general design discussions are certainly interesting but my concern is to get Seaside running at all and to have it better designed for the future.

    Class WARequest should only accumulate those responsibilities, which result from its data and its purpose in life.

    And making use of these responsibilities should happen only through protocols, which hide implementation details from the consumers.

    Currently, this is not the case in Seaside.

    For example, the instance variable 'fields' is directly interrogated inside consumer classes, which even know about the keys and about how these keys are used inside 'fields'. This are distributed responsibilities, which make re-factoring or enhancing WARequest multiple times more difficult and dangerous and this is completely in contrast to your statement.

    Knowing about the contents and the usage of "fields" is exclusively the responsibility of the holder class, in this case WARequest.

    In our implementation there are in the meantime a few additional logical conditions, which are calculated from more than one key in "fields". The knowledge of these details must be hidden from any other class. Only the testing protocols, something like "isTypeOfBlahBlahRequest", are available to senders (I don't mean technically available but logically).

    With the implementation proposed by me, any change or enhancement would solely be done inside WARequest and none of the corresponding classes (senders) would be affected. Today, any change affects a lot of senders of low level protocols, which must at minimum be checked by the developer and tested again.

    Many good chances for bugs!

    Also, the consumers of certain functionalities or data atoms can be tracked much easier in my form of implementation. In the current form, one has to look at all senders of the method "fields" and there happen to be a lot more in our software beyond Seaside (but this argument would also be true if this wasn't the case).

    Provided that we are really talking about the same subject, I would be very surprised that you are preaching some design so fundamentally contrary to mine (saying this I assume that you have a lot of Smalltalk experience). From my experience I know for sure that software designed like Seaside today is multiple times more complicated and labourious to maintain and enhance compared to my design that we have implemented in our application software. And it is also much more difficult to read and to understand.

    ReplyDelete
  7. Hmm. So I fired up a Seaside image, and looked for all senders of #fields in the Seaside namespace. There are 9, one of which is in an example application (WABrowser). Of those, 2 have hardcoded strings in them - and they are used in 6 different classes (again, one of those is an example application).

    Seems to me that you've created an enormous mountain out of a molehill. There's no reason for you to work in WARequest; If you have arguments being passed in the url, you can look for them in #initialRequest: in your component. There's an API for that:

    aRequest>>at:ifPresent:

    I'm really not sure why you would be mucking around in WARequest directly, nor do I know why you think there's lots of senders of #fields.

    ReplyDelete
  8. In my view, the number of of references to this example instance variable "fields" is little relevant. Much more important is the general distribution of low-level protocols and responsibilities into consumer methods of other classes implemented in Seaside.

    Also, WARequest is just one example for this, in my view, bad design, which has been implemented all over the place within Seaside. By the way: I remember that some of the accessors to keys used in "fields" were also implemented in several different classes. Another no-no.

    I consider sending low-level protocols like at:ifPresent: or at:ifAbsent: from a foreign class a violation of encapsulation principles. This is never allowed in implementations under my supervision and I have made very good experience with sticking to such principles.

    I can assure you that we have already had a couple of good reasons to enhance WARequest unless we wanted to continue with distributing knowledge about implementation details into consumer methods of other classes.

    It seems that we have a very different view of implementing protocols and encapsulating responsibilities.

    Honestly, calling the use of at:ifPresent: an "API" is in my view, I beg your pardon, ridiculous, because it happens to be at the lowest possible level and it is exposing details, which should only be handled by the owner class.

    In some other programming languages these would be implemented as private methods and for such situations it would be useful if Smalltalk would support this, because it requires much discipline to avoid what you obviously regard as being normal. And few programmers have discipline.

    I hope that this is not the same reason why there seems to be very little to no understanding for my critics regarding the missing documentation. And documentation always means in-code documentation in the first place.

    Probably all of this is due to cultural differences, because assuming you that you are from the USA, you have a more easygoing approach compared to central Europeans (especially from the Germanic countries, Romanics and Slavs are different again), which, as always in life, is sometimes good (in my view in sales) and sometimes less advantageous (in my view in engineering).

    This is why, as a rule of thumb, US people are better in sales and Swiss or Germans much better in engineering. Taking the industrial situation in account, which has finally become obvious, there is no need to waste time in discussing, which is the better way. Another general statement that you probably don't like to hear: It would help North Americans tremendously if they changed their attitude towards other cultures from trying to impose their own low-level to adopting the best from others (very generally meant, although I would also know some examples from the Smalltalk arena).

    I have no time to follow this up but I would still be interested in knowing what the early conceptual designers thought about this design question. I regard these "forefathers" as multiple times wiser than most of the younger followers (as an example see Bryant's arrogant comment on my praise to the quality of the early class library on the Seaside mailing list "25 years old code...").

    So, we agree to disagree!
    My greetings!

    ReplyDelete