HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Injecting the Maharajah into the Barbarous Child's introduction

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
injectingtheintobarbarouschildintroductionmaharajah

Problem

I'm building a random text generator!

My idea was to borrow slightly from the string.Format method styling, have a "format string" that controlled the overall construction of the string, and include "control string" segments inside them that classes will inject text into depending on their own logic.

For example, the input string:


Before you stands {Person:FullName}.

Where {Person:FullName} expands to:


{Prefix} {Name} {Postfix}, {Rank} of {Region:FullName}

Generates:


Before you stands The Barbarous Pompey The Child, Maharajah of The The
Ephemeral Citystate of Charr.


Before you stands Bertram The Churlish, Prefect of County of Dragons.


Before you stands The Honourable Launce The Child, Pastor of The
Stormy Duchy of Key.


Before you stands The Roguish Parolles The Holy, Grand Duke of Sands
of Avarice.


Before you stands The Barbarous Dogberry The Great, Colonel of
Elvenhome.


Before you stands The Cowardly Lysander The Sinner, Archon of
Principality of Elvenhome.


Before you stands The Honourable Snug The Holy, Saopha of The Shining
Principality of Old Thatch.

The code

public class Parser
{
    private IControlStringMatcher matcher;

    public Parser(IControlStringMatcher matcher)
    {
        this.matcher = matcher;
    }

    public string Parse(string input)
    {
        var finder = new ControlStringFinder('{', ':', '}');

        var controlStrings = finder.FindAllControlStrings(input);

        string result = input;

        foreach (var controlString in controlStrings)
        {

            if (matcher.Matches(controlString))
            {
                var originalString = input.Substring(controlString.Index, controlString.Length);

                var newString = matcher.Match(controlString);

                result = result.Replace(originalString, newString);
            }
        }

        return result;
    }
}


Parser is designed to split up the input string and stitch the

Solution

At a quick glance the ControlStringFinder.FindAllControlStrings() method would benefit from the usage of the String.IndexOf(char, int) method.

public IEnumerable FindAllControlStrings(string input)
{
    for (var i = 0; i (internalString.Split(valueSeparator));

            yield return new ControlString(i, end - i + 1, values);

            i = end;
        }
    }
}


To be on the safe side for changing accidentially the values of the controlStringStarter, controlStringTerminator or valueSeparator in the ControlStringFinder class you should make them readonly.

None of the properties of the classes needs to be public writeable. For encapsulation you should make the setters private.

Instead of throwing an ArgumentException with the same message 3 times I would suggest to throw a MatchException with an overloaded constructor which sets the message to this repeated value.

In its current state a consumer of the Parser class can't know what the values for controlStringStarter, controlStringTerminator or valueSeparator should be. Also, if you want to use other chars for this purpose you would need to change the Parser class.

This would be a good place to use properties with public getters and setters.

public Person(Pronoun pronouns, Region region)


This looks strange. A parameter of a Pronoun type which is called pronouns which is indicating that it is more than one Pronoun. Better use the singular.

Code Snippets

public IEnumerable<ControlString> FindAllControlStrings(string input)
{
    for (var i = 0; i < input.Length; i++)
    {
        if (input[i] == controlStringStarter)
        {
            var end = input.IndexOf(controlStringTerminator, i + 1);

            if (end == -1)
            {
                throw new FormatException("Input string has opening control string starter with no matching control string terminator.");
            }

            var internalString = input.Substring(i + 1, end - i - 1);

            var values = new Queue<string>(internalString.Split(valueSeparator));

            yield return new ControlString(i, end - i + 1, values);

            i = end;
        }
    }
}
public Person(Pronoun pronouns, Region region)

Context

StackExchange Code Review Q#88264, answer score: 5

Revisions (0)

No revisions yet.