Wednesday 22 April 2009

Smalltalk without inheritance

Ever heard of that? Well, it's neither the 1. April, nor am I drunk.

Parts of Seaside is Smalltalk without inheritance! That's what I just found out.

It's only a small part but still: The file library tree is searched by a very special, cryptic and really "hacked" code for method names and this code intentionally ignores inheritance.

If you want to inherit code from some super class in the Seaside file library you must implement the same accessors in every sub-class or they will be ignored.

In case you don't trust me and want to look it up yourself (2.8), have a look at these two methods:
WALibrary -> documentAt:ifAbsent:
WALibrary -> fileSelectors

The mistake is in looping through all selectors of aClass instead of just sending this famous message "perform: selector".

Also, I am quite sure that this current implementation is much slower than "perform:", which is extremely fast.

I can't see any reason for such an implementation and in my >10 years Smalltalk I have never come about such an imbecillity. Again: Seaside has no documentation in the code! If there was any sense in this fancy implementation, a warning would be the minimum one could expect. But there is nothing!

You must read and understand the very cryptic Seaside code yourself to find out what's going on - and that will cost you weeks!

Don't get me wrong: Smalltalk is the most productive and brilliant programming language but even here applies: "Give a foul a tool"!

9 comments:

  1. << Well, it's neither the 1. April, nor am I drunk. >> ... option 3: you are nuts. that must be right.

    ReplyDelete
  2. Insults instead of technical arguments to prove me wrong!

    ReplyDelete
  3. Ha, so finally a post with some reference to code. How nice! So I can actually make up my mind if your flabbergasting tone is proportional to your technical knowledge.

    Your suggestion is thus to 'just' send the perform message rather than checking if the selector represents a file of the library? Well... you might not want to do that because your library class inherits from Object eventually, making it very easy to invoke something there just by entering the right url in the browser.

    What you want to do is create a subclass of FileLibrary and write the 'fileSelectors' method such that it takes inheritance into account, as you wish. Me personally, I think it makes a lot of sense not use inheritance in the classes that merely are easy file storage modules. Indeed, inheritance and overriding does not make sense when methods are string holders.

    What I am seeing is a good factorization in methods so that I can very easily change it into what you want. Alas, it seems you don't agree...

    ReplyDelete
  4. "finally a post with some reference to code"
    Wrong: read the post about WARequest.

    My comments:

    1) Assuming that there are good technical or practical reasons to discard inheritance in this case, the absolute requirement would be to very clearly document this unusual behaviour.

    There is no hint whatsoever in the code!

    2) In a typical situation I want either the server administrator or even the end-user give a chance to decide, which library ("skin") to use. In reality that means that most JavaScript and CSS files are used for all or most of the skins and only a few will differ. From this typical scenario I cannot see where there is in any sense in discarding inheritance.

    3) I found out about the files selectors myself the hard way when reading the code. I don't think this is a good idea, because it essentially means registering files for every library. Complicated and error prone and in my view a useless waste of time.

    4) Assuming that there are other situations, that I could not yet think of, it would be very easy to build in a switch "hasInheritance", which is fed from some system or configuration class, so that every Seaside developer can decide what he wants.

    5) Discarding inheritance without any form of notice is just sick, iditioc, or if you prefer this: highly unusual and extremely misleading.

    ReplyDelete
  5. I conclude that you see that 'just sending perform' is also not the right way to do it.

    I also see that your idea of inheriting file libraries makes sense, so why not make that small modification and submit it as a patch? The way I see it, the fileSelectors method should be adapted such that it takes all selectors up to, but not including, Object into account.

    For someone who has (as you say) invested a lot of money in using Seaside (I assume it means you are making a lot of money with it as well), it should be common sense to adapt and submit patches like this. However, you shouldn't start crying and kicking like a baby either when your patch is not (or not immediately) applied too.

    In the case your patch to the framework is not accepted, Smalltalk gives you inheritance and you can very easily create your own FileLibrary subclass that overrides those methods you want to have different. This is the way I do it and it almost always works, for any given framework.

    ReplyDelete
  6. No, we could not yet make any money with Seaside as the project has been substantially delayed and is even at risk at the moment. The customer is very annoyed.

    I had offered my help several times. Read "History of my critics". I had to make the problems public to perhaps wake up some people.

    I regularly change default class behaviour but I do mind having to understand undocumented cryptic code with often misleading names, because the authors prefer to write their comments to a mailing list instead of into the code where it belongs in the first place.

    ReplyDelete
  7. Yes yes, you always seem to come back to that missing documentation. But you know what? You're absolutely right on that! It is much better to have everything well documented. But then again:
    - does pissing on people help to make them do it ?
    - did you not know about the state of the code before accepting the project? Did you just start to use Seaside without assessing it?
    In my opinion, you screwed up on both questions and now you're trying to blame someone else for it.

    Oh, and I'm still waiting for you to remove the misleading name of this blog. It has nothing to do with Smalltalk as a general subject.

    ReplyDelete
  8. Why not simply submit some documented classes for a start?

    ReplyDelete
  9. I definitely will - ASAP. Currently, I have 7/14 hrs and my only interest is saving my project from failing.

    ReplyDelete