patterncsharpMinor
Injecting the Maharajah into the Barbarous Child's introduction
Viewed 0 times
injectingtheintobarbarouschildintroductionmaharajah
Problem
I'm building a random text generator!
My idea was to borrow slightly from the
For example, the input string:
Before you stands {Person:FullName}.
Where
{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
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 theSolution
At a quick glance the
To be on the safe side for changing accidentially the values of the
None of the properties of the classes needs to be public writeable. For encapsulation you should make the setters
Instead of throwing an
In its current state a consumer of the
This would be a good place to use properties with public getters and setters.
This looks strange. A parameter of a
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.