Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WithCollectionProperty Icollection #21

Closed
xavave opened this issue Oct 14, 2019 · 9 comments · Fixed by #23
Closed

WithCollectionProperty Icollection #21

xavave opened this issue Oct 14, 2019 · 9 comments · Fixed by #23
Assignees

Comments

@xavave
Copy link

xavave commented Oct 14, 2019

I use Entity framework virtual ICollection for proxy creation
how can I use WithCollectionProperty with ICollection please ?

There are constructors for HashSet, List and Collection

If I try to add a new constructor like

public ICollectionPropertyConfiguration<TRow> WithCollectionProperty<TCollectionItem>(
        Expression<Func<TRow, ICollection<TCollectionItem>>> propertyCollection,
        string startColumn, string endColumn) where TCollectionItem : class
    {
        var collectionConfiguration = new SimpleCollectionColumnDataExtractor<TRow, ICollection<TCollectionItem>, TCollectionItem>
            (propertyCollection, startColumn, endColumn);

        this.simpleCollectionColumnSetters.Add(collectionConfiguration);

        return this;
    }

but I get an error : ICollection must be a non abstract type with a public parameterless constructor

thank you

@ipvalverde
Copy link
Owner

Hi @xavave, thanks for noticing that one.
Unfortunately, the SimpleCollectionColumnDataExtractor creates a new instance of the collection type inside of it. So in this case, it would try to create an insurance of ICollection<TCollectionItem>, that is why you see that error.

I'm not on my computer at the moment, I'll try to give a look into it tomorrow on how else this can be done. If you have any suggestions, feel free to share as well.

@xavave
Copy link
Author

xavave commented Oct 15, 2019

I've added a constructor with ObservableCollection, it should fit my requirements

@xavave
Copy link
Author

xavave commented Oct 26, 2019

Awesome ! Thank you !
I will take a look at your code to understand how you have made it

@xavave xavave closed this as completed Oct 26, 2019
@ipvalverde
Copy link
Owner

You're welcome! Just released version 2.1.0 on Nuget with this feature

@xavave
Copy link
Author

xavave commented Oct 29, 2019

Hi Israel,
Can I use an observable Collection with this new implementation ?
I get an error :
public virtual ObservableCollection<Ingredient> FormulaIngredients { get => formulaIngredients; set => Set(ref formulaIngredients, value); }
error :
'Expression of type 'System.Collections.ObjectModel.Collection1[Formula.DAL.Model.Ingredient]' cannot be used for assignment to type 'System.Collections.ObjectModel.ObservableCollection1[Formula.DAL.Model.Ingredient]''

and
.WithCollectionProperty(p => p.FormulaIngredients,
1, "F", cfg => cfg
.WithProperty(ing => ing.TempImportId, "RM ID")
.WithProperty(ing => ing.MainTypeId, "RM type", FnConvertMainTypeName)
.WithProperty(ing => ing.Weight, "RM poids", fnParseDouble)
)

@xavave xavave reopened this Oct 29, 2019
@ipvalverde
Copy link
Owner

Could you check if your formulaIngredients is null?

If it is null, since ObservableCollection inherits from Collection, the overload used in the WithCollectionProperty is the Collection one and then the code tries to create a new instance of Collection and assign it to your property...

@ipvalverde
Copy link
Owner

Nevermind, I was able to reproduce the issue, it is not related to the property being initialized or not. I should publish a fix by tomorrow.

ipvalverde added a commit that referenced this issue Nov 3, 2019
@ipvalverde
Copy link
Owner

ipvalverde commented Nov 3, 2019

Thanks for noticing that error @xavave. Unfortunately, in order to provide a proper fix, I introduced a breaking change on version 2.2.0.

TL;DR
For your case, of an initialized collection property that is not a List<T>, HashSet<T> or Collection<T>, you can use the new method: WithInitializedCollectionProperty instead of WithCollectionProperty.

Full story
If you are using the method WithCollectionProperty for a collection property that is not a List<T>, HashSet<T> or a Collection<T>, an exception will be thrown at runtime. In your case, since your property is of type ObservableCollection<T>, the overload that accepts a Collection<T> is picked and this generates an error.
What should happen in this case, is that the overload with ICollection<T>, for initialized collection properties, should be picked instead. To prevent the necessity of specifying generic types when calling the method, now the WithCollectionProperty overloads that accept ICollection<T> are obsolete, new methods were created specifically for the ICollection<T>: WithInitializedCollectionProperty

@xavave
Copy link
Author

xavave commented Nov 3, 2019

thank you, it works with WithInitializedCollectionProperty

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants