Source generator creates Vector reference setter from schema when types=j2

Why would you need - in j2 mode - a second setter with identical signature ? And as mentioned on the mailing list, this method has a particular purpose: to allow the member to be populated by passing a Vector to it. Now, having said that, one could argue that the name itself is not really intuitive ...,

If the setWidgetsList() at previous releases took a List as argument in j2 mode I think we should change that back from Vector to List. In addition it does not make sense to have 2 methods (setWidgets() and setWidgetsList()) with the same List parameter. I therefore think we should declare the setWidgetsList() as deprecated asap and remove it on the long term.

I agree with Ralf. And something that takes a List will accept a Vector anyway, right?

Okay, will make the method name more 'flexible's as requested, but at the same time make this method 'deprecated' (if that's achievable with the code generator at all).

Looks like the damage happened between versions 6202 and 6214 of CollectionInfo(J2), when basically all 'method creation' methods moved into CollectionInfo only without taking into consideration that the implementation of createAccessMethods() used different JParameter objects (siffernetly tpyed) for creating calling the createSetCollection() methods.

After a few more tests, here's what I found.

Currently, we create the following methods for 'j1':

public void setWidgets(int index, java.lang.String vWidgets)
public void setWidgets(java.lang.String[] vWidgetsArray)
public void setWidgets(java.util.Vector vWidgetsList)
public void setWidgetsAsReference(java.util.Vector WidgetsVector)

and for 'j2':

public void setWidgets(int index, java.lang.String vWidgets)
public void setWidgets(java.lang.String[] vWidgetsArray)
public void setWidgets(java.util.List vWidgetsList)
public void setWidgetsAsReference(java.util.Vector WidgetsVector)

Whilst the first three methods get generated as expected, the last one seems to be wrong. I will make the changes for this method to behave according to the otehr ones, i.e not ignoring the field info factory used.

Just wondering whether it would be acceptable for the parameter of the setWidgetsAsReference to carry a suffix that actually represents the actual java.util.* class type, i.e. 'Vector', 'List', 'Collection', etc., and not just 'Collection' for all Java 2 collection types ?

Initial patch for review.

It would easy review if you could attach a snipped of the generated code for such issues.
I wonder why the method is called setWidgetsAsReference() instead of setWidgetList().
It seams you have not found an easy way to define deprecation for a generated method.

I think the most general solution would be to have a method (for J2) that accepts Collection and to have a method that returns Collection. That is, why not:

public void setWidgets(java.util.Collection vWidgetsList)

if we can truly handle Collection, then we don't care what kind of collection they pass in and we don't have to worry about making a plethora of differently-named methods to handle different intputs. Then we could get rid of the "as reference" (because what does that mean?) method entirely.

Is there any reason (other than possible backwards compatibility) that we need the method

public void setWidgetsAsReference(java.util.Vector WidgetsVector)

if we already have the setWidgets that takes Collection? If we need it for backwards compatiblity, then how about it just is a proxy that immediately passes control to setWidgets(java.util.Collection)?

Ralf, the method has been deliberately been renamed to reflect the fact that you are injecting a reference (as opposed to constructing a new collection) s part of quite some big refactoring a few weeks ago.

Werner, are you saying that the setXXXXXAsReference **REPLACES** whatever collection is currently held with the one being passed in? If so, I would suggest another renaming to make it more obvious that values previously in the collection will be lost.

Based upon the comments that will be generated ....

/** Sets the value of 'widgetList' by setting it to the given Vector. No type checking is performed. */

and the code generated as follows

this.widgetList = widgetList;

this is what happens.

All others either use Collection.addAll() or Collection.add() to copy elements in to the target collection (after a Collection.clear(), that is).

I now understand much better what your questions are. You are asking ONLY about the method

setWidgetsAsReference(java.util.Vector WidgetsVector)

correct? What is the value of this method to someone who is using Castor? It's more efficient than doing Collection.clear() then .addAll(), but other than this, is there value added by this method? (I imagine this is why Ralf suggests getting rid of this method altogether.)

As long as we generate this method, we definitely want the method parameter type to be the same as the type of the collection used. (So we will not need type verification or casting and there'll be no risk of class cast exception.)

Other than this, I cannot really think of a more meaningful name than setXXXXXXAsReference.

Fine by me. I'll add add the @deprecated comment, and make sure that the method is correctly typed, according to what the user specified either when calling the SourceGenerator or through the binding file (as per the recently added new collection type support).

Whilst we are actually talking about things related to collections, does anybody have a few what methods should be generated for Map-based collection types ?

Edward, let me just make sure that I understand you correctly. Above, you state ...

> As long as we generate this method, we definitely want the method parameter type to be the same as
> the type of the collection used. (So we will not need type verification or casting and there'll be no risk of class cast exception.)

My original idea was to bring this in line with the FieldInfoFactory used when creating the SourceGenerator instance ('j1', 'j2'). SO it could either be

public void setWidgetsAsReference(java.util.Vector widgetsVector), or
public void setWidgetsAsReference(java.util.List widgetsList)

Having said that, you might have noted that users are now able to override the J2 collection type used for a collection member, by e.g. using java.util.Set, java.util.SortedSet et alias. How am I meant to be reading your statement (quoted above) with regards to this question ?

If users override the J2 collection type, then our best course of action is to use THAT type in the signature. That way we expose a clear API and we avoid the risk of incompatible types or of a class cast exception.