You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by bu...@apache.org on 2003/01/08 09:59:44 UTC

DO NOT REPLY [Bug 9075] - [PATCH] Contribution of SAP R/3(r) connectivity components

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 haul@informatik.tu-darmstadt.de  2003-01-08 08:59 -------
Michael, 
I've looked at the code and have a number of questions / comments:

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

RfcTransfomer

	setup()
	uses exception to detect non-existing configuration - using a default value
would be better

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

	startElement()
	many String.equals() - would it be better to load a HashMap with element names
and use a
	switch statement?
	in finally - unconditional release of selector (could be null)

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

	characters()
	m_fillMe.setValue() does not throw an exception (at least not mock class) - why is
	there a try-catch block around?
	why does streamer not use avalon IOW why is the classloading done manually and
not based on avalon components?


Web3DataSource

	interface implements ThreadSafe - why? In general it is considered bad to
enforce an
	execution style in an interface

Web3DataSourceSelector

	why is this emtpy interface necessary?
        (My knowledge here is limited, so forgive if this is a 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?


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?

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