Forum OpenACS Q&A: Re: Chat package HEAD branch

Posted by Gustaf Neumann on
Dear Pablo,

With your changes to xotcl-core, you broke a couple of applications here. Furthermore you introduced a hidden dependency to xotcl-core requiring now the chat package. By your changes to the methods of the class ::xo::Chat you added database operations for the tables, which are created by the chat package. For a code maintenance point of few, these operations should be as well in the chat package. There is as well no need for defining e.g. a render2 in addition to render, if one uses the oo abtractions.

Alltogether, i don't see a strong need from your changes for altering the base class ::xo::Chat at all, since chat has already a specialized subclass named ::chat::Chat (which you are using as well).

/ \


if you need a special render, add it to
::chat::Chat (no need to name it render2; btw., why does render2 need a different interface (with non-positonal argument -chat_id, were every chat instance has the chat_id as instance variable). There is now a lot of duplicated code in render2 of ::chat::Chat and ::xo::Chat that you have added. Furthermore, render2 of ::chat::Chat never calls render2 of ::xo::Chat, so i fail to see why this is needed. Code duplications makes maintenance a nightmare and is a bad practice in general.

If you need additional actions to be happending, when a user sends a message (or enters the chat etc.), provide your additional functionality e.g. in the method add_msg of class ::chat::Chat and call from this method the method add_msg of ::xo::Chat via next at an appropriate place. If you need an additional interface in ::xo::Chat, i would prefer if you talk to me about this.

By using the already existing subclass one has a lot of freedom and can easily use the methods of the base class. If one needs for some purposes an even more specialized class, on can maybe subclass ::chat::Chat further and just implement what's different in the specialized case there. So one can be sure, not break existing code. Code duplication is kept to a minimum. There is no need to change the base class for every different requirement.

I would be glad to offer you some help for extending the base-class if this is necessary Aand if you want. Since xotcl-core lives only in head, and i want to keep the existing installations working, i'have rolled back your changes to xotcl-core in cvs head for now.