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

Splitting words in a selected file

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

Problem

I'm at loggerheads between performing operations one-by-one within a method, performing all of them at once (in one block) within the method, or breaking them up into four different methods.

In terms of best-practice, which of the following three options seems most advantageous, and why would it be preferred over the other options?

1st Option

Call the code:

string filename = string.Empty;
DialogResult result = openFileDialog1.ShowDialog();
if (result == DialogResult.OK)
{
    filename = openFileDialog1.FileName;
}
else
{
    MessageBox.Show("No file selected - exiting");
    return;
}
SpacelessWordBreakAerator(filename);


Implementation of code:

```
private void SpacelessWordBreakAerator(string filename)
{
string soughtCombo = string.Empty;
string desiredCombo = string.Empty;
List specialWordEndings = new List();
specialWordEndings.Add("é");
specialWordEndings.Add("í");
specialWordEndings.Add("ñ");
specialWordEndings.Add("?");
specialWordEndings.Add("!");
specialWordEndings.Add(",");
specialWordEndings.Add(".");
specialWordEndings.Add(":");
specialWordEndings.Add(";");
specialWordEndings.Add("\"");
specialWordEndings.Add("»");
specialWordEndings.Add("ß");
List specialWordBeginnings = new List();
specialWordBeginnings.Add("¿");
specialWordBeginnings.Add("¡");
specialWordBeginnings.Add("\"");
specialWordBeginnings.Add("«");

// Aerate "special" combinations:
Cursor.Current = Cursors.WaitCursor;
try
{
using (DocX document = DocX.Load(filename))
{
foreach (string endChar in specialWordEndings)
{
foreach (string beginChar in specialWordBeginnings)
{
soughtCombo = string.Format("{0}{1}", endChar, beginChar);
desiredCombo = s

Solution

I'd take the last option and then extract common functionality between

  • SpacelessWordBreakUnusualCombo



  • SpacelessWordBreakNormalBeginUnusualEnd



  • SpacelessWordBreakUnusualBeginNormalEnd



and other methods into a single method that accepts parameters.

On the glance, they look too similar so it should be just one method.

If I'm not mistaken, it should look like this:

private void ReplaceTextInFile(string filename, IDictionary replacementMap)
{
    Cursor.Current = Cursors.WaitCursor;

    try
    {
        using (DocX document = DocX.Load(filename))
        {
            foreach (var replacement in replacementMap)
            {
                document.ReplaceText(replacement.Key, replacement.Value);
            }

            document.Save();
        }
    }
    finally
    {
        Cursor.Current = Cursors.Default;
    }
}


All your methods seem to be special cases of this method, with different replacementMap parameter.

Then create a method to build a Dictionary representing all replacements you'll ever need:

private static readonly List SpecialWordBeginnings = new List { "¿", "¡", "\"", "«" };
    private static readonly List SpecialWordEndings = new List { "é", "í", "ñ", "?", "!", ",", ".", ";", "\"", "»", "ß" };

    private static readonly IDictionary ReplacementMap = BuildReplacementMap();

    private static IDictionary BuildReplacementMap()
    {
        var result = new Dictionary();

        foreach (var ending in SpecialWordEndings)
        {
            foreach (var beginning in SpecialWordBeginnings)
            {
                result.Add(
                    ending + beginning,
                    ending + " " + beginning
                );
            }

            for (var uppercaseChar = 'A'; uppercaseChar <= 'Z'; uppercaseChar++)
            {
                result.Add(
                    ending + uppercaseChar,
                    ending + " " + uppercaseChar
                );
            }
        }

        for (var lowercaseChar = 'a'; lowercaseChar <= 'z'; lowercaseChar++)
        {
            foreach (var beginning in SpecialWordBeginnings)
            {
                result.Add(
                    lowercaseChar + beginning,
                    lowercaseChar + " " + beginning
                );
            }

            for (var uppercaseChar = 'A'; uppercaseChar <= 'Z'; uppercaseChar++)
            {
                result.Add(
                    string.Concat(lowercaseChar, uppercaseChar),
                    lowercaseChar + " " + uppercaseChar
                );
            }
        }

        return result;
    }


Also note that you can do for on chars directly, no need to convert from int.

Since the replacement map is always the same, it is better to compute it once and store in a static field (which I did).

Finally, pass the map to our first method:

ReplaceTextInFile(filename, ReplacementMap);

Code Snippets

private void ReplaceTextInFile(string filename, IDictionary<string, string> replacementMap)
{
    Cursor.Current = Cursors.WaitCursor;

    try
    {
        using (DocX document = DocX.Load(filename))
        {
            foreach (var replacement in replacementMap)
            {
                document.ReplaceText(replacement.Key, replacement.Value);
            }

            document.Save();
        }
    }
    finally
    {
        Cursor.Current = Cursors.Default;
    }
}
private static readonly List<string> SpecialWordBeginnings = new List<string> { "¿", "¡", "\"", "«" };
    private static readonly List<string> SpecialWordEndings = new List<string> { "é", "í", "ñ", "?", "!", ",", ".", ";", "\"", "»", "ß" };

    private static readonly IDictionary<string, string> ReplacementMap = BuildReplacementMap();

    private static IDictionary<string, string> BuildReplacementMap()
    {
        var result = new Dictionary<string, string>();

        foreach (var ending in SpecialWordEndings)
        {
            foreach (var beginning in SpecialWordBeginnings)
            {
                result.Add(
                    ending + beginning,
                    ending + " " + beginning
                );
            }

            for (var uppercaseChar = 'A'; uppercaseChar <= 'Z'; uppercaseChar++)
            {
                result.Add(
                    ending + uppercaseChar,
                    ending + " " + uppercaseChar
                );
            }
        }

        for (var lowercaseChar = 'a'; lowercaseChar <= 'z'; lowercaseChar++)
        {
            foreach (var beginning in SpecialWordBeginnings)
            {
                result.Add(
                    lowercaseChar + beginning,
                    lowercaseChar + " " + beginning
                );
            }

            for (var uppercaseChar = 'A'; uppercaseChar <= 'Z'; uppercaseChar++)
            {
                result.Add(
                    string.Concat(lowercaseChar, uppercaseChar),
                    lowercaseChar + " " + uppercaseChar
                );
            }
        }

        return result;
    }
ReplaceTextInFile(filename, ReplacementMap);

Context

StackExchange Code Review Q#38781, answer score: 7

Revisions (0)

No revisions yet.