From procedural to OOP

Question

I recently discover by reading some blog topics on web that 90% of code lines I produced in my career was procedural and not OOP oriented (big slap in the head). So now, I try to lobotomy my brain in order to produce only OOP codes. The problem is that I have a lot of questions about OOP practice.

Here is one :

I got a factory RawLineCollectionFactory that take string lines and create from it a domain entity called RawLineCollection. My question is about argument type pass to the Create method.

Should I pass it the object (IDataSource) that allows me to retrieve the string lines like that :

public class RawLineCollectionFactory : IRawLineCollectionFactory
{
 public IRawLineCollection Create(IDataSource dataSource)
 {
 ICollection<RawLine> lineGroup = new List<RawLine>();
 if (dataSource.GetLines() != null)
 {
 foreach (string line in dataSource.GetLines())
 {
 lineGroup.Add(new RawLine(line));
 }
 }
 return new RawLineCollection(lineGroup);
 }
}

Or call getLines() of the dataSource object on higher abstraction level and pass the result as argument of Create method ?

public class RawLineCollectionFactory : IRawLineCollectionFactory
{
 public IRawLineCollection Create(IEnumerable<string> lines)
 {
 ICollection<RawLine> lineGroup = new List<RawLine>();
 if (lines != null)
 {
 foreach (string line in lines)
 {
 lineGroup.Add(new RawLine(line));
 }
 }
 return new RawLineCollection(lineGroup);
 }
}

Edit :

The context is very close to the application entry point. RawLine is an object representation of a line in a file that contain data settings for a game creation.

 public class RawLine
{
 private string value;
 public RawLine(string value)
 {
 this.value = value;
 }
 public IEnumerable<string> Split(string separator)
 {
 if (string.IsNullOrEmpty(separator))
 {
 throw new ArgumentException("The separator is required");
 }
 if (!string.IsNullOrEmpty(value))
 {
 return value
 .Replace(" ", "", StringComparison.CurrentCulture)
 .Split(separator)
 .Where(item => !string.IsNullOrEmpty(item));
 }
 return Array.Empty<string>();
 }
}

The behavior of this class is to split an actual representation of his state for the application-specific rules purpose while RawLineCollection is a collection of this entity type.

 public class RawLineCollection : IRawLineCollection
{
 private readonly IEnumerable<RawLine> lines;
 private readonly string separator = "-";
 public RawLineCollection(IEnumerable<RawLine> lines)
 {
 this.lines = lines;
 }
 public SplittedLineCollection Split()
 {
 ICollection<IEnumerable<string>> result = new List<IEnumerable<string>>();
 if (IsValidStringGroup(lines))
 {
 foreach (RawLine line in lines)
 {
 if (line.Split(separator).Any())
 {
 result.Add(line.Split(separator));
 }
 }
 }
 return new SplittedLineCollection(result);
 }
 private bool IsValidStringGroup(IEnumerable<RawLine> settingsToSplit)
 {
 return settingsToSplit != null && settingsToSplit.Any();
 }
}

The RawLineCollectionFactory is used just one time like DataSource.GetLines() method.

• slepic: Please show us more context. The RawLine and RawLineCollection classes as well as how the factory is used. We need to know what they are and why they exist to give you a good review.
• shanif: You said this code is for reading game settings but you didn't provide the game settings class.
• shanif: To make sure you are not reinvent the wheel, Are you familiar with .NET Application settings? docs.microsoft.com/en-us/dotnet/framework/winforms/advanced/…
• MigMax: Yes but the requirements want to extract data settings game by writing them line by line with a special format (like C - 5 - 5) in a file. I actually use appsettings to store parse settings infos.
• Roland Illig: The code still doesn't compile. SplittedLineCollection is missing, as well as an example file that your code is trying to read.

Answer 1

Thanks for your question. IMHO, converting procedural code to OO code is an excellent thing to practice. Congratulations on your epiphany!

In general, the problem of persisting your game's data is a serialization problem. I would invite you to explore whether .NET's native "settings" functionality might be able to meet your needs. And beyond that, standard serialization to JSON or XML might be worth a look.

If you insist on "rolling your own" serialization, you can certainly do so in an object-oriented way. My take on getting started is below.

If you have complex data like game level definitions, you might be serializing quite a bit of data, which would be all the more reason to use a standard format like JSON or XML.

For serializing and deserializing a relatively simple config file, you can probably skip the factory pattern. But, if the config spans multiple files, including image blobs, etc., then a "Config Factory" might be worth considering.

I would recommend building one (or more) classes to properly model the actual config properties, and serializing/deserializing that instead of processing everything as strings.

For example:

public class Config
{
 public string PlayerName {get; set;}
 public DateTime LastPlayed {get; set;}
 public int HighScore {get; set;}
} 

The below example takes a "hybrid" approach. It goes further than processing raw strings by using KeyValuePair, but stops short of a fully-typed Config class (as shown above).

Another quick tip: Instead of RawLineCollection, I'd call it RawLines, or even better, Lines.

Welcome to the wide world of OOP. In some of my other code review answers I go deeper into the principles that guide my OOP practice.

Here's the sample (this code compiles, but I didn't test it):

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
public class App_ConfigFile
{
 /* Config file example 
 player1Name:Jane
 player2Name:Joe
 difficulty:medium
 lastPlayed:04/20/2020
 lastLevel:5
 highScore:31000
 */
 public void Run()
 {
 var file = new ConfigFile(@"c:\myGame\config.txt");
 var config = file.ToConfigData();
 var player1name = config.GetByName("player1Name");
 var lastPlayed = DateTime.Parse(config.GetByName("lastPlayed"));
 var highScore = int.Parse(config.GetByName("highScore"));
 }
}
public class ConfigFile
{
 private string path;
 private List<Line> lines;
 public List<Line> Lines => lines ?? (lines = getLines());
 public bool HasContent => Lines.Count > 0;
 public ConfigFile(string path) => this.path = path;
 public ConfigData ToConfigData(char separator = ':') => new ConfigData(Lines.Select(l => l.ToKvp(separator)).ToList());
 private List<Line> getLines() => File.ReadAllLines(path).Select(l => new Line(l)).ToList();
}
public class Line
{
 public string Raw { get; private set; }
 public Line(string line) => Raw = line;
 public KeyValuePair<string, string> ToKvp(char separator)
 {
 var tokens = Raw.Split(separator);
 return new KeyValuePair<string, string>(tokens.First(), tokens.Last());
 }
}
public class ConfigData
{
 private List<KeyValuePair<string, string>> data;
 private Dictionary<string, string> _dictionary;
 private Dictionary<string, string> dictionary => _dictionary ?? (_dictionary = data.ToDictionary(d => d.Key, d => d.Value, StringComparer.OrdinalIgnoreCase));
 public ConfigData(List<KeyValuePair<string, string>> data) => this.data = data;
 public string GetByName(string key) => dictionary[key];
 public bool TryGetByName(string key, out string value) => dictionary.TryGetValue(key, out value);
}

}

• MigMax: First of all, thank you for taking the time to answer me. This is a project whose goal is to train in order to enter a company. There is absolutely no data persistence even if it's just a detail for a good architecture. For me, the input data, the data entered by the user should not be coupled with a technical notion which is the JSON format. On the other hand, the analysis parameters will actually be in an appsettings. Your approach is a great help, and it gives me a lot of orientation on the idea that the object-oriented approach can be.