Community
Participate
Working Groups
There are several methods relating to a legacy persistence scheme that can be removed. In addition several methods use Vector in their signature. Vector is an implementation type and should be replaced with a List or an array of the appropriate type. Arrays are preferred since they are typed, and since they are typically transformed to a more appropriate form in the implementation. This transformation discourages the kind of bugs associated with exposing the internal state of the implementation.
Uwe, Tobias - please review in what respect such changes could break the WR implementation of our filters, and comment on the bug if necessary.
I will be attaching a large patch for review. This first patch removes filter pool naming policies, save policies, obsolete save/restore methods, and cleans up some javadoc. The patch tests OK with a previous workspace. I don't think the changes will break source compatibility for any users at this point. This gets rid of a number of commented out method bodies and many warning messages. I've not begun working on the actual method signatures in ISystemFilter and ISystemFilterPool other than those associated with save/restore. Uwe, Tobias -- please load this patch and give me some feedback on it.
Created attachment 92227 [details] patch removing filter pool save and naming policies
*** Bug 213647 has been marked as a duplicate of this bug. ***
david, i applied the patch and everything seems to work normal, filters are working as they did before, i got no additional errors or warnings after the patch was applied. so from my point of view there seems to be no influence on our product.
Good so far. That is what I expected with the first patch. I have a new one that includes the removal of Vector and List from the interfaces in org.eclipse.rse.core.filters. This is more extensive and affected several places in org.eclipse.rse.ui so I expect it will affect you. Can you please tell me if this is absorbable? I think it cleans up the internal code quite a bit.
Created attachment 92337 [details] 2nd version of the patch patch includes the changes made in 222270a and removes the usage of List and Vector from the method signatures found in org.eclipse.rse.core.filters.
I talked with tobias this morning and we're able to give you a "go" for any further refactorings. We're ok with having to adjust our code. We're generally in favor of cleaning up things - at least in this area (core.filters).
Interface changes are commited. Summary of changes to follow.
API Change Summary: RSEFilterNamingPolicy was removed ISystemFilterSavePolcies was removed SystemFilterNamingPolcy was removed ISystemFilter getFilterStringsObjectsVector() was removed getFilterStringsVector() was removed getNestedFilters() returns an array instead of a list getStrings returns an array instead of a list setFilterStrings(Vector) was removed ISystemFilterContainer createSystemFilter was changed to take an array of strings instead of a Vector getSystemFilterNames() now returns an array instead of a Vector getSystemFilterVector() was removed ISystemFilterPool getFilters returns an array instead of a Vector getNamingPolicy was removed setNamingPolicy was removed setSavePolicy was removed ISystemFilterPoolManager createSystemFilter methods were changed to take String[] rather than Vector getPools() was removed getSystemFilterPoolNames(Vector) was removed ISystemFilterPoolReferenceManager getFolder() was removed resetManagerFolder(Folder) was removed resolveReferencesAfterRestore(...) was removed save() was removed ISystemFilterStartHere createSystemFilterNamingPolicy() was removed createSystemFilterPoolReferenceManager was changed to remove the naming policy parameter SystemFilterSimple getFilterStringsVector() was removed getNestedFilters() now returns an array instead of a List getStrings() now returns array instead of a List getSystemFilterNames() now returns an array instead of a Vector getSystemFiltersVector() was removed setFilterStrings(Vector) was removed
Looks good, overall. I'm surprised that this could go away in SubSystem.java: fprm.resolveReferencesAfterRestore();
Created attachment 92413 [details] Additional patch for examples.tutorial, and tests Additional patch fixing build errors in examples.tutorial and tests plugins. It worries me that especially the build error in the "tests" plugin was not caught on original checkin. This broke the build. I'm committing the patch right away, but necessity for this extra patch shows that by this breaking API change, each and every existing subsystem is affected. I'm wondering whether it wouldn't be a better idea to at least keep the ISystemFilterPoolManager.createSystemFilter() methods with a "List" argument rather than String[] Note that when we move to Java 1.5 eventually, we still have the chance to get static type checking by moving to List<String> if we want -- without breaking binary compatibility with old implementers who keep using List (They would need to update their sources though). This is not a strong opinion for now, we can also keep using String[], but a suggestion to discuss.
Patch committed: [222270][api] adapt to new createSystemFilter API Reopening bug to discuss using List vs String[] in create...
I had made the changes to the test projects, but for some reason they were not included in the change set I committed. I guess the lesson here is to not implicitly trust change sets managed by Mylyn. I think using parameterized collections in 1.5 is OK, but I find that usually they are not necessary in interfaces. I'd rather just stick with a style that's consistent with most of the platform.
Ok we don't need to anticipate the future yet. My main point right now is that I'm wondering whether it's worth breaking virtually all subsystems in order to replace List by String[] -- and I'd personally think it's not worth it so I'd rather stay with List in this particular case.
Added back a new set of ISystemFilterPoolManager.createSystemFilter(...) interfaces that allow Lists of strings in addition to String[]. Refactored the implementation of SystemFilterPoolManager.createSystemFilter(...) methods to simplify the code.
(In reply to comment #10) > API Change Summary: > > RSEFilterNamingPolicy was removed > ISystemFilterSavePolcies was removed > SystemFilterNamingPolcy was removed > > ISystemFilter > getFilterStringsObjectsVector() was removed > getFilterStringsVector() was removed > getNestedFilters() returns an array instead of a list > getStrings returns an array instead of a list > setFilterStrings(Vector) was removed > ISystemFilterContainer > createSystemFilter was changed to take an array of strings instead of a Vector > getSystemFilterNames() now returns an array instead of a Vector > getSystemFilterVector() was removed > ISystemFilterPool > getFilters returns an array instead of a Vector > getNamingPolicy was removed > setNamingPolicy was removed > setSavePolicy was removed > ISystemFilterPoolManager > createSystemFilter methods were changed to take String[] rather than Vector createSystemFilter methods added to allow a List of Strings instead of String[] > getPools() was removed > getSystemFilterPoolNames(Vector) was removed > ISystemFilterPoolReferenceManager > getFolder() was removed > resetManagerFolder(Folder) was removed > resolveReferencesAfterRestore(...) was removed > save() was removed > ISystemFilterStartHere > createSystemFilterNamingPolicy() was removed > createSystemFilterPoolReferenceManager was changed to remove the naming policy > parameter > SystemFilterSimple > getFilterStringsVector() was removed > getNestedFilters() now returns an array instead of a List > getStrings() now returns array instead of a List > getSystemFilterNames() now returns an array instead of a Vector > getSystemFiltersVector() was removed > setFilterStrings(Vector) was removed
committed.
Ahem... it looks like this new impl cannot work because the "filterStrings" list is not passed into the delegate method but rather an emtpy String[] array is passed: public ISystemFilter createSystemFilter(..., List filterStrings, ...) { String[] filterStringsArray = new String[filterStrings.size()]; ISystemFilter result = doCreateSystemFilter(parent, aliasName, filterStringsArray, type, promptable); return result;
1) added population of the array from the list 2) the list form is now called from the new filter wizard
