Proposal 12: Rename WATagBrush "class"

A CSS "class" is not real a class in the sense of object-oriented design. It just has "same taste" of a class.

I find this term in Seaside misleading and somewhat confusing for a Smalltalker.

Therefore, it would be much better to call this attribute and its accessors "cssClasses:". This tells exactly what it is:
a) CSS related
b) More than one are possible.
c) The term "class" is used so often in Smalltalk that this alone is another reason to distinguish this special cssClass(es).

Note: It is not good to adopt existing terms even if (or because?) they are widespread, if they are imprecise.

Proposal 11: Ajax requests without callbacks

There are many reasons supposedly in most Seaside applications to use Ajax requests without storing callbacks. In our application we have a couple of such cases where it makes no sense to store callbacks and where callbacks cause useless overhead.

This is another reason why the class WARequest of Seaside should be enhanced, as proposed by me earlier, by request type identifiers and testing methods.

In our case, at least one of these situations will have very heavy traffic and we can therefore intercept the logic flow at a very early stage and direct it immediately to appropriate actions without having to go through the many nested loops in Seaside, which make it also very difficult to follow up logic in debugging.

Requests without callbacks save a huge amount of overhead, CPU time and memory and the make debugging easier, too.

I therefore propose:
a) Add request identifier methods to WARequest (see proposal 1)
b) Add support for Ajax requests without callbacks.

English is a difficult language...

...apparently for some Seaside developers in the USA!

Yesterday I stumbled over the method "initialRequest:", which is implemented in a few classes. Being fairly capable of reading English, I thought that some initial request was processed. When looking at the code I found out that this was completely wrong.

In reality, the receiver is initialized with a request. Which is something very different, of course.

Now, it is indisputable that most US Americans have castrated Shakespeare's originally beautiful language to the absolute minimum and have deformed it to a horribly harsh pronunciation.

However, not seeing the difference between "initial" and "initialize" is what we in Europe would call a Freudian mistake, which leads the Seaside user into a completely wrong direction.

Therefore, I am strongly recommending these language-poor developers to more carefully select their method names and to have better somebody look at the chosen names who is still capable of using Shakespeare's language properly.

Otherwise one must assume "poor language - poor thinking"!

My proposal: Please change this misleading method name to something like "initializeFromRequest:", which really expresses what is done.

Proposal 10: Move it all to WARequest

Handling requests is one of the most important atomic tasks of Seaside, because this governs all the rest.

Currently, the knowledge about requests is scattered over different classes. Just two example out of many:

WABrowser -> initialRequest
aRequest fields at: 'class' :.............
(A string used as key and, even worse, one which is impossible to track, because there are so many.)

WAApplication handleRequest:
aRequest fields at: self handlerField..........

This violates encapsulation and it's no object-oriented design. Only the class WARequest should know about its internals, it's data, keys etc. and it should be interrogated by other classes via clear protocols, which hide all internals from these other classes.

I have mentioned this before, but because this bad design was essentially the cause for having to search for a bug yesterday I am bringing this up again.

Proposal 9: Improving Seaside class trees

There are different types of requests handled in Seaside and at least two types are handled by classes in completely separate class trees. This is true (at least) for WARequest and for HTTPGet, which essentially do the same (respond to user requests) but are completely separate and share no code at all.

Of course, I understand that this has probably historical reasons as one is from Seaside and the other one probably from Swazoo and that these are separate projects.

Nevertheless this is no good design and it should be changed over time.

We had to enhance various classes with the same code (how uly) in order to fulfill some of our needs. No chance to inherit. Several changes were (and probably still will be) related to these two essential classes.

We had to duplicate various code to achieve this. And redundant code is something we seriously hate!

Unfortunately, this is another prove that Seaside has a very low degree of "object-orientability".

Also, it would be most useful if the protocols between these two different but very closely interconnected class trees were aligned (but please use some better semantics than currently in Seaside).

Proposal 8: Use symbolic file names

Not only is the current file handling implementation of Seaside very inefficient (see proposal 7), but it also requires sending long path names to the browser and back.

This makes little sense, it is wasting bandwidth and CPU performance in an environment, which needs the fastest responses possible.

In the current implementation Seaside is not fast.

