Looks OK at first glance; though I've not had a chance to try it out yet...
One minor quibble, not specifically aimed at this change: It is impossible to tell which identifiers (and source files) refer to Interfaces, and which to classes without checking the source file(s). It would presumably be a major effort to rename all the existing Interfaces, but given that you are proposing to change this one anyway, perhaps there is a chance to start fixing things? It is not always easy to find an adjective for each Interface, so perhaps one could standardise on the prefix I for Interface? [There are only about 7 Java files that start with I at present, but anyway it should be obvious whether the first letter is part of the rest of the name or just a prefix.] Perhaps I should now duck and wait for some choice adjectives to be sent in reply! -- The opinions expressed herein are my own, and are not necessarily endorsed by my employer ... -----Original Message----- From: Jeremy Arnold [mailto:[EMAIL PROTECTED] Sent: 16 March 2003 22:17 To: [EMAIL PROTECTED] Subject: Proposed JavaSamplerClient changes Hello, In my recent work with the JavaSampler I have noticed a couple of minor shortcomings in the API. I would like to fix these shortcomings, but this will require changes to the JavaSamplerClient interface, which will in turn require modifications to any tests that use the JavaSampler. I believe this will be an easy migration, and I've tried to define the changes in a way that will make it possible to make future changes without breaking existing code again. However, since this will break any existing implementations of JavaSamplerClient, I want to make sure that people have a chance to discuss/object before the changes are committed. So if you don't agree with these changes, please speak up in the next couple days. If I don't hear anything, I'll ask the committers to go ahead with the change. I think now is the best time to make these changes, because the recent fixes to JavaSampler have changed the behavior somewhat, so some tests may need to be modified anyway. Take a look at the old and new implementation of the SleepTest. I think you will see that the differences aren't extensive (it should be an easy migration), but that the new implementation is a bit simpler, mostly because of some new convenience methods. Along with the previous two changes I've submitted for the JavaSampler, this new implementation should also be more efficient. Here is a list of the changes I have made. I haven't gone into great detail about the reasons for the changes -- let me know if it's not readily apparent. 1) I've added a method to JavaSamplerClient called getDefaultParameters. This returns an Arguments instance with the names and default values of parameters. This allows the client to specify which parameters it accepts. The JavaConfigGui has been modified to get this list of parameters and populate the arguments table with this information. This means the user doesn't have to remember the names of the parameters, and they can see the default values that will be used if they don't specify new values. The GUI will continue to allow the user to specify additional parameters. And in fact, if a JavaSamplerClient returns an empty list or null for the default parameters then it will continue to work just as it does today. This makes migration very simple -- just add the method declaration and return null. But clients which actually return an argument list will be easier for people to use. 2) A HashMap of arguments (parameters) is passed in to every call of setupTest, runTest, and teardownTest. I don't like this for a couple of reasons: a) it should probably be a Map rather than a HashMap b) the HashMap currently gets recreated for every call, which I would like to avoid. Changing it to keep a single instance across all calls for a test would technically be changing the API, since currently a test could modify the contents without affecting future calls. Although I doubt that this change would actually affect any existing tests. c) passing the HashMap in to runTest might encourage a JavaSamplerClient implementation to retrieve the parameters from the HashMap in every call to runTest, while I would think it would be best to get the parameters once in setupTest and then save them off somewhere for all of the runTest iterations. However, since I'm requring minor code changes anyway, I would like to change the parameter from a HashMap to a new JavaSamplerContext class. Currently this is just a wrapper around the parameters, with some convenience methods to simplify tests. This wrapper also forces the arguments to be used read-only. This new implementation doesn't force clients to get the parameters only during setupTest, but it does encourage this through the naming and the JavaDoc. I also hope that this would help avoid further changes to JavaSamplerClient in the future -- if we decide we need to give more data to the client, this data could be added to JavaSamplerContext and existing tests would continue to work without modification. Migration to these new method signatures is quite simple -- change the parameter from a HashMap to a JavaSamplerContext, and use the getParameter( ) method to get the value of a parameter as a String (instead of using HashMap.get( )). There are additional methods for getting the parameter as an integer or long value (others can be added if needed). This can simplify the client code since it doesn't have to be responsible for doing the parsing and exception handling. 3) I've changed JavaSampler to not implement JavaSamplerClient. The setupTest, runTest, and teardownTest methods in JavaSampler have been removed. The current error-handling semantics (which these methods were used for) are now implemented through an inner class. The main reason for this change is to reduce confusion over which class/interface should be used when implementing a Java test. Documentation updates should also help with this. (I'll do the documentation updates if this patch is committed.) This shouldn't affect any existing tests, unless they previously (incorrectly) extended JavaSampler instead of implementing JavaSamplerClient. 4) I've created an AbstractJavaSamplerClient which implements JavaSamplerClient and provides default implementations of all methods besides runTest( ). This may simplify development of Java tests somewhat, but my main reason for adding this class is an attempt to protect JavaSamplerClient implementations from any future changes. As I claim in the JavaDoc for these classes, it might be necessary at some point to add methods to JavaSamplerClient, requiring code changes from all implementations. But we could add default implementations for any new methods to AbstractJavaSamplerClient. Therefore, a test which extended AbstractJavaSamplerClient instead of implementing JavaSamplerClient directly would be protected from these changes and shouldn't need to be updated. I think that's about it. As always, let me know if there are any questions. Jeremy [EMAIL PROTECTED] http://xirr.com/~jeremy_a/ _________________________________________________________ This email is confidential and intended solely for the use of the individual to whom it is addressed. Any views or opinions presented are solely those of the author and do not necessarily represent those of SchlumbergerSema. If you are not the intended recipient, be advised that you have received this email in error and that any use, dissemination, forwarding, printing, or copying of this email is strictly prohibited. If you have received this email in error please notify the SchlumbergerSema Helpdesk by telephone on +44 (0) 121 627 5600. _________________________________________________________ --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
