Friday 17 April 2009

Proposal 3: Stick to Smalltalk naming conventions

The authors of Seaside violated the general Smalltalk naming conventions in a huge number of cases! Actually, they far more often violate them than adhered to them.

One of the great things about Smalltalk is that most of the developers of class libraries adhere to common coding conventions. That makes it much easier for others to reuse these class libraries. This is one of the most powerful "de facto" features of Smalltalk!

One of the most commonly used and undisputed convention says that a factory method, which creates a new instance, should start with the word "new".

This common convention is consistently ignored in Seaside!

This is very very bad style! Absolutely amateurish and far away from "engineering"!

This extremely bad habit makes it very difficult for a user (developer) of the Seaside library to understand the code. It forces the developer to look up the code, typically over several methods, to understand what is really going on.

Actually, these wrong method names really lie to us! And there are far more such cases than discussed here.

In most cases, the method names just don't say what they do! And the worst situation of this is regarding factory methods!

For example: in WATree root: anObject really creates a new instance! How the fuck shall one know this?

This is against all Smalltalk rules! I have to read to code to know what's going on.

Must better would be:

WATree -> newWithRoot: anObject

This makes clear:
a) A new instance is created.
b) The root is set.

Why the hell can't you adhere to what the great majority of Smalltalkers has been successfully doing for more than 25 years?!

Therefore, I strongly urge you to finally adhere to what is commonly regarded as Smalltalk convention! You are definitely not any cleverer or better than the forefathers of Smalltalk at PARC (and, of course, neither am I)! You are most likely some bright but rather young guys with little experience.
So forget your pride and try to learn from the elder Smaltalkers
(I mean the general conventions. They are good and make much sense!).

Please do rename all factory methods to start with "newBlahBlah" instead of just using what you had in your mind when first writing this code.

A general recommendation from my experience:

Polymorphism is a great thing - but not always! Polymorphism for methods like "new" or "initialize" hurts more than it helps, because it is almost impossible to track the senders, because there are simply far too many!

Therefore, it has proven to be a good practice to combine the general convention "new" with some information about what is created or initialized. This can either be a shortcut of the class, which receives this message, or some other essential information related to this factory method.

In such cases it is very simple to track down the senders without having to go via the class names, which is often not so precise and which is always very much slower (at least in VisualWorks and in our images, which have typically between 60 and 80 MB in the development versions.

...more to follow soon! I have a very long list to go through!

6 comments:

  1. Stephan Eggermont18 April 2009 at 03:23

    This is not a smalltalk convention. It could be a company convention, though. Please take a look at Kent Beck's Smalltalk Best Practice Paterns, pages 23 and 24. There Kent Beck presents the x:y: constructor for Point.

    ReplyDelete
  2. @Stephan

    1) Primary question should be: It is good and helpful or not? You did not discuss this.

    I bet it is!

    2) The use of "new" definitely is a convention. Especially from senders outside the receiving class. Beyond the basic library, see my posting on that to Avy Bryant.

    A good example how readers care less about the substance of my proposal and more about defending the current state (or themselves?).

    Sadly!

    The Smalltalk Blogger

    ReplyDelete
  3. Stephan Eggermont18 April 2009 at 07:23

    1 Having a consistent naming schema as part of coding conventions is basically a good thing. I am not convinced the new... convention is a good one for Seaside to use.

    2 Open source projects can only handle coding conventions which are accepted by the major contributors. Decision making in an open source project is in general not democratic but meritocratic. If you want more influence, provide more value to the community. Current conventions can be found at http://www.seaside.st/community/conventions/initialization

    2 When I take a good look in my Squeak images, the use of new... as a convention is not recognizable.

    3 Basic frameworks do not use the new... convention. (for me Seaside is a basic package, as it was the reason I switched to smalltalk)

    3 In the smalltalk literature the new... is not described as a convention.

    4 Seaside is a multi-smalltalk framework, originally coming from Squeak. It is likely to show some squeak idioms, more than VW.

    5 Seaside has marked all instance-creation methods as such.

    6 Open source projects tend to have lots of things to do, and not enough people doing them. "We are looking forward to your change set and unit tests (and documentation and new texts for the website)". They also tend to be driven by pragmatic concerns, not theoretical. How much work is involved, and what is it going to bring me (and others).

    7 There are a number of applications depending on Seaside. If you want to break the API, please provide upgrade scripts. Your perspective (with the one commercial application in VW) is not like that of others in the community. Avi has the largest commercial Seaside application on Squeak, Lukas has lots of different applications on multiple platforms, I have lots of small project prototypes.

    8 The example you give (WATree root:) is one where I'd consider this only a minor problem. Compared to the problem that there are several usable perspectives for using trees, meaning that I always have to take a look at the actual implementation, and that it is a rather smalltalk specific one (wouldn't have one like that in java, c# or Delphi).

    9 An international project can only handle issues that are in the issue tracker or on the mailing list (or come to Esug) . I've always found the mailing lists to be very responsive. Why can't I find this issue when I google for it?

    10 I have extensive experience with open source projects, and find Seaside to be the one where major contributors are most open to explain and help others.

    11 In your posts, you sound like someone with a lot of stress. Why did you decide to start this blog now? Are the problems in Seaside threatening your project? Do you need more implementation time than you expected?

    ReplyDelete
  4. @Stephan Eggermont

    Thank you for your long comment. I will come back to that later in detail.

    But please note that Squeak is not really a product for professional use but VisualWorks is.

    The Smalltalk blogger

    ReplyDelete
  5. >Squeak is not really a product for >professional use

    incorrect - many seaside driven webpages use Squeak/Pharo. For instance dabbledb.com is AFAIK using squeak.

    ReplyDelete
  6. "not really" implies that there are some exceptions. I know.

    At least, with it's "un-sexy" old-fashioned GUI it cannot be used for products - unfortunately.

    People expect something like Windows or Next/Apple.

    ReplyDelete