JoshClose / CsvHelper

Library to help reading and writing CSV files
http://joshclose.github.io/CsvHelper/
Other
4.65k stars 1.05k forks source link

`Default` attribute should use `useOnConversionFailure` #2174

Open YaroslavKormushyn opened 11 months ago

YaroslavKormushyn commented 11 months ago

Is your feature request related to a problem? Please describe. The .Default() mapping includes a useOnConversionFailure parameter, which sets the default value for a field on conversion failure. The attribute for Default does not know about this parameter. Unfortunately, the .Default() is broken, too, as I described in #2156.

Describe the solution you'd like Extend the DefaultAttribute to include the useOnConversionFailure parameter, like so:

[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Parameter, AllowMultiple = false, Inherited = true)]
    public class DefaultAttribute : Attribute, IMemberMapper, IParameterMapper
    {
        public object Default { get; private set; }
        public bool UseOnConversionFailure { get; private set; }

        public DefaultAttribute(object defaultValue, bool useOnConversionFailure = false)
        {
            Default = defaultValue;
        }

        public void ApplyTo(MemberMap memberMap)
        {
            memberMap.Data.Default = Default;
            memberMap.Data.IsDefaultSet = true;
            memberMap.Data.UseDefaultOnConversionFailure = this.UseOnConversionFailure;
        }

        public void ApplyTo(ParameterMap parameterMap)
        {
            parameterMap.Data.Default = Default;
            parameterMap.Data.IsDefaultSet = true;
            parameterMap.Data.UseDefaultOnConversionFailure = this.UseOnConversionFailure;
        }
    }

This will result in a cleaner solution for the client. Instead of:

this.Map(x => x.Id).Default((Guid?) null, useOnConversionFailure: true);

we will use:

[Default(null, UseOnConversionFailure = true)]
    public Guid? Id { get; set; }

Describe alternatives you've considered Only use the .Default() method for the fields. However, this is really tedious for declarative models, since I don't use any custom mappings aside from this one. Also, this means that the .Default() API and the attribute API will differ slightly, so there's no feature parity.

Additional context I have a generic type with a generic property, called Twin. Instead of specifying the attributes directly in the necessary model, I need to follow antipatterns (or duplicate class map code) in order to cover all possible types passed to the generic (note the typeof check):

public class TwinImportRequestMap<TTwin> : ClassMap<TwinImportRequest<TTwin>>
    where TTwin : TwinImportData
{
    public TwinImportRequestMap(IEnumerable<string> schemaKeys)
    {
        this.References<TwinImportDataMap<TTwin>>(m => m.Twin);
        this.Map(m => m.Properties).Name(schemaKeys.ToArray());
    }

    public class TwinImportDataMap<TTwinImportData> : ClassMap<TTwinImportData>
        where TTwinImportData : TwinImportData
    {
        public TwinImportDataMap()
        {
            this.AutoMap(CultureInfo.InvariantCulture);
            this.Map(x => x.Id)
                .TypeConverter<NullableGuidConverter>()
                .Default((Guid?) null, useOnConversionFailure: true);

            if (typeof(TTwin) == typeof(AssetImportData))
            {
                (this as TwinImportDataMap<AssetImportData>)
                    .Map(x => x.LocatedIn)
                    .TypeConverter<NullableGuidConverter>()
                    .Default((Guid?) null, useOnConversionFailure: true);
            }
        }
    }
}