This implementation also makes the software relatively inflexible, because practice shows that people tend to include these absolute path names in strings, that constitute CSS or JavaScript. (This is not the fault of Seaside, of course, but a normal human reaction resulting from Seaside's design.)

This problem could be very easily solved as a side-effect of my proposal 7, if one only sends the real or symbolic file names without any paths to the browser.

When a file request comes in, these symbolic file names (of CSS, JavaScript, images etc) are used for a look-up in the dictionary mentioned in proposal 8. The library path and the related method name come from this dictionary. All it needs is one:
classFromDict perform: methFromDict and it's all done.

- Much faster than today
- Much simpler to implement and change
- Shorter file strings sent out (not a must, but de-facto)
- Symbolic file names can be switched depending on user or system settings
- Simpler to debug
- and a few more.

Of course:
- The new static must be reset before starting an app
- It must provide lazy init (see my post: "To lazy for lazy initialization")

Proposal 7: On inefficient Seaside file handling

During my work today I detected how inefficiently Seaside handles file requests.

I was deeply shocked to see this highly complicated implementation, which over and over again makes the Smalltalk VM busy with inefficient operations, which could easily be handled in a much simpler fashion.

It does not need to be discussed that a web server must be extremely efficient and fast. This is not the case for Seaside!

As I was corrected by a comment, Seaside is not a web server in the precise technical sense. That is correct. Nevertheless, one generally speaks also of PHP as a web server although typically the actual server work is done by a front-end Apache. Therefore, I still stick to this term "web server" for Seaside even if it is not absolutely precise.

When a file requests comes in, the class WAFileHandler loops recursively through all the subclasses of all of those library classes, which were registered as "root classes" and executes aBlock on each of them.

This is not only highly inefficient and wasting a lot of CPU performance but it is also very difficult to debug (like Seaside in general) and to step through. Probably another intention to keep Seaside as a cryptic black box!

My concrete proposal, which is by no means perfect but involves minimum changes to the current situation: Introduce a static in class WAFileHandler, which answers a correlation (library name and method name) for every incoming file request.

A dictionary look up is extremely fast in Smalltalk (at least in VisualWorks) and certainly multiple times faster than the current implementation. At the same time one could also solve the problem that I am discussing in my next proposal.

That has another good side-effect: It would be much easier than today to switch between different libraries - even at run-time when a user wants to select his user-specific skin.

Proposal 6: Seaside stop lying to us!

Seaside seems designed and coded intentionally in a very cryptic style presumably with a goal to make it difficult for new users to understand it and to become productive without consultancy of specialists!

Here is another argument:

There are plenty of methods with extremely misleading names and this is, apart from the totally missing documentation, a main reason why it will be very difficult, labourious and time-consuming for new users to really understand and properly use Seaside!

Here is a perfect example:

In the class WARenderContinuation there is the method "newHtmlRoot:". From general Smalltalk conventions one would expect that this answers some new instance with data of the HTML root.

That is not false but not correct either, because unfortunately this method does a lot more: It starts a main generation loop through all child classes and sends them the very important message "updateRoot:"

When reading the message "newHtmlRoot" one would never expect that this factory method also immediately performs such important and vital actions, which are hidden behind a false name. In most libraries it is common practice to keep factory methods clean from such important actions.

I consider this extremely bad style and it makes me very angry that they are stealing my time with such highly individual and extremely uncommon bundling of functionalities behind false and lying method names! If at least they had given it some correct name like "newHtmlRootAndCollectFiles:" then I would just argue about this fancy but at least honest method name.

By the way: the method name "updateRoot:" is also a bad choice in my view. What it really does is collect references to script and css files etc, which are added to the new WAHtmlRoot instance that is created in newHtmlRoot:.

The semantics of "update" imply that some content is changed with some newer content (more "up-to-date"). This is not the case here. What happens is that some data is collected and added for the first time to that new instance. That is clearly not an update!

In my view, something like "collectFilesForRoot" or "initializeRootWithFiles" would be much clearer and much simpler to understand.

I would not discuss this if there were not many dozens of other wrong names. Nobody always and immediately finds the right words. That is why I regularly change ("update") method names when I see that they were not well selected or because their purpose and actions have changed over time.

I don't know why the authors of Seaside have made such a bullshit (remember: I promised open words and no false politeness)! I can only think of three reasons:

1) Either their goal was to sell their consultancy by making it hard for new users to become productive without external consultancy!

2) Maybe these people are verbally so limited in their capabilities of expressing their thoughts that they simply couldn't do any better. This would not be unusual for highly mathematical people and it is by no means any disqualification. I have never met anybody who is equally good in mathematical and in verbal logic. I myself have often my great problems with maths, for example, but never with open words.

3) Or these people are so self-assured and regards themselves so ingenious that they just give a shit on what other people think and don't understand.

I don't know but I would bet on a combination of the first and the third reason.

In any case, I think that these many substantial problems must be resolved quickly to make Seaside easily usable. Currently, this is unfortunately not the case.

For a quick start it would help a lot if all classes and methods had clear and concise documentation inside the code.

Unfortunately, there is almost nothing!

Proposal 5: Too lazy for lazy initialisation?

This is rather a general Smalltalk subject than solely related to Seaside but it is still an important subject that strikes us regularly.

The general Smalltalk concept assumes that a class is filed into an image and that during this file-in process various initialisations are performed, which henceforth stay valid during the entire life cycle of a class inside an image.

