Implicit conversions in 4.0

[kirk-marple]

First off, thanks for making the new release, it cleaned up a lot of my messy data access code.

I think I've found a few edge cases where this still doesn't work, though.

For example, this should work, but gives the error below:

            IGeoCoordinates geo = place.Geo;

Cannot implicitly convert type 'Schema.NET.Values<Schema.NET.IGeoCoordinates, Schema.NET.IGeoShape>?' to 'Schema.NET.IGeoCoordinates'

I need to keep my old code like this, for it to compile:

            IGeoCoordinates geo = place.Geo.Value.Value1.FirstOrDefault();

I've found similar problems:
IPostalAddress address = musicVenue.Address;
IMusicGroup artist = musicAlbum.ByArtist;
IMusicGroup artist = musicRecording.ByArtist;

[RehanSaeed]

This should work:

IGeoCoordinates geo = place.Geo.Value;

[kirk-marple]

That doesn't work either, unfortunately. And to be more specific, I'm using MusicVenue not Place.

            IGeoCoordinates g = musicVenue.Geo.Value;

Cannot implicitly convert type 'Schema.NET.Values<Schema.NET.IGeoCoordinates, Schema.NET.IGeoShape>' to 'Schema.NET.IGeoCoordinates'

[RehanSaeed]

You are indeed correct! You can do a collection:

List<IGeoCoordinates> coordinates = musicVenue.Geo;

I'm not sure why the implicit operator is not working. Investigating.

[RehanSaeed]

This is an overlooked side effect of switching to interfaces. We could fix this by adding more generic type parameters. I have a working sample here that turns OneOrMany<T> into OneOrMany<T, TConcrete> and Values<T1, T2> into Values<T1, T1Concrete, T2, T2Concrete>. I'm not certain if that is a good thing to do.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;

namespace ConsoleApp9
{
    class Program
    {
        static void Main(string[] args)
        {
            var place = new Place()
            {
                Geo = new GeoCoordinates()
            };
            GeoCoordinates coordinates = place.Geo;
            GeoCoordinates foo = place.Foo;
        }
    }

    public interface IPlace
    {
        Values<IGeoCoordinates, GeoCoordinates, string, string>? Geo { get; set; }
        OneOrMany<IGeoCoordinates, GeoCoordinates> Foo { get; set; }
    }

    public class Place : IPlace
    {
        public Values<IGeoCoordinates, GeoCoordinates, string, string>? Geo { get; set; }
        public OneOrMany<IGeoCoordinates, GeoCoordinates> Foo { get; set; }
    }

    public interface IGeoCoordinates { }
    public class GeoCoordinates : IGeoCoordinates { }

    public struct Values<T1, T1Concrete, T2, T2Concrete> : IReadOnlyCollection<object>, IEnumerable<object>
    {
        public Values(OneOrMany<T1, T1Concrete> value)
        {
            this.Value1 = value;
            this.Value2 = default;
            this.HasValue1 = true;
            this.HasValue2 = false;
        }
        public Values(OneOrMany<T2, T2Concrete> value)
        {
            this.Value1 = default;
            this.Value2 = value;
            this.HasValue1 = false;
            this.HasValue2 = true;
        }
        public Values(params object[] items)
            : this(items.AsEnumerable())
        {
        }
        public Values(IEnumerable<object> items)
        {
            if (items == null)
            {
                throw new ArgumentNullException(nameof(items));
            }

            var items1 = items.OfType<T1>().Concat(items.OfType<OneOrMany<T1, T1Concrete>>().SelectMany(x => x)).ToList();
            var items2 = items.OfType<T2>().Concat(items.OfType<OneOrMany<T2, T2Concrete>>().SelectMany(x => x)).ToList();

            this.HasValue1 = items1.Count > 0;
            this.HasValue2 = items2.Count > 0;

            this.Value1 = items1;
            this.Value2 = items2;
        }
        public int Count => this.Value1.Count + this.Value2.Count;
        public bool HasValue1 { get; }
        public bool HasValue2 { get; }
        public OneOrMany<T1, T1Concrete> Value1 { get; }
        public OneOrMany<T2, T2Concrete> Value2 { get; }

