cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 9075] - [PATCH] Contribution of SAP R/3(r) connectivity components
Date Mon, 13 Jan 2003 23:27:59 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=9075>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=9075

[PATCH] Contribution of SAP R/3(r) connectivity components





------- Additional Comments From michael.gerzabek@gmx.net  2003-01-13 23:27 -------
> I've looked at the code and have a number of questions / comments:
Here we go :)

> 
> why do all instance variables have a "m_" prefix ? Perhaps it 
> would be more
> obvious if instead the "this." qualifier is used?
fixed.

> 
> RfcTransfomer
> 
> 	setup()
> 	uses exception to detect non-existing configuration - using 
> a default value would be better
Yes intended. I want the user to know which configuration currently is
used. This is only for visibility. Shall i change it?

> 
> 	configure()
> 	just keeps reference to configuration - parsing should be 
> done here so that it
> is (a) more obvious and (b) occurs only once
fixed.

> 
> 	startElement()
> 	many String.equals() - would it be better to load a HashMap 
> with element names
> and use a
> 	switch statement?
done, looks much better now :)

> 	in finally - unconditional release of selector (could be null)
Do you mean the Web3DataSourceSelector or the ComponentManager? I
thought the ComponentManager can handle null's.?

> 
> 	debug()-calls are not conditional - should be 
> if(getLogger().isDebugEnabled())
> getLogger().debug(...)
fixed.

> 
> 	characters()
> 	m_fillMe.setValue() does not throw an exception (at least 
> not mock class) - why is there a try-catch block around?
mock updated, i missed the ConversionException

> 	why does streamer not use avalon IOW why is the 
> classloading done manually and not based on avalon components?
Ok. Here we go. Now a standard CS is used. Therefore i implemented
a DefaultWeb3Streamer as i already have done before :( but thought
it would be to much overhead. Well now this has got a nice real
CS and i know how to use the excalibur standard.

> 
> 
> Web3DataSource
> 
> 	interface implements ThreadSafe - why? In general it is 
> considered bad to enforce an execution style in an interface
Ok. I've done it away.

> 
> Web3DataSourceSelector
> 
> 	why is this emtpy interface necessary?
>         (My knowledge here is limited, so forgive if this is a dumb one)
Ok, it isn't necessary, i'm the dumb one.

> 
> 
> Web3DataSourceSelectorImpl
> 
> 	configure()
> 	select() does lazy initialization. OK. Would be nice to put 
> that into an extra 
> 	method, though, so that it would be more obvious.
> 	How does this selector different from the already existing 
> selectors?
This is the thing with SAP and the JavaConnector. The JCO brings
a robust and ready to work pooling implementation. You can establish
multiple pools to multiple backends at the same time. The DataSource
itself stays the same implementation that does the work right now.

But to have a mechanism obvious to the user i wanted to bring the
process of selecting a specific backend into this component. Maybe
an example would help.

Say you have a webapp that uses a CRM and a R/3 backend. You will
go to define the connections in the xconf and give both backends
a characteristic name, say 'crm' and 'accounting'.
Now in the pipeline you have to decide against which backend the
RFC has to go.
Here you have to `select` the right connection(-pool).

Does this clarify the cirumstances?

> 
> 
> Web3DataSourceImpl
> 	client pool: if a new client is requested, a new instance 
> is created. With a pool
> 	this should not be necessary if there are released clients
>         Am I missing something?
Puh, you're right, did get it in mind now. It's for historical
reasons but no longer needed. Now avalon does the work all over.

Thanks for advice
Michael

---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Mime
View raw message