Convert to dictionary

Question

Earlier I did post with full code about Prepare and export data to MS Word file. The user t3chb0t recommended me to separate some part of full code to this post.

Context

The task of code for review is convert my types (Contracts, ActCompletion) to Dictionary<>.

I will describe the context of this task. I have a template of MS Word. I need to insert some text to it. For doing it, I use MergeField. MergeField has name. The key of dictionary is name for it. The value of dictionary is some text. I take all MergeFields and replace to text.

The name of MergeField can change. Therefore I need to use additional dictionary. For it, the key is const name for code and the value is MergeField's name. If don't do it, the program has to recompile every time when MergeField's name change.

In order to store the dictionary with MergeFiled's names I use Settings of program.

Properties.Settings.Default.MergeFieldDictionaryAsString

This is string with value like this

{"NUM_CONTRACT":"НОМЕР_ДОГОВОРА","FULL_NAME":"ПОЛНОЕ_ФИО", and so on }

For translate such string into the dictionary I use Newtonsoft.Json like this

// parse
Dictionary<string, string> fields = JsonConvert.DeserializeObject<Dictionary<string, string>>
    (Properties.Settings.Default.MergeFieldDictionaryAsString);

// add new value
Properties.Settings.Default.MergeFieldDictionaryAsString = JsonConvert.SerializeObject(dict);

And another clarification. From my previous post:

There are two documents. Contracts is the main document. ActCompletion is derived from Contracts. It contains some already known data from Contracts and adds its own. Contracts describes a table in the database. ActCompletion takes existing Contracts and runtime receives their individual data from the user. They are not stored anywhere in the database.

That's why I need the method AddGeneralField()

Elements

• ShaperField is auxiliary static class. Contains methods for formatting data into string.
• ConverterBase the class to be inherited. It is common for Contracts and ActCompletion
• ConverterContract and ConverterActCompletion it is a shell to hide the inner implemented. Both have private class Converter : ConverterBase with override Convert(). If I do not make such a shell, will have access to public fields and ConverterBase methods from the outside.

I mean that will be like this

But now with "shell" is

Code

Contracts and ActCompletion

// Generated by provider database
// add from me:
// "int id = 0" and "this.Id = id"
public partial class Contracts
{
    public Contracts(int id = 0)
    {
        this.Id = id; 
        ListKindWorks = new HashSet<ListKindWorks>();
        ListSubjects = new HashSet<ListSubjects>();
    }

    public int Id { get; set; }
    public string Num { get; set; }
    public DateTime DateConclusion { get; set; }
    public int Worker { get; set; }
    public DateTime DateStartWork { get; set; }
    public DateTime DateEndWork { get; set; }
    public double Salary { get; set; }

    public virtual Workers WorkerNavigation { get; set; }
    public virtual ICollection<ListKindWorks> ListKindWorks { get; set; }
    public virtual ICollection<ListSubjects> ListSubjects { get; set; }
}

// Not applicable to database, set values in runtime and not stored
public class ActCompletion
{
    public Contracts Contract { get; set; }
    public double Salary { get; set; }
    public Dates Dates { get; private set; }

    public ActCompletion()
    {
        Dates = new Dates();
    }
}

public class Dates
{
    public DateTime DateConclusion { get; set; }
    public DateTime DateStart { get; set; }
    public DateTime DateEnd { get; set; }
}

ConverterContract

class ConverterContract
{
    private readonly Converter converter;

    public ConverterContract()
    {
        converter = new Converter();
    }

    public Dictionary<string, string> Convert(Contracts contract)
    {
        return converter.Convert(contract);
    }

    private class Converter : ConverterBase
    {
        public override Dictionary<string, string> Convert(object obj)
        {
            Contracts c = obj as Contracts;

            InitializeDict(c, c.Salary);
            AddKindWork(c.ListKindWorks);
            AddSubject(c.ListSubjects);
            AddDates(c.DateStartWork, c.DateEndWork);
            AddPassport(c.WorkerNavigation);
            AddBank(c.WorkerNavigation);

            return Dict;
        }

        private void AddKindWork(ICollection<ListKindWorks> list)
        {
            Dict.Add(Fields["KIND_WORK"], ShaperField.ShapeKindWork(list));
        }

        private void AddSubject(ICollection<ListSubjects> list)
        {
            Dict.Add(Fields["SUBJECT"], ShaperField.ShapeSubject(list));
        }

        private void AddDates(DateTime start, DateTime end)
        {
            Dict.Add(Fields["DATE_START_CONTRACT"], ShaperField.ShapeDate(start));
            Dict.Add(Fields["DATE_END_CONTRACT"], ShaperField.ShapeDate(end));
        }

        private void AddPassport(Workers w)
        {
            Dict.Add(Fields["ADDRESS"], ShaperField.ShapeAddress(w.Address));
            Dict.Add(Fields["PASSPORT"], ShaperField.ShapePassport(w));
            Dict.Add(Fields["PASSPORT_NUMBER"], ShaperField.ShapePassportNumber(w.PassportNumber));
        }

        private void AddBank(Workers w)
        {
            Dict.Add(Fields["BANK"], ShaperField.ShapeBank(w));
        }
    }
}

ConverterActCompletion

class ConverterActCompletion
{
    private readonly Converter converter;

    public ConverterActCompletion()
    {
        converter = new Converter();
    }

    public Dictionary<string, string> Convert(ActCompletion act)
    {
        return converter.Convert(act);
    }

    private class Converter : ConverterBase
    {
        public override Dictionary<string, string> Convert(object obj)
        {
            ActCompletion act = obj as ActCompletion;

            InitializeDict(act.Contract, act.Salary);
            AddDates(act.Dates);

            return Dict;
        }

        private void AddDates(Dates dates)
        {
            Dict.Add(Fields["DATE_FILL_ACT_COMPLETION"], ShaperField.ShapeDate(dates.DateConclusion));
            Dict.Add(Fields["DATE_START_ACT_COMPLETION"], ShaperField.ShapeDate(dates.DateStart));
            Dict.Add(Fields["DATE_END_ACT_COMPLETION"], ShaperField.ShapeDate(dates.DateEnd));
        }

    }
}

ConverterBase

using Newtonsoft.Json;

abstract class ConverterBase
{
    public Dictionary<string, string> Fields { get; private set; }
    public Dictionary<string, string> Dict { get; private set; }

    public ConverterBase()
    {
        Dict = new Dictionary<string, string>();
        Fields = JsonConvert.DeserializeObject<Dictionary<string, string>>
            (Properties.Settings.Default.MergeFieldDictionaryAsString);
    }

    public void InitializeDict(Contracts c, double salary)
    {
        Dict.Clear();
        AddGeneralField(c);
        AddSalary(salary);
    }

    public abstract Dictionary<string, string> Convert(object obj);


    private void AddGeneralField(Contracts c)
    {
        Dict.Add(Fields["NUM_CONTRACT"], ShaperField.ShapeNum(c.Num));
        Dict.Add(Fields["DATE_FILL_CONTRACT"], ShaperField.ShapeDate(c.DateConclusion));
        Dict.Add(Fields["FULL_NAME"], ShaperField.ShapeFullName(c.WorkerNavigation.FullName));
        Dict.Add(Fields["SHORT_NAME"], ShaperField.ShapeShortName(c.WorkerNavigation.FullName));
    }

    private void AddSalary(double salary)
    {
        ConvertSalary cs = new ConvertSalary();
        cs.setSalaryWithTax(salary);

        Dict.Add(Fields["SALARY_GROSS"], cs.GetSalaryWithTax());
        Dict.Add(Fields["INCOME_TAX_PROCENT"], Properties.Settings.Default.ConvertSalary_IncomeTaxProcent.ToString());
        Dict.Add(Fields["INCOME_TAX_SUM"], cs.GetIncomeTaxValue());
        Dict.Add(Fields["INSURANCE_TAX_PROCENT"], Properties.Settings.Default.ConvertSalary_InsuranceTaxProcent.ToString());
        Dict.Add(Fields["INSURANCE_TAX_SUM"], cs.GetInsuranceTaxValue());
    }
}

ShaperField

static class ShaperField
{
    public static string ShapeNum(string value)
    {
        return value;
    }

    public static string ShapeDate(DateTime value)
    {
        return value.ToLongDateString();
    }

    public static string ShapeFullName(string value)
    {
        return value;
    }

    // TODO: зачем в KindWork и Subject - ToArray()
    public static string ShapeKindWork(ICollection<ListKindWorks> list)
    {
        return string.Join(", ", list.Select(x => "«" + x.IdKindWorkNavigation.Title + "»").ToArray());
    }

    public static string ShapeSubject(ICollection<ListSubjects> list)
    {
        return string.Join(", ", list.Select(x => "«" + x.IdSubjectNavigation.Title + "»").ToArray());
    }

    public static string ShapeAddress(string value)
    {
        return value;
    }

    public static string ShapePassport(Workers w)
    {
        return string.Format("{0} выдан {1} {2} г.", w.PassportSeries, w.IssuedNavigation.Title, w.DateIssued.ToString("dd.MM.yyyy"));
    }

    public static string ShapePassportNumber(string value)
    {
        return value;
    }

    public static string ShapeBank(Workers w)
    {
        return string.Format("{0}\nв {1}", w.BankAccount, w.BankNavigation.Title);
    }

    public static string ShapeShortName(string fullName)
    {
        string[] name = fullName.Split(' ');
        if (TryFormatShortName(name))
        {
            return FormatShortName(name);
        }
        else
        {
            ShowMessage.Error(string.Format("Во время получения инициалов от полного ФИО произошла ошибка. " +
                "Возможно ФИО не содержит трех слов.\nВ случае продолжения, сокращенное имя получит значение \"{0}\"",
                fullName));

            return fullName;
        }
    }

    private static bool TryFormatShortName(string[] name)
    {
        try
        {
            string shortName = FormatShortName(name);
        }
        catch (Exception ex)
        {
            return false;
        }

        return true;
    }

    private static string FormatShortName(string[] name, string pattern = "{0}.{1}. {2}")
    {
        return string.Format(pattern, name[1][0], name[2][0], name[0]);
    }
}

Answer 1

Here're are my suggestions:

---

Remove ConverterContract and ConverterActCompletion These two classes are unnecessary wrappers to hide the inner converter. You're saying that you're doing this to hide Fields and Dict so here's the next suggestion:

Make Fields and Dict protected You don't want the outside world see them? Use proper access modifiers like protected or internal where applicable.

Make ConverterBase generic The current Convert(object obj) overload is too general and can and should be more specific because you always cast obj to a concrete type:

abstract class ConverterBase<T>
{
    public abstract Dictionary<string, string> Convert(T obj);

    // ..
}

Then implement the other two converters with concrete types and concrete names. This is one of them:

public class ActCompletionConverter : ConverterBase<ActCompletion>
{
    public override Dictionary<string, string> Convert(ActCompletion obj)
    {
        // casting not necessary as 'obj' already has the desired type
        // ..
    }

    // ..    
}

Favor pure methods Pure methods do not change any class state so they are easier to debug and to test and they can safely be used in parallel. Your current implemention doesn't allow that because Convert requries the caller to change the state, this is the Dict. In order to improve that, let InitializeDict(c, c.Salary); return the initialized dictionary instead of using a field. Then make all of the AddX methods like AddKindWork accept the dictionary as a parameter. When you do this, you'll notice that all of them can now be made static. In order to make the signature shorter and not use the Dictionary<string, string> all the time, I suggest a helper class:

internal FormattedFieldDictionary : Dictionary<string, string> 
{ 
    public Dictionary<string, string> Mappings { get; set; }
}

You can then implement all the AddX methods as extensions like:

public static FormattedFieldDictionary AddSubject(this FormattedFieldDictionary fields, ICollection<ListSubjects> list)
{
    fields.Add(fields.Mappings["SUBJECT"], ShaperField.ShapeSubject(list));
    return fields;
}

so the final implementation of Convert would become a chain of extensions:

public override Dictionary<string, string> Convert(Contracts obj)
{
    return
        InitializeFormattings(c, c.Salary)
            .AddKindWork(c.ListKindWorks)
            .AddSubject(c.ListSubjects)
            ..;
}

ShaperField is a Formatter We usually call utilities like this one Formatters so I think it'd be easier to understand your code if you named this one FieldFormatter and each of the methods FormatX like FormatSubject. This naming convention would be consistent with other framework API like string.Format etc.