From 1cfb8be85b87aca8b7b98429e42935eeaee8e144 Mon Sep 17 00:00:00 2001 From: Mguy13 Date: Fri, 6 Jan 2023 16:08:26 +0100 Subject: [PATCH] minor refactor --- Problems.md | 2 +- Program.cs | 10 +- README.md | 6 +- assets/input2.json | 2 +- review.cs | 244 +++++++++--------- ....cs => ApplicationConfigurationService.cs} | 8 +- .../{CsvWriter.cs => CsvWriterService.cs} | 4 +- 7 files changed, 139 insertions(+), 137 deletions(-) rename services/{DemoApplicationConfiguration.cs => ApplicationConfigurationService.cs} (91%) rename services/{CsvWriter.cs => CsvWriterService.cs} (88%) diff --git a/Problems.md b/Problems.md index 2e4d9a2..c9d0dec 100644 --- a/Problems.md +++ b/Problems.md @@ -12,7 +12,7 @@ This is a program manages the processing of orders in the following way: when an 5. The `DeleteOrder` method enqueues a single ReviewExportLine object with a Type field value of "9" to the queue. It is not clear what the purpose of this line is, or how it is related to the process of deleting an order. -6. `GenerateRandomString` method does not check the validity of the returned path: Since it generates these characters randomly, there could be a chaater that is considered illegal to be used as in a system-path. Additionally, 7 characters (which are very bounded) might not prove to be enough to prevent collisions. +6. `GenerateRandomString` method does not check the validity of the returned path: Since it generates these characters randomly, there could be a character that is considered illegal to be used as in a system-path. Additionally, 7 characters (which are very bounded) might not prove to be enough to prevent collisions. ## How to solve them 1. Add a condition to the loop to terminate it after a certain number of iterations or after a certain amount of time has passed Alternatively, a cancellation token to the method and pass it a token that can be used to cancel the loop. Or used better design-patterns diff --git a/Program.cs b/Program.cs index ee29cc3..6e60ea8 100644 --- a/Program.cs +++ b/Program.cs @@ -1,6 +1,5 @@ -using Configuration; -using DTO; -using static Configuration.DemoApplicationConfiguration; +using DTO; +using static Configuration.ApplicationConfigurationService; namespace OrderParser { @@ -10,11 +9,12 @@ namespace OrderParser { try { - var filesInfoConfig = DemoApplicationConfiguration.InitializeFrom(args); - var csvFileWriter = new CsvFileWriter(filesInfoConfig.OutputFile); + var filesInfoConfig = Configuration.ApplicationConfigurationService.InitializeFrom(args); + var csvFileWriter = new CsvFileWriterService(filesInfoConfig.OutputFile); foreach (var jsonFile in filesInfoConfig.JsonFilesInfo) { + // Could have easily been another service, but only do that when it needs to be scaled. var orderDTO = Newtonsoft.Json.JsonConvert.DeserializeObject(File.ReadAllText(jsonFile.FullName)); var orderModel = new OrderModel(orderDTO); diff --git a/README.md b/README.md index a522f7c..77306fa 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,8 @@ ## Infrastructure disclaimer I come from a background of developing mobile applications in Dart/Flutter, with a different set of conventions (such for file naming etc). I have done some research into doing it and tried sticking to it. But in the end, the architecture is still heavily influenced by practices in mobile development. +Additionally, I haven't focussed too much on Exception handling, since I feel that the scope of this assignment, is just a simple PoC. I do have some basic safeguards for input validation, and basic exception printing. + ### Structure #### Assets @@ -19,8 +21,8 @@ There ae 2 JSON files in the `assets` directory. These can be used as input as c 2. [OrderModel](models/OrderModel.cs): Captures the usable part of the `OrderDTO` that will be written out to the CSV. #### Services -1. [DemoApplicationConfiguration](services/DemoApplicationConfiguration.cs): Stores the configuration that the program ran with. Such as the command-line args passed (and hence the mode), the actual input data, and the ouput destination. Along with the file-existnece checks -2. [CsvWriter](services/CsvWriter.cs): Writes an order to a file. +1. [ApplicationConfigurationService](services/ApplicationConfigurationService.cs): Stores the configuration that the program ran with. Such as the command-line args passed (and hence the mode), the actual input data, and the ouput destination. Along with the file-existnece checks +2. [CsvWriterService](services/CsvWriterService.cs): Writes an order to a file. 3. Deserializing the JSON was handled by a package. Could have made this into a service ideally, for better scaling up. #### Packages used diff --git a/assets/input2.json b/assets/input2.json index c2173cf..6ada497 100644 --- a/assets/input2.json +++ b/assets/input2.json @@ -12,7 +12,7 @@ "ProductId": "PID4", "Description": "pid4 description", "Amount": 2.5, - "price": 169.0 + "price": 69.0 } ] } \ No newline at end of file diff --git a/review.cs b/review.cs index dd67e81..c22c052 100644 --- a/review.cs +++ b/review.cs @@ -1,144 +1,144 @@ -using SollicitantReview.Models; -using SollicitantReview.Services; -using Microsoft.Extensions.Options; -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Text; -using System.Threading; -using System.Threading.Tasks; +// using SollicitantReview.Models; +// using SollicitantReview.Services; +// using Microsoft.Extensions.Options; +// using System; +// using System.Collections.Generic; +// using System.IO; +// using System.Linq; +// using System.Text; +// using System.Threading; +// using System.Threading.Tasks; -namespace SollicitantReview -{ - public class SollicitantReview - { - private SollicitantReviewSettings options; - private IWriter writer; - private Queue> exportLineQueue; +// namespace SollicitantReview +// { +// public class SollicitantReview +// { +// private SollicitantReviewSettings options; +// private IWriter writer; +// private Queue> exportLineQueue; - public SollicitantReview(IOptions options, IWriter writer) - { - this.options = options.Value; - this.writer = writer; - this.exportLineQueue = new Queue>(); +// public SollicitantReview(IOptions options, IWriter writer) +// { +// this.options = options.Value; +// this.writer = writer; +// this.exportLineQueue = new Queue>(); - var processExportLinesThread = new Thread(new ThreadStart(ProcessReviewExportLines)); - processExportLinesThread.Start(); - } +// var processExportLinesThread = new Thread(new ThreadStart(ProcessReviewExportLines)); +// processExportLinesThread.Start(); +// } - public void RegisterOrder(Order order) - { - DeleteOrder(order); - exportLineQueue.Enqueue(ConvertOrderToExportLines(order)); - } +// public void RegisterOrder(Order order) +// { +// DeleteOrder(order); +// exportLineQueue.Enqueue(ConvertOrderToExportLines(order)); +// } - private string FilterString(string value) - { - return value.Replace(";", ":"); - } +// private string FilterString(string value) +// { +// return value.Replace(";", ":"); +// } - private void DeleteOrder(Order order) - { - var exportLines = new List() - { - new ReviewExportLine() - { - Type = "9", - OrderNumber = FilterString(order.OrderNumber), - Amount = "1", - UserId = FilterString(order.User), - JournalPostIndication = "Y" - } - }; +// private void DeleteOrder(Order order) +// { +// var exportLines = new List() +// { +// new ReviewExportLine() +// { +// Type = "9", +// OrderNumber = FilterString(order.OrderNumber), +// Amount = "1", +// UserId = FilterString(order.User), +// JournalPostIndication = "Y" +// } +// }; - exportLineQueue.Enqueue(exportLines); - } +// exportLineQueue.Enqueue(exportLines); +// } - private List ConvertOrderToExportLines(Order order, string userId = null) - { - var exportLines = new List(); +// private List ConvertOrderToExportLines(Order order, string userId = null) +// { +// var exportLines = new List(); - foreach (var orderLine in order.OrderLines) - { - exportLines.Add(new ReviewExportLine() - { - OrderNumber = FilterString(order.OrderNumber), - Name = FilterString(orderLine.Name), - Amount = FilterString(orderLine.Amount), - ProductNumber = FilterString(orderLine.Number), - ProductDescription = FilterString(orderLine.Description), - UserId = userId == null ? FilterString(order.User) : userId, - Location = FilterString(orderLine.Location), - JournalPostIndication = "Y" - }); - } +// foreach (var orderLine in order.OrderLines) +// { +// exportLines.Add(new ReviewExportLine() +// { +// OrderNumber = FilterString(order.OrderNumber), +// Name = FilterString(orderLine.Name), +// Amount = FilterString(orderLine.Amount), +// ProductNumber = FilterString(orderLine.Number), +// ProductDescription = FilterString(orderLine.Description), +// UserId = userId == null ? FilterString(order.User) : userId, +// Location = FilterString(orderLine.Location), +// JournalPostIndication = "Y" +// }); +// } - return exportLines; - } +// return exportLines; +// } - private void ProcessReviewExportLines() - { - while(true) - { - if (exportLineQueue.Any() && IsFolderEmpty(options.ReviewFolderPath)) - { - var exportLines = exportLineQueue.Dequeue(); - var randomStringLength = 7; +// private void ProcessReviewExportLines() +// { +// while(true) +// { +// if (exportLineQueue.Any() && IsFolderEmpty(options.ReviewFolderPath)) +// { +// var exportLines = exportLineQueue.Dequeue(); +// var randomStringLength = 7; - writer.OpenFile($"{options.ReviewFolderPath}\\{GenerateRandomString(randomStringLength)}.txt"); +// writer.OpenFile($"{options.ReviewFolderPath}\\{GenerateRandomString(randomStringLength)}.txt"); - foreach (var exportLine in exportLines) - { - writer.WriteLine($"{exportLine.Type};" + - $"{exportLine.OrderNumber};" + - $"{exportLine.Name};" + - $"{exportLine.Amount};" + - $"{exportLine.ProductNumber};" + - $"{exportLine.ProductDescription};" + - $"{exportLine.UserId};" + - $"{exportLine.Location};" + - $"{exportLine.JournalPostIndication};" + - $"{exportLine.Info2}"); - } +// foreach (var exportLine in exportLines) +// { +// writer.WriteLine($"{exportLine.Type};" + +// $"{exportLine.OrderNumber};" + +// $"{exportLine.Name};" + +// $"{exportLine.Amount};" + +// $"{exportLine.ProductNumber};" + +// $"{exportLine.ProductDescription};" + +// $"{exportLine.UserId};" + +// $"{exportLine.Location};" + +// $"{exportLine.JournalPostIndication};" + +// $"{exportLine.Info2}"); +// } - writer.CloseFile(); - } +// writer.CloseFile(); +// } - Thread.Sleep(2000); - } - } +// Thread.Sleep(2000); +// } +// } - private bool IsFolderEmpty(string path) - { - DirectoryInfo dirInfo = new DirectoryInfo(path); +// private bool IsFolderEmpty(string path) +// { +// DirectoryInfo dirInfo = new DirectoryInfo(path); - if (!dirInfo.Exists) - { - throw new CouldNotFindReviewDirectory(); - } +// if (!dirInfo.Exists) +// { +// throw new CouldNotFindReviewDirectory(); +// } - return !dirInfo.GetFiles("*.txt").Any(); - } +// return !dirInfo.GetFiles("*.txt").Any(); +// } - private string GenerateRandomString(int length) - { - StringBuilder stringBuilder = new StringBuilder(); - Random random = new Random(); - var ASCIIOffset = 65; - var ASCIIRange = 25; +// private string GenerateRandomString(int length) +// { +// StringBuilder stringBuilder = new StringBuilder(); +// Random random = new Random(); +// var ASCIIOffset = 65; +// var ASCIIRange = 25; - char letter; +// char letter; - for (int i = 0; i < length; i++) - { - double randomDouble = random.NextDouble(); - int shift = Convert.ToInt32(Math.Floor(ASCIIRange * randomDouble)); - letter = Convert.ToChar(shift + ASCIIOffset); - stringBuilder.Append(letter); - } +// for (int i = 0; i < length; i++) +// { +// double randomDouble = random.NextDouble(); +// int shift = Convert.ToInt32(Math.Floor(ASCIIRange * randomDouble)); +// letter = Convert.ToChar(shift + ASCIIOffset); +// stringBuilder.Append(letter); +// } - return stringBuilder.ToString(); - } - } -} +// return stringBuilder.ToString(); +// } +// } +// } diff --git a/services/DemoApplicationConfiguration.cs b/services/ApplicationConfigurationService.cs similarity index 91% rename from services/DemoApplicationConfiguration.cs rename to services/ApplicationConfigurationService.cs index 8e2049c..abe6b6b 100644 --- a/services/DemoApplicationConfiguration.cs +++ b/services/ApplicationConfigurationService.cs @@ -1,6 +1,6 @@ namespace Configuration { - public sealed class DemoApplicationConfiguration + public sealed class ApplicationConfigurationService { private FileInfo[] jsonFiles; public FileInfo[] JsonFilesInfo @@ -22,7 +22,7 @@ namespace Configuration } } - public static DemoApplicationConfiguration InitializeFrom(string[] args) + public static ApplicationConfigurationService InitializeFrom(string[] args) { if (args.Length < 3) throw new DemoApplicationConfigurationException("Must have atleast: 1 flag, 1 path, 1 output filename"); @@ -41,14 +41,14 @@ namespace Configuration // Assert only a single 'd' was passed if ( givenFlags.Length == 1 && givenFlags[0].Equals("-d") ) { string[] filesInDirectory = Directory.GetFiles(givenFilePaths[0]); - return new DemoApplicationConfiguration { + return new ApplicationConfigurationService { OutputFile = outputFileName, JsonFilesInfo = filesInDirectory.Select(name => new FileInfo(name)).ToArray(), }; } // Assert only '-f' were passed else if (givenFlags.All((string flag) => flag.Equals("-f"))) { - return new DemoApplicationConfiguration { + return new ApplicationConfigurationService { OutputFile = outputFileName, JsonFilesInfo = givenFilePaths.Select(name => new FileInfo(name)).ToArray(), }; diff --git a/services/CsvWriter.cs b/services/CsvWriterService.cs similarity index 88% rename from services/CsvWriter.cs rename to services/CsvWriterService.cs index 4b9ca0f..2f7a8f7 100644 --- a/services/CsvWriter.cs +++ b/services/CsvWriterService.cs @@ -1,12 +1,12 @@ using System.Globalization; using CsvHelper; -class CsvFileWriter +class CsvFileWriterService { private TextWriter textWriter; private CsvWriter csvWriter; - public CsvFileWriter(FileInfo fileInfo) { + public CsvFileWriterService(FileInfo fileInfo) { textWriter = new StreamWriter(fileInfo.FullName); csvWriter = new CsvWriter(textWriter, CultureInfo.InvariantCulture);