Applying a TypeConverter after Automap

[gwintering]

I'm having a problem with using a TypeConverter on immutable classes after upgrading from 7.1.0 to the current version (12.1.2). The TypeConverter is designed to call the special factory methods used to construct the immutable classes. I want to use AutoMap to map most of the properties, but apply a TypeConverter for the properties that are a special immutable class, like #505 mentions. The problem is that AutoMap creates a faulty ReferenceMap to the immutable class, and the .TypeConverter call creates and additional MemberMap, without affecting the ReferenceMap. So even though the MemberMap succeeds, the ReferenceMap fails, causing the entire record to fail.

I figured out how to work around the issue by adding ReferenceMaps.Remove(References<ThisMap>(m => m.Problem));, which looks like it also might be viable for #1147 . But in order to get the reference I have to call References<>, which carries a "Meant for internal use only" label.

This problem is similar to #613, but in this case I want to apply a special mapping procedure instead of ignoring the property.

It seems like TypeConverter (and ConvertUsing) should override all generated mappings, so can it be updated to call ReferenceMaps.Remove to clean up anything that AutoMap has done? Otherwise, can the ReferenceMaps.Remove(References<>()) usage be legitimized, possibly with ClassMap method?

Here's an example:

        private class Outer
        {
            // Imagine more properties here, some of them references

            // Then this class needs special conversion logic 
            public Inner Inner { get; set; }
        }

        private class Inner
        {
            public int Immutable { get; }
            // We get a similar issue if the constructor is private
            // and factory methods are exposed
            public Inner(int immutable) =>
                Immutable = immutable;
        }

        public class InnerConverter : ITypeConverter
        {
            public object ConvertFromString(string text, IReaderRow row, MemberMapData memberMapData) =>
                // Obviously string => int is already supported,
                // but imagine this is more involved 
                new Inner(int.Parse(text));

            public string ConvertToString(object value, IWriterRow row, MemberMapData memberMapData)
            {
                throw new System.NotImplementedException();
            }
        }

        private class ConverterMap : ClassMap<Outer>
        {
            public ConverterMap()
            {
                AutoMap();
                // Uncommenting this fixes the test
                //ReferenceMaps.Remove(References<ConverterMap>(m => m.Inner));
                Map(m => m.Inner).TypeConverter<InnerConverter>();
            }
        }

		[TestMethod]
		public void AutoMapPlusConverter()
		{
			var queue = new Queue<string[]>();
			queue.Enqueue( new[] { "Inner" } );
			queue.Enqueue( new[] { "9" } );
			queue.Enqueue( null );
			var parserMock = new ParserMock( queue );
			var csv = new CsvReader( parserMock );
			csv.Configuration.RegisterClassMap<ConverterMap>();
			var list = csv.GetRecords<Outer>().ToList();
		}

[JoshClose]

I don't think I've seen someone do this yet. Mapping a reference isn't an intended capability. I guess I don't see a reason why it shouldn't be allowed though.

In your case, you're removing a mapping, so using References should be fine. You can also use ReferenceMaps.Find.

ReferenceMaps.Remove(ReferenceMaps.Find<Outer>(m => m.Inner));

To avoid this, would adding an [Ignore] attribute to the Inner property work for you? Currently [Ignore] only works on non-reference types, but I could add it to reference types too.

If TypeConverter or ConvertUsing overwrote all mappings, it wouldn't matter in your case because a ReferenceMap was created and you're applying it to a MemberMap. Maybe if you call Map it should remove any ReferenceMaps already assigned to it, since you're overriding it at that point. Map does have a second argument of bool useExistingMap = true, but I don't think that would apply since the other map is a ReferenceMap. Would it work in reverse then too? Meaning, if you created a MemberMap on the class, then called Map(m => m.Inner.Immutable) or References(m => m.Inner), should it remove any existing MemberMaps? I would think both ways would need to be there.

It might be a good idea to do both of these.

[gwintering]

Thanks for the quick reply!

To avoid this, would adding an [Ignore] attribute to the Inner property work for you?

Would [Ignore] only affect AutoMapping, or reading in general? The former would help, but not the latter, because I do want to read this field, but through a custom TypeConverter.

If [Ignore] would affect both AutoMapping and reading, we may want a new attribute ([NotMapped]?) that would affect only AutoMapping.

The attribute would be nice, and would match the library's general approach of letting you configure both through attributes and Map lambdas. But you'd still have to remember to apply Ignore/NotMapped/ReferencMaps.Remove every time you assign a TypeConverter to a reference type.

I think the main issue is:

Mapping a reference isn't an intended capability.

Which leads to the current situation where a TypeConverter for a value type replaces AutoMap's work but a TypeConverter for a reference type does not. This behavior is surprising if you're not familiar with the MemberMap ReferenceMap distinction because it seems like in both cases you're just "mapping".

So if it's workable, I'd prefer:

if you call Map it should remove any ReferenceMaps already assigned to it, since you're overriding it at that point.

because that seems like the most intuitive way to blend AutoMap with custom Map statements (or attributes) on reference types.

As for

if you created a MemberMap on the class, then called Map(m => m.Inner.Immutable) or References(m => m.Inner), should it remove any existing MemberMaps?

I'm having a hard time imagining how that situation would arise so I don't have a strong opinion on it.

[JoshClose]

Would [Ignore] only affect AutoMapping, or reading in general? The former would help, but not the latter, because I do want to read this field, but through a custom TypeConverter.

Just auto mapping. You could then specify the correct map. In your example, it would allow you to not have to remove the map.

I'm having a hard time imagining how that situation would arise so I don't have a strong opinion on it.

Well, I never thought this situation would arise either! We should probably have both just in case.

I'll mark this as a feature.