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.

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);
}

}