1
\$\begingroup\$

While setting up applications to communicate between various different trade partners and our WMS software I often have to parse very similar information from various different file formats. In this specific case i have to retrieve the tracking data from different files in csv, xml or json format. I've set up one template class for each file format to map to the respective fields. Each template derives from TrackingInformationTemplate class. Now I have come up with this code to select the specific template i need for the parsing.

using CommandLine;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;

namespace RBHelper
{
    [Verb("versanddatenimport")]
    class TrackinDataImportCommand : ICommand
    {
        [Option('i',"input", Required = true)]
        public string FileName { get; set; }

        [Option('v',"vorlage")]
        public string Template{ get; set; }

        private Dictionary<DateiEndung, Func<List<TrackingInformations>>> _convertOperations;
    
        public TrackinDataImportCommand()
        {
            _convertOperations = new Dictionary<FileExtension, Func<List<TrackingInformations>>>()
            {
                {FileExtension.csv, ()=> FromCsv()},
                {FileExtension.json, ()=> FromJson()},
                {FileExtension.xml, ()=> FromXml()}
            };
        }
    
        public void Execute()
        {
            // load TrackinData
            List<TrakingInformations> vis = ParseVis();


            // TODO: Working with loaded data
            
        
        }

        private List<TrackingInformations> ParseVis()
        {
            FileExtension ext = (FileExtension)Enum.Parse(typeof(FileExtension), Path.GetExtension(FileName).Substring(1));
            if(!_convertOperations.Keys.Contains(ext))
                throw new NotSupportedException($"{ext} not supported.");

            return _convertOperations[ext]();
        }

        private List<TrackingInformations> FromCsv()
        {
            CsvtrackingInformationTemplate template= CsvTrackingInformationTemplate.Default;
            if (!string.IsNullOrEmpty(Template)) template= TrackingInformationTemplate.Load<CsvTrackingInformationTemplate>(Template);
            return template.DeserializeFile(FileName);
        }

        private List<TrackingInformations> FromJson()
        {
            JsonTrackingInformationTemplate template = JsonTrackingInformationTemplate.Default;
            if (!string.IsNullOrEmpty(Template)) template = TrackingInformationTemplate.Load<JsonTrackingInformatioTemplate>(Template);
            return template.DeserializeFile(FileName);
        }

        private List<TrackingInformations> FromXml()
        {
            XmlTrackingInformationTemplate template = XmlTrackingInformationTemplate.Default;
            if (!string.IsNullOrEmpty(Temlate)) template = TrackingInformationTemplate.Load<XmlTrackingInformationTemplate>(Template);
            return template.DeserializeFile(FileName);
        }
    }
}

Here is the Enum I use for file extension selection:

[JsonConverter(typeof(StringEnumConverter))]
    public enum FileExtension
    {
        csv,
        xml,
        json
    }

I am considering extracting the code related to the parsing into its own class. I just don't know how I would refactor the selection of the templates (FromCsv, FromXml, FromJson). these methods are near identical and i am certain that they can be improved.


Extra: I feel this might be outside of the scope of my question. For completion sake I'll show you the VersandInformationenVorlage cass and one of the dreived classes to demonstrate how those work:

using Newtonsoft.Json;
using System.Collections.Generic;
using System.IO;

namespace RBHelper
{
    abstract class TrackingInformationTemplate
    {
        public FileExtension FileExtension { get; private set; }
        public int kUser;
    
        public string CustomInfo;
        public string CustomRoute;

        public TrackingInformationTemplate(FileExtension FileExtension) => FileExtension = FileExtension;

        public static T Load<T>(string fileName) where T : TrackingInformationTemplate=> JsonConvert.DeserializeObject<T>(File.ReadAllText(fileName));

        public void Save(string fileName) => File.WriteAllText(fileName, JsonConvert.SerializeObject(this, Formatting.Indented));

        public abstract List<TrackingInformations> DeserializeFile(string fileName);

        protected List<TrackingInformations> Deserialize(List<TrackingInformations> input)
        {
            List<TrackingInformations> output = new List<TrackingInformations>();
            foreach (TrackingInformations v in input)
                if (!output.Contains(v)) output.Add(v);

            return output;
        }

        protected void Serialize(string serialized, string fileName)
        {
            using (FileStream fs = new FileStream(fileName, FileMode.Create, FileAccess.Write))
            using (StreamWriter sw = new StreamWriter(fs))
            {
                sw.Write(serialized);
            }
        }
    }
}

And here the JsonTrackingInformationTemplate class

using Newtonsoft.Json.Linq;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;

namespace RBHelper
{
    class JsonTrackingInformationTemplate: TrackingInformationTemplate
    {
        public JsonTrackingInformationTemplate() : base( DateiEndung.json) { }

        public string RootNode;
        public string TrackingDate;
        public string Id;
        public string Info;
        public string TrackingId;
        public string Route;

        public static JsonTrackingInformationTemplate Default => new JsonTrackingInformationTemplate()
        {
            kUser = 1,
            RootNode = "[*]",
            TrackingDate = "Versanddatum",
            Id = "Id",
            Info = "Versandinfo",
            TrackingId = "TrackingId",
            Route = "Route"
        };

        public override List<TrackingInformations> DeserializeFile(string fileName)
        {
            JToken doc = JToken.Parse(File.ReadAllText(fileName));
            return doc.SelectTokens(RootNode).Select(a => DeserializeRecord(a)).ToList();
        }

        public TrackingInformations DeserializeRecord(JToken token)
        {
            string info = string.IsNullOrEmpty(CustomInfo) ? token[Info].Value<string>() : CustomInfo;
            string route = string.IsNullOrEmpty(CustomRoute) ? token[Route].Value<string>() : CustomRoute;

            return new TrackingInformations()
            {
                TrackingDate = DateTime.Parse(token[TrackingDate].Value<string>()),
                Id = token[Id].Value<string>(),
                Info = info,
                TrackingId = token[TrackingId].Value<string>(),
                Route = route
            };
        }
    }
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ it would be more helpful to the reviewers if the code is written in plain English. \$\endgroup\$
    – iSR5
    Nov 3, 2021 at 22:41
  • 1
    \$\begingroup\$ Translated for you, @iSR5 I hope I didn't miss anything \$\endgroup\$
    – JS Garcia
    Nov 4, 2021 at 8:20

1 Answer 1

1
\$\begingroup\$

For the purposes of international collaboration, for better or worse the de-facto language of software development is English, so:

  • versanddaten -> ShippingData
  • vorlage -> Template
  • DateiEndung -> FileExtension

It is a great idea to localise user-facing content to German, but code should be in English.

"Informations" should be "Information".

Your Template property suffers from an inadequate guarantee of validity - it could be null, or an empty string. Neither should be possible. If you're able, try reworking this property to be read-only, non-null and initialized on construction. Promote the use of immutable objects in your codebase, and reduce the possible number of invalid runtime states.

You can rework your three methods:

        private List<TrackingInformations> FromCsv()
        {
            CsvtrackingInformationTemplate template= CsvTrackingInformationTemplate.Default;
            if (!string.IsNullOrEmpty(Template)) template= TrackingInformationTemplate.Load<CsvTrackingInformationTemplate>(Template);
            return template.DeserializeFile(FileName);
        }

        private List<TrackingInformations> FromJson()
        {
            JsonTrackingInformationTemplate template = JsonTrackingInformationTemplate.Default;
            if (!string.IsNullOrEmpty(Template)) template = TrackingInformationTemplate.Load<JsonTrackingInformatioTemplate>(Template);
            return template.DeserializeFile(FileName);
        }

        private List<TrackingInformations> FromXml()
        {
            XmlTrackingInformationTemplate template = XmlTrackingInformationTemplate.Default;
            if (!string.IsNullOrEmpty(Temlate)) template = TrackingInformationTemplate.Load<XmlTrackingInformationTemplate>(Template);
            return template.DeserializeFile(FileName);
        }

to one method that is based on a template parameter:

private List<TrackingInformation> DeserializeFrom<TemplateT>()
where TemplateT: TrackingInformationTemplate
{
    TrackingInformationTemplate template = TrackingInformationTemplate.Load<TemplateT>(TemplateFilename);
    return template.Deserialize(FileName);
}

This is both more extensible if other template types are added, and less code repeated.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you for your response. That is exactly what i was looking for. Of course you're right with the class names and I will change them to english. This mixed laguage programming is a bad habit I'm trying to get rid of \$\endgroup\$
    – JS Garcia
    Nov 5, 2021 at 7:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.