From production to staging

Question

The story here is about a console application that replays to a test system string clauses already imported in the production system.

Looking forward to your comments.

namespace MyCompany.Department.ProjectXYZ;
{
    using System;
    using System.Collections.Generic;
    using System.IO;
    using System.Linq;
    using System.ServiceModel;
    using MyCompany.XYZ;

    /// <summary>
    /// Books message clauses to staging Diamant via web service.<para/>
    /// If no arguments are provided, the program reads from the production database log table (<see cref="ImportJobData"/>) 
    /// all clauses that were imported in the last 24 hours.<para/>
    /// If a single filename argument is provided, the program reads clauses from that file.<para/>
    /// The program writes clauses that encounter an error on booking to Diamant in a text file in the FailedClauses directory.
    /// </summary>
    public class Program
    {
        /// <summary>
        /// Client to the staging web service.
        /// </summary>
        private static ImportVSMessageClient _client;

        /// <summary>
        /// Error messages for all processed clauses.
        /// </summary>
        private static readonly List<string> _errorMessages = new List<String>();

        private static readonly IApplicationLogger _logger = ApplicationMonitoring.GetLogger(typeof(Program));

        private static readonly Lazy<ErrorService> _errorService = new Lazy<ErrorService>(() => new ErrorService());

        internal static void Main(string[] args)
        {
            WebServiceClientApplication.Initialize();

            _client = new ImportVSMessageClient();

            try
            {
                var clauses = GetClausesToImport(args);
                DoImport(clauses);
            }
            catch (Exception ex)
            {
                _errorMessages.Add(ex.Message);
            }
            finally
            {
                if (_errorMessages.Any())
                {
                    _errorService.Value.ProcessError(
                        "WebService.Client - Error",
                        string.Join(Environment.NewLine, _errorMessages));
                }

                _client.Close();
            }
        }

        #region Private methods

        private static IEnumerable<string> GetClausesToImport(string[] args)
        {
            if (!args.Any())
            {
                return GetImportedClausesOnPreviousDay();
            }

            string filename = args[0];
            return File.ReadAllLines(filename);
        }

        private static IEnumerable<string> GetImportedClausesOnPreviousDay()
        {
            IImportJobsService importJobsService = new ImportJobsService();

            return importJobsService.Load(DiamantSystem.Produktion, DateTime.Now.AddDays(-1), DateTime.Now)
                .Where(IsValid).OrderBy(ijd => ijd.Erstelldatum)
                .Select(ijd => ijd.Eingangswerte);
        }

        private static Boolean IsValid(ImportJobData ijd)
        {
            return ijd.Status == VerarbeitungsStatus.ImportErfolgreich && ijd.Satzart != Satzart.LOGOUT;
        }

        private static void DoImport(IEnumerable<string> clauses)
        {
            var clausesList = clauses.ToList();

            if (clausesList.Any())
            {
                _logger.Info("Importing {0} messages.", clausesList.Count);
                ImportClauses(clausesList);

                _logger.Info("Closing connection.");
                ImportClause(Satzart.LOGOUT);

                _logger.Info("Finished importing.");
            }
            else
            {
                _logger.Info("Nothing to import.");
            }
        }

        private static void ImportClauses(IEnumerable<string> clauses)
        {
            var failedClauses = new List<String>();

            foreach (var clause in clauses)
            {
                var success = ImportClause(clause);

                if (!success)
                {
                    failedClauses.Add(clause);
                }
            }

            if (failedClauses.Any())
            {
                WriteFailedClauses(failedClauses);
            }
        }

        // Returns true when successful.
        private static bool ImportClause(String clause)
        {
            var errorMessage = RunImportAction(() => _client.ImportMessage(clause));
            if (string.IsNullOrEmpty(errorMessage))
            {
                return true;
            }

            LogError(string.Format("Clause: {0}", clause));
            LogError(string.Format("Error: {0}", errorMessage));
            return false;
        }

        private static void LogError(string errorMsg)
        {
            _errorMessages.Add(errorMsg);
            _logger.Error(errorMsg);
        }

        // Returns empty string when successful, otherwise the error message.
        private static String RunImportAction(Action action)
        {
            try
            {
                action();
                return string.Empty;
            }
            catch (FaultException<DiamantWarningFaultContract> faultException)
            {
                return faultException.Detail.ErrorMessage.Aggregate((a, b) => a + Environment.NewLine + b);
            }
            catch (FaultException<DiamantFatalFaultContract> faultException)
            {
                return faultException.Detail.ErrorMessage.Aggregate((a, b) => a + Environment.NewLine + b);
            }
            catch (FaultException exception)
            {
                return string.Format("Code: {0}, Message: {1}", exception.Code.Name, exception.Message);
            }
            catch (Exception exception)
            {
                return exception.Message;
            }
        }

        private static void WriteFailedClauses(IEnumerable<string> failedClauses)
        {
            string filename = String.Format("FailedClauses_{0}.txt", DateTime.Now.ToString("yyyy-MM-dd-HHmmss"));
            filename = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "FailedClauses", filename);

            File.WriteAllLines(filename, failedClauses);
        }

        #endregion
    }
}

Answer 1

Few small observaions.

1. Good use of readonly

2. Might want to declare your _errorMessages as a more generic type, e.g.:

   private static readonly ICollection<string> _errorMessages = new List<String>();

3. While we're at it, it seems like it might benefit from some laziness, especially if no errors/exceptions are the norm, e.g.:

   private static readonly Lazy<ICollection<string>> _errorMessages = new Lazy<ICollection<string>>(() => new List<String>());

...

        try
        {
            var clauses = GetClausesToImport(args);
            DoImport(clauses);
        }
        catch (Exception ex)
        {
            _errorMessages.Value.Add(ex.Message);
        }
        finally
        {
            if (_errorMessages.IsValueCreated)
            {
                _errorService.Value.ProcessError(
                    "WebService.Client - Error",
                    string.Join(Environment.NewLine, _errorMessages.Value));
            }

            _client.Close();
        }

4. Even in this small case, I'd go with inversion of control:

   private static IEnumerable<string> GetImportedClausesOnPreviousDay()
   {
       IImportJobsService importJobsService = new ImportJobsService();
   
       return importJobsService.Load(DiamantSystem.Produktion, DateTime.Now.AddDays(-1), DateTime.Now)
           .Where(IsValid).OrderBy(ijd => ijd.Erstelldatum)
           .Select(ijd => ijd.Eingangswerte);
   }
   

have the caller create new ImportJobsService() and pass it as a parameter to the new method:

    private static IEnumerable<string> GetImportedClausesOnPreviousDay(IImportJobsService importJobsService)
    {
        return importJobsService.Load(DiamantSystem.Produktion, DateTime.Now.AddDays(-1), DateTime.Now)
            .Where(IsValid)
            .OrderBy(ijd => ijd.Erstelldatum)
            .Select(ijd => ijd.Eingangswerte);
    }

5. Same Lazy trick in ImportClauses method.

6. In RunImportAction, I'm not really a fan of returning error strings. Throw exceptions, that's what they're there for. If you need to wrap them in a custom one for the caller to catch, make a custom exception.