I am absolutely sure from our own experience that this is a very bad concept, which caused us a great number of wasted working days for fixing the results of this unrealistic assumption.

I don't want to waste your and my time on discussing why this initialisation at file-in time often doesn't work. The plain fact is that this is not sufficient and there are very many good reasons for it.

Therefore, I state the requirement that in general all variables but especially all statics and class variables must perform lazy initialisation, which is the only way to safely ensure that their values are correct when they are used for the first time.

Of course, this requires using getter methods as I already demanded in my porposal 4.

Expecting a programmer to think of manually initializing classes, which is often found in how initialize methods are implemented, is completely unrealistic. This must be automated and securely insured that all unneeded variables are in the expected state when a class is used.

All of our own classes work exclusively with lazy initialisation and I can't remember a single case where this simple and absolutely secure procedure did not fit.

For all imported libraries we have gone through the hard way of writing special initialisation methods that ensure that all statics and class variables are properly initialised when an image is started (typically the application). Additionally, we had to change many situations where instance variables were not properly initialized, which was typically due to implementation mistakes (we all make mistakes and this is why lazy initialisation is the by far securest form).

I have never come about any good arguments against lazy initialisation and from my really comprehensive experience (even beyond Smalltalk) I know for sure that there is no alternative.

Therefore I am asking not only the authors of the Seaside library but all Smalltalk class library developers in general to please adhere to lazy initialisation especially for statics and class variables.

You will save all of us who are using your libraries an awful lot of time!

Proposal 4: Documention - the Holy Grail

This is the absolute topmost and primary subject!

I have mentioned this before and I'm here giving some more details on what a good documentation should comprise of. Here I am only discussing the internal documentation, leaving general overviews and tutorials etc. completely aside.

1) Class comments
This is the most rudimentary documentation, which gives programmers an overview on the purpose and the responsibility of a particular class. This together with a clear and concise class name is absolutely mandatory in order to understand what a class does. It is not acceptable to force a user (programmer) to read the code in order to acquire the knowledge of what a class does. This is currently the case in Seaside.

For several reasons it has proven to be very practical to put the class comments into a separate method, which should be in a separate category. In my view this is far better than placing the comment into the class comment view (which is the default in VisualWorks).

There are many advantages in putting the comment into separate methods, which contain nothing but the comment and which have the same method name through old all class.

We are using the following convention, which applies to absolutely every (of our and most of the imported) classes:

In category "documentation" on the instance side there is a method called "documenationOnClass", which holds the documentation for this class.

In many cases there are additional methods in the same category, which then start with something like "documentationOnBlahBlah" where "BlahBah" characterises a particular subject, which is worth having its own separate documentation method.

The class comments should be short, concise but complete. This is difficult to achieve, I know!

Another advantage of these class documenting methods is that they can be exported easily and made available to new members of the team or used for a separate plain text documentation. However, in such cases it is always wise to keep the original texts inside the code, because life experience shows that external documentation is typically not updated when changes are made to a system.

2) Documenting instance variables
The standard practice in VisualWorks adds the instance variables to the class comment view.

I don't think that this is a good idea, especially since many instance variables are not handled as instance variables in the technical sense but are rather stored in some dictionary. In fact, we primarily use dictionaries for user application data, because they are far more flexible and easier to configure. There is hardly any speed difference in VW. Dictionaries are extremely fast.

We are documenting all instance variables in their getter methods.

This has been a good practice for my teams for far more than 20 years (I was in OOD technology for many years before my Smalltalk times). As I wrote before, we are typically not using instance variables directly in the code but we almost always access them via get and set methods (with some very few and well justified exceptions like for Semaphores or in the very rare cases where instance variables should be protected from being accessible from outside a class).

We always place the instance variable documentation into the getter methods. The setter methods typically do not contain any regular documentation except in cases where some special handling is implemented.

This has proven to be practical also because one looks at the getter methods far more often than at the setter methods.

3) Documenting methods in general
Now, let us face the facts: Many Smalltalkers believe that because their language and development environment is so brilliant and because they themselves are so brilliant as well, there is no need to document their code!

This is stupid ignorant bullshit!

Documenting Smalltalk code is just as mandatory as for code in any other language!

By the way: You can call me intolerant or whatever else you prefer but this subject is something that I am not willing to discuss, because I consider this the highest and most holy rule of software development at all! And you can take it very personal if I am stating that I cannot take anybody seriously who objects to comment his code!

Virtually every method must be documented!

The documentation text may not consist of the method name itself but it should substantially enhance the semantics of the method name. Ideally, the method name explains already the most important facts but in reality this is often not the case. This is where the method documentation comes in.

The way of phrasing this method documentation should be as consistent throughout the entire system as possible. The words themselves should be short, concise but still complete and cover all important aspects.

The method documentation must under all circumstances tell what the method answers.