        public static implicit operator Values<T1, T1Concrete, T2, T2Concrete>(T1 item) => new Values<T1, T1Concrete, T2, T2Concrete>(item);
        public static implicit operator Values<T1, T1Concrete, T2, T2Concrete>(T2 item) => new Values<T1, T1Concrete, T2, T2Concrete>(item);
        public static implicit operator Values<T1, T1Concrete, T2, T2Concrete>(T1[] array) => new Values<T1, T1Concrete, T2, T2Concrete>(array);
        public static implicit operator Values<T1, T1Concrete, T2, T2Concrete>(T2[] array) => new Values<T1, T1Concrete, T2, T2Concrete>(array);
        public static implicit operator Values<T1, T1Concrete, T2, T2Concrete>(List<T1> list) => new Values<T1, T1Concrete, T2, T2Concrete>(list);
        public static implicit operator Values<T1, T1Concrete, T2, T2Concrete>(List<T2> list) => new Values<T1, T1Concrete, T2, T2Concrete>(list);
        public static implicit operator Values<T1, T1Concrete, T2, T2Concrete>(object[] array) => new Values<T1, T1Concrete, T2, T2Concrete>(array);
        public static implicit operator Values<T1, T1Concrete, T2, T2Concrete>(List<object> list) => new Values<T1, T1Concrete, T2, T2Concrete>(list);

        public static implicit operator T1Concrete(Values<T1, T1Concrete, T2, T2Concrete> values) => (T1Concrete)(object)values.Value1.FirstOrDefault();
        public static implicit operator T2Concrete(Values<T1, T1Concrete, T2, T2Concrete> values) => (T2Concrete)(object)values.Value2.FirstOrDefault();
        public static implicit operator T1[](Values<T1, T1Concrete, T2, T2Concrete> values) => values.Value1.ToArray();
        public static implicit operator List<T1>(Values<T1, T1Concrete, T2, T2Concrete> values) => values.Value1.ToList();
        public static implicit operator T2[](Values<T1, T1Concrete, T2, T2Concrete> values) => values.Value2.ToArray();
        public static implicit operator List<T2>(Values<T1, T1Concrete, T2, T2Concrete> values) => values.Value2.ToList();

        public IEnumerator<object> GetEnumerator()
        {
            if (this.HasValue1)
            {
                foreach (var item1 in this.Value1)
                {
                    yield return item1;
                }
            }

            if (this.HasValue2)
            {
                foreach (var item2 in this.Value2)
                {
                    yield return item2;
                }
            }
        }
        IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator();
    }

    public struct OneOrMany<T, TConcrete>
        : IReadOnlyCollection<T>, IEnumerable<T>
    {
        private readonly List<T> collection;
        private readonly T item;
        public OneOrMany(T item)
        {
            this.collection = null;
            this.item = item;
        }
        public OneOrMany(params T[] array)
            : this(array == null ? null : new List<T>(array))
        {
        }
        public OneOrMany(IEnumerable<T> collection)
            : this(collection == null ? null : new List<T>(collection))
        {
        }
        public OneOrMany(List<T> list)
        {
            if (list == null)
            {
                throw new ArgumentNullException(nameof(list));
            }

            if (list.Count == 1)
            {
                this.collection = null;
                this.item = list[0];
            }
            else
            {
                this.collection = list;
                this.item = default;
            }
        }
        public int Count
        {
            get
            {
                if (this.HasOne)
                {
                    return 1;
                }
                else if (this.HasMany)
                {
                    return this.collection.Count;
                }

                return 0;
            }
        }
        public bool HasOne => this.collection == null && this.item != null;
        public bool HasMany => this.collection != null;
        public static implicit operator OneOrMany<T, TConcrete>(T item) => item != null && IsStringNullOrWhiteSpace(item) ? default : new OneOrMany<T, TConcrete>(item);
        public static implicit operator OneOrMany<T, TConcrete>(T[] array) => new OneOrMany<T, TConcrete>(array?.Where(x => x != null && !IsStringNullOrWhiteSpace(x)));
        public static implicit operator OneOrMany<T, TConcrete>(List<T> list) => new OneOrMany<T, TConcrete>(list?.Where(x => x != null && !IsStringNullOrWhiteSpace(x)));

        public static implicit operator TConcrete(OneOrMany<T, TConcrete> oneOrMany) => (TConcrete)(object)oneOrMany.FirstOrDefault();
        public static implicit operator T[](OneOrMany<T, TConcrete> oneOrMany) => oneOrMany.ToArray();
        public static implicit operator List<T>(OneOrMany<T, TConcrete> oneOrMany) => oneOrMany.ToList();

        public IEnumerator<T> GetEnumerator()
        {
            if (this.HasMany)
            {
                foreach (var item in this.collection)
                {
                    yield return item;
                }
            }
            else if (this.HasOne)
            {
                yield return this.item;
            }
        }
        IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator();
        private static bool IsStringNullOrWhiteSpace(T item) => item.GetType() == typeof(string) && string.IsNullOrWhiteSpace(item as string);
    }
}

