Mailing-List: contact dev-help@cocoon.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cocoon.apache.org
Message-ID: <4085A446.5050103@outerthought.org>
Date: Wed, 21 Apr 2004 00:29:26 +0200
From: Marc Portier <mpo@outerthought.org>
Organization: Outerthought
User-Agent: Mozilla Thunderbird 0.5 (Windows/20040207)
MIME-Version: 1.0
To: dev@cocoon.apache.org
Subject: [cforms] refactoring questions (was Re: cvs commit:
 cocoon-2.1/src/blocks/forms/java/org/apache/cocoon/forms/formmodel
 Struct.java Messages.java Repeater.java MultiValueField.java
 AbstractContainerWidget.java
 Output.java Upload.java Action.java Form.java ContainerDelegate.java
 AbstractWidget.java
 Field.java Union.java BooleanField.java Widget.java)
References: <20040420221928.11898.qmail@minotaur.apache.org>
In-Reply-To: <20040420221928.11898.qmail@minotaur.apache.org>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit


mpo@apache.org wrote:

> mpo         2004/04/20 15:19:27
> 
>   Modified:    src/blocks/forms/java/org/apache/cocoon/forms/formmodel
>                         Struct.java Messages.java Repeater.java
>                         MultiValueField.java AbstractContainerWidget.java
>                         Output.java Upload.java Action.java Form.java
>                         ContainerDelegate.java AbstractWidget.java
>                         Field.java Union.java BooleanField.java Widget.java
>   Log:
>   Another sweap in the cforms refactoring.
>   - Added javadoc here and there
>   - Realized that Repeater is not a container widget (it's rows are): 
>     we should split the aspect of containing multiple values (like multivalue) (these are 'repeating' or 'iterating'
>     from the aspect of containing child-widgets (being 'composed' seems more appropriate)
>   - Removed confusing overloaded versions of generateSAXFragment from the Abstract(Container)Widget
>     --> this introduced getXMLElementName on all subclasses, and needs additional cleanup
>           (that should remove quite some load of copy-paste)

Sorry for the massive commit, however when walking around the code it 
only looked like the proverbial tip of the iceberg.

 >   - left quite some TODO markers for next sweaps

Maybe some of you have some suggestions on some of them, feel free to 
step in and comment:

1/ should getWidget(id) be removed from Widget? It is already on 
ContainerWidget (which is the true context that makes sense IMHO)

2/ should getNamespace() exist at all, it seems to return the same thing 
as getFullQualifiedId()? Maybe a historical idea waiting to get thrown out?

3/ can getId() ever return null or "" on a widget instance? Can't we 
carefully asume programming error and allow for the accidental NPE to be 
thrown

4/ same question on getDefinition()

5/ should we rename ContainerDelegate to simply WidgetList (and the 
ContainerDefintionDelegate to WidgetDefinitionList)

6/ union seems to generate fi:field in stead of fi:union this surprised 
me a bit, is that the goal?

7/ should validation stop as soon as possible or continue to allow all 
validation errors to be set?

8/ setParent() on abstractWidget should be write-once IMHO, possibly 
yielding RTE (IllegalState?) when someone tries to reset it

9/ should not all generateSAXFragments include the 
getDefinition.generateDisplayData() by default

-marc= (the happy cleaning lady, be warned: she'll be back :-))
-- 
Marc Portier                            http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
Read my weblog at                http://blogs.cocoondev.org/mpo/
mpo@outerthought.org                              mpo@apache.org