patterncsharpMinor
Splitting words in a selected file
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:
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
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
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:
All your methods seem to be special cases of this method, with different
Then create a method to build a
Also note that you can do
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:
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.