[kirk-marple]

Ah very interesting. Feels like a lot of churn for an edge case benefit, though.

At least knowing I can get a list easily, then I just need to add a FirstOrDefault. It only affects about a half dozen places in my code.

[lorenpaulsen]

I just ran into this as well, so I wanted to give a couple more examples.

I had a CreateOffer method that returned an Offer and was used in two ways:

// single
data.Offers = CreateOffer(s);
// multiple
data.Offers = products.Select(x => CreateOffer(x)).ToList();

This change broke the second usage:

Cannot implicitly convert type 'System.Collections.Generic.List<Schema.NET.Offer>' to 'Schema.NET.OneOrMany<Schema.NET.IOffer>

Changing the return type of CreateOffer to IOffer actually fixes the second case, but breaks the first, which was not complaining before. For now, my best option instead seems to be adding a cast to the second case:

data.Offers = products.Select(x => CreateOffer(x)).Cast<IOffer>().ToList();

BTW, the root issue here is that the T in OneOrMany<T> is not covariant, for example, List<Offer> can't be assigned to a variable of type List<IOffer> as you might expect. Changing the signature to public struct OneOrMany<out T> would have fixed it, but only interface and delegate type parameters can be specified as variant (not structs or classes), so that's not possible. One solution to this would be to instead have public interface IOneOrMany<out T>.

[RehanSaeed]

I was messing with adding a OneOrMany<TInterface, TImplementation> type which would allow us to add lots of implicit conversions for TImplementation. It's on a branch here: https://github.com/RehanSaeed/Schema.NET/tree/implicit-concrete-type-conversion

@lorenpaulsen I've had some time to really read your comment and I think there are some good ideas here. It would be great to expand on the public interface IOneOrMany<out T> idea and see how it compares to what I did above.

[Turnerj]

It seems like this problem is meant to be this way for interfaces as Eric Lippert points out on Stack Overflow.

Looking into IOneOrMany<out T> sounds good though as far as I can tell, we can't add implicit conversions for interfaces. For example (though imagine that is IOneOrMany<T>):

public static implicit operator T(IEnumerable<T> oneOrMany) => oneOrMany.FirstOrDefault();

That has a compilation error of "user-defined conversions to and from an interface are not allowed". So to the best of my knowledge, this would mean that using IOneOrMany<out T> it is limited to only helping down cast the generic but not actually give implicit conversion to a list, array or single object - kinda one step forward and two steps back.

While OneOrMany<TInterface, TImplementation> would work in the sense that we can have an implicit conversion to TImplementation, it feels a little hacky to me. If you only work with interfaces, you'd need to write something like (IMyInterfaceType)(MyConcreteType)myOneOrMany just to get the single instance as the interface type.

Here is a controversial idea which I know would be a major breaking change - what if we dropped all the interfaces and only had the concrete types?

All our types are really just basic models with no specific implementation. If someone was wanting to add their own types, they likely would be building off of the implemented types unless they felt compelled to re-implement (probably in exactly the same way) the properties from the concrete types we already have.

The only concern I see would be if someone extends our concrete types and doesn't re-override the equality checks we do (as our types implement IEquatable<T>).

If it was going to be a problem with mocking in tests or something, we could mark the properties as virtual. This will apparently have a small overhead on performance as it but would allow Moq to work.

[RehanSaeed]

@Turnerj Interfaces were initially introduced to resolve #13 via #14. Ideally, we could find a way to support both features but I'm not sure that's possible. I messed about with some code in a branch here https://github.com/RehanSaeed/Schema.NET/tree/implicit-concrete-type-conversion some time ago but ran out of time.

[Turnerj]

Oh - I saw the combined classes before but didn't realise that was because of multiple inheritance in schema.org. The only way I could see #13 resolved without interfaces would be using namespacing to separate different types and then adding implicit conversion from the combined types to the non-combined.

So yeah, that really goes back to what you've got in that branch by having the concrete types specified in the generic arguments.