Ideally, this method documentation should fully explain what a method does without having to show the code itself.

4) Special documentation inside methods
There are regularly cases where methods, especially longer multi-line methods, need additional documentation of certain statements. This is especially the case where the reason for a statement is not obvious and requires knowledge of outside or additional facts or circumstances.

Such special documentation should be placed above the respective lines of code, typically with an empty row above, so that it is clearly visually separated from the rest of the code. All other rules for documentation apply accordingly.

Please remember the first commandment:

Thou shall document thy code!

Proposal 2: Remove all namespaces

Important addition on 19.04.09: James Robertson of Cincom pointed out in his blog that the speed of namespaces has improved in VW 7. I cannot comment this (but I trust him), because we are not on VW 7. Therefore, please keep in mind that my following comments apply only to older VW releases.
-------------------------------------------------------------

A technical foreword: The sequence of my postings does not reflect the importance of the subjects.

In VisualWorks accessing classes by their namespaces bindings is very slow!

Unfortunately, both Seaside as well as the Swazoo classes use namespace bindings. This is in some cases needed, because the authors of these classes refuse to use a stringent naming convention for their classes like in the WABlahBlah cases, which is good but not yet applied to all classes.

When testing our application I was wondering why making a timestamp took such a long time! I found out that the reason was solely in the use of namespace bindings throughout many classes (primarily Swazoo and ported Squeak classes).

I have removed these namespace bindings and this decreased the time by more than 70% !!!

Therefore, I recommend you to make these changes:

1) Rename all classes to follow a stringent naming convention. The first two or three characters must under all circumstances start with a homogeneous number of characters, which identify the application. WABlahBlah is good. But it must apply to all classes.

2) Remove all usages of "Namespace.ClassName". This is slow. The class name alone will be sufficient.

3) We find it cleaner to never store the expressive class name but to use symbols like in: #XYZMyClass asClass. This avoids undeclareds when a system is configured in different versions, which might not comprise of all classes (frequently the case for us). In such cases and for speed purposes a static in class String does perfectly well, which translates "asClass" to the wanted class. But, of course, this is more a matter of taste.

Two remarks:

1) This does not contradict my previous statement that execution speed (generally) does not matter. There was more involved than mentioned above and on a heavily used server this accumulates to simply a lot of wasted time.

2) A long time ago we have made an enhancement to compensate this VisualWorks performance problem so that we can send a message "asClass" to every symbol and string. It's a hack but very useful and very much quicker than going through the normal "binding value" thing. We have simply added a static to class String, which holds a dictionary where the keys are the class symbols and the values are the classes. This has improved the entire speed of our application considerably!

...more to follow soon!

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!

Four mistakes in one line of code

Here is a first and good example of the very bad coding style in Seaside. There is one line of code with four severe mistakes in it:

WARenderContinuation -> shouldRedirect: aRequest
^ aRequest isGet not or: [(aRequest fields includesKey: '_n') not]

1) There is no sense in castrating Smalltalk and forcing the developer to use a text editor for finding the occurrences of the key '_n' by placing this value into a string. Smalltalk has this great feature of showing the references to symbols! So why does one use a string for something, which is really a symbol (a key)? This is very bad style! Amateurish! Just stupid!

2) There are several places where the key '_n' is used. Therefore, the actual key should not only be a symbol but it should also be stored at one central place, in something like a config class, which acts as the holder of all such symbols. From there on, the references can be tracked easily.

3) The knowledge about where the value for this key is to be found is spread over many other classes (in this case WARenderContinuation). That is very bad, too. Only the class WARequest should know where the value for this key is implemented. Therefore, WARenderContinuation should not know anything about somebody else's instVar 'fields'.

4) The knowledge about how the value of this key is stored (in this case in a dictionary) should also not be spread over other classes. Therefore, the usage of "includesKey:" in another class is another bad mistake in this case. Solely the class WARequest should know how this key is stored in its instances.

There is a very simple solution for these bad style codings:

^ aRequest isGet not or: [
(aRequest isTypeOfRequestForRedirect) not]

All we need is one additional little method in class WARequest:

isTypeOfRequestForRedirect
^self fields includesKey: self class requestKeyForRedirect

This method hides the place and the form of storing the key inside itself and that is where it should (only) be. In this case, the key itself is taken from a central definition method on the class side, because this key is used by other methods within the class WARequest and possibly also from by classes.

Unfortunately, similar occurrences of very bad coding style are spread all over the Seaside classes.

Obviously, the authors of Seaside have not a good clue of encapsulating class behaviour and class data into the particular classes. Instead, they spread this knowledge freely all over other classes and then even in the form of strings, which cannot be tracked by the brilliant Smalltalk development environments.

This is just one example of many for the very bad coding style in Seaside. It's more "structured programming with objects" rather than object-oriented design. From my developers I would not tolerate such basic mistakes.