Leszek Gawron wrote:

Daniel Fagerstrom wrote:

Leszek Gawron wrote:

Daniel Fagerstrom wrote:

The main thing left to do in the invoker is to get rid of the code for handling macros in the StartElement part of execute. My idea there is to create a class that implements StartInstruction and contains all the macro creation and execution code. So it is such classes that is stored in the definition map, if a element name is found in the definition map one need just call the execution method on it. By doing like this we get rid of all the refernces to tags from the invoker. Concerning the parser I think we allready have discussed how it can work, check the archive (marc.theaimsgroup.com seem to be down so I can't give anty detailed refernces right now).


For starters: what do you say if every Event had an execute method and we got rid of

} else if (ev instanceof EndCDATA) {
    consumer.endCDATA();
} else if (ev instanceof EndDTD) {
    consumer.endDTD();
} else if (ev instanceof EndEntity) {
    consumer.endEntity(((EndEntity) ev).getName());
} else if (ev instanceof StartCDATA) {
    consumer.startCDATA();
} else if (ev instanceof StartDTD) {
    StartDTD startDTD = (StartDTD) ev;
    consumer.startDTD(startDTD.getName(), startDTD.getPublicId(),
                      startDTD.getSystemId());
} else if (ev instanceof StartEntity) {
    consumer.startEntity(((StartEntity) ev).getName());

    // Instructions
}

?



Sure, what interface do you have in mind?

I mean just add:
public Event execute(final XMLConsumer consumer,
ExpressionContext expressionContext,
ExecutionContext executionContext,
StartElement macroCall, Event startEvent, Event endEvent)
throws SAXException {
return getNext();
}

Ok, I would prefer having just:

   public Event execute(final XMLConsumer consumer,
                        ExpressionContext expressionContext)
       throws SAXException {
       return getNext();
   }

or maybe even:
   public void execute(final XMLConsumer consumer,
                        ExpressionContext expressionContext)
       throws SAXException {
   }

To make the Event handling independent of the instruction handling as I propsed in http://marc.theaimsgroup.com/?l=xml-cocoon-dev&m=110132582421156&w=2. But that might require some further refactoring so if you don't feel like it you can go ahead with the fatter interface.

<snip/>

A cosmetic change but:
1. gives better encapsulation and Event based classes do not look that stupid anymore (some of them were totally empty just to be matched by instanceof)


2. Invoker looks much cleaner

Yes.

The only thing that might be against is that instead of if else if else if we have method invocations which might (or not?) be less efficient. '

Don't think it should affect performance in any noticable way.

/Daniel



Reply via email to