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 [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, email: [EMAIL PROTECTED]
