Friday 17 April 2009

Proposal 1: Class WARequest

With this posting I am starting a series of concrete proposals for improving Seaside. Of course, the documentation has absolutely the topmost and very first and highest priority, because:

There is never any excuse for not documenting!

In my view class WARequest is of the most important classes, especially in debugging and while getting to know the functionality of the system. This is why I start with this class.

I therefore propose to enhance this class as follows:

1) All responsibilities to WARequest
Among experienced Smalltalkers it should be out of any discussion that responsibilities about implementation details (how and where to store data) are solely responsibilities of the respective class.

In Seaside, this is not the case. Instead, the knowledge about internals of WARequest is regularly and frequently spread over many other classes.

See my example further below where a request is tested for some state. You will find many such tests in different classes and this is just very bad coding style.

It is programming at best with objects (or may be programming around objects) but not object-oriented programming!

Therefore, all such method that are checking or manipulating data inside WARequest must be moved into WARequest, which responds by a Boolean answers or via delivering or accepting the data.

2) Introduce states of requests
In the beginning it was extremely difficult for us to find out where what happens inside Seaside,which was worsened by the fact that no stringent naming conventions for the methods were used (that is another subject not for now).

This essential class WARequest must be enabled to answer its states, i.e. what type of request it is etc. All of this must be checkable via testing methods in a stringent syntax like "isReqestBlahBlah" so that it is easy to find out all available test methods.

3) Add a language code
As for the moment I'm leaving out all my heavy claims on this Anglo-Saxon ignorance about multilingualism. Also, the language code is even more important in WASession, but I will come to that later.

Add an instance variable to WARequest, which can be interrogated by all consumer methods, which need to process or query the requests instance. Many of these are language dependent (Anglo-Saxons: You are not alone in this world and this world is not yours!) and it is often simpler to ask the request instead of the session. Also, the requests can be often reached directly while the current session needs this fancy event, which is much more difficult to use while debugging (and also slower).

4) Add a starting and ending time
We have invested some work in a set of classes, which are recording not only the time but also the events (including menus, forms, errors, missing translations etc), which are processed through a request. In order to be able to report such data, the request must be able to deliver its starting and its ending time, which (in our case) will be recorded when the response is sent to the client.

5) Use setter and get a methods only!
I know that this is an academic subject, to which many Smalltalkers will oppose, but in our work we hardly ever use instance variables directly even inside a class. I personally consider this as bad style (with some very few exceptions like Semaphores). Getters and setters are very much better in many respects!

In concrete words, this does not mean what is implemented today: aRequest fields at: #someValue -> very bad!

But instead this means one of these three much better tractable versions:
a) aRequest fieldsAt: #someValue
b) aRequest get: #someValue
c) aRequest someValue
where the third one has the advantage that the implementation of the storage of this symbol inside WARequest is even better hidden. We have implemented b) and c) depending on the situation.

6) Accessors for nativeRequest
Often one needs to query the instance of nativeRequest, which sits inside WARequest. This is an implementation detail, which should be hidden inside WARequest without consumers (other classes) having to know this internal of WARequest.

Therefore, I recommend you to include accessor methods to WARequest for all services supplied by the nativeRequest, so that consumers only have to deal with WARequest.

7) Accessors for other internals
Of course, the same as stated in 6) is true for all other instances held by WARequest in its instances. It is never good to let foreigners (classes outside WARequest) know about implementation details inside another class!

To reming you: This is what is called encapsulation in the schooldays for Smalltalk. The authors of Seaside have forgotten most of these lessons.

8) All WARequest key fields on the class side
There are various keys handled essentially (not only but mainly) by WARequest. Currently, they are spread all over many places in several other classes. Very bad!

Create unique accessors with the stringent naming syntax (proposed: requestkeyForBlaBlah) on the class side of WARequest. All other classes where these keys are used, must refer to these unique identifiers. Alternatively and a little bit more liberally one could use these identifiers as symbols elsewhere, but I would prefer the first version (we have done it that way).

9) Testing methods for all WARequest keys
in order to ease the life of consumers of WARequest, there should be testing methods with the stringent syntax (isRequestForBlahBla) inside class WARequest. I've shown this already in my first example further below.

Résumée:

Please see this as my first very constructive contribution to improving Seaside.

I hope that the critical voices in the mailing list who knew nothing better then making their fun about my blogging will now start to understand slowly that I mean it very seriously and that my proposals come from a very long time in the software industry.

Best regards from Mr. Cucumber
(as you called me in the mailing list)
(my gilfriend happily agrees *ggg*)
...more to come soon!

No comments:

Post a Comment