[csharp] Return ICollection<T> instead of List<T>

[jimschubert]

This change is inline with Microsoft's recommended guidelines for
collections.

This code is based on the discussion in #1338

As mentioned in the issue, converting method signatures to return ICollection<T> rather than List<T> should be considered a breaking change.

This change also reduces the specific type of input parameters from List<T> to ICollection<T>, which is not a breaking change because List<T> implements ICollection<T>.

[wing328]

@jimschubert thanks for the PR. Given that it's a breaking (non-backward compatible) change, I would suggest adding a CLI option (e.g. returnICollection) that defaults to false to begin with.

[jimschubert]

@wing328 I've added two options: useCollection and returnICollection. I rebased my branch and merged master.

• useCollection: Deserialize responses into and return Collection<T>
• returnICollection: For List<T> or Collection<T>, return ICollection<T>
  instead of the concrete type

[wing328]

@jimschubert thanks for adding the option. I tested with useCollection only and got the following error:

But when I tested witih both useCollection and returnICollection set to true, I didn't get the error.

[jimschubert]

@wing328 I don't know why that would happen. Commented on the line above which includes the import.

I noticed that any time I changed the resources, I had to run mvn clean package before generating the sample. I didn't get a chance to look into why.

[wing328]

Forgot to mention that the error I got is in Pet.cs (model.mustache) and I don't see System.Collections.ObjectModel hardcoded in model.mustache.

[jimschubert]

Ah. That's weird. I can update the model template tomorrow. I can also rebase.

[wing328]

Thank you 🍺

[jimschubert]

@wing328 I've added that using directive to model.mustache. Looked like my config was only testing both options. Sorry about that. Couldn't rebase all commits because of the merge from master, but everything on my branch is now up to date.

[wing328]

@jimschubert no problem bro. I'll give it another try later and merge into master accordingly.