patterncsharpMinor
Small function and level of abstraction - clean code - refactor further
Viewed 0 times
levelfunctionabstractionfurthercodesmallandrefactorclean
Problem
In the book
I already cleaned the following method up a bit by extracting a few methods (
The partly refactored code example
Update
I changed to extract even the few lines - but it still looks not pretty.
```
private string Fix(ChangeLogFixOperationPreferences preferences,str
Clean Code from Robert C. Martin are given many recommendations. Two of them are - to keep functions small and
- within a function to only have one level of abstraction.
I already cleaned the following method up a bit by extracting a few methods (
RemoveFontColor(), RemoveTextHighlight()). But i am not sure if the few-liners (font-style, font-weight, etc.) should be extracted into a method as well. The advise from the author is to not mix different-levels of abstraction within a function because this could lead to other details creeping in. This would mean that even the two liners will be extracted into their own methods.The partly refactored code example
private string Fix(FixPreferences preferences,string value)
{
if (preferences.IsRemoveBold) {
value = value.Replace("font-weight:bold", "font-weight:normal");
value = value.Replace("font-weight: bold", "font-weight:normal");
}
if (preferences.IsRemoveItalics) {
value = value.Replace("font-style:italic;", "font-style:normal;");
value = value.Replace("font-style: italic;", "font-style:normal;");
}
if (preferences.IsRemoveFontColor) {
value = RemoveFontColor(value);
}
if (preferences.IsRemoveTextHighlight) {
value = RemoveTextHighlight(value);
}
if (preferences.IsRemoveStrikethrough){
value =
value.Replace("text-decoration: line-through;", "text-decoration:none;");
value =
value.Replace("text-decoration:line-through;", "text-decoration:none;");
}
if (preferences.IsRemoveUnderline)
{
value =
value.Replace("text-decoration: underline;", "text-decoration:none;");
value =
value.Replace("text-decoration:underline;", "text-decoration:none;");
}
return value;
}Update
I changed to extract even the few lines - but it still looks not pretty.
```
private string Fix(ChangeLogFixOperationPreferences preferences,str
Solution
First of all, each condition and formatting will be moved to your own class. Let's create a common interface:
Now we can use it to abstract the formatting:
In your class you'll need a list with all implementantions of FormattingRemover, iterate over it and when the value matches, you can remove its formatting.
For now on, you just need to add new implementations to provide it with AllRemovers. A good idea is inject it with some DI container, like Ninject.
interface FormattingRemover
{
bool Match(string value);
string RemoveFormat(string value);
}Now we can use it to abstract the formatting:
public class RemoveBold : FormattingRemover
{
public bool Match(string value) {
// your condition here
return true;
}
public string RemoveFormat(string value) {
return RemoveBold(value);
}
}In your class you'll need a list with all implementantions of FormattingRemover, iterate over it and when the value matches, you can remove its formatting.
private removers = AllRemovers();
private string Fix(ChangeLogFixOperationPreferences preferences,string value)
{
foreach (var remover in removers)
{
if (remover.Match(value))
{
value = remover.RemoveFormat(value);
}
}
return value;
}For now on, you just need to add new implementations to provide it with AllRemovers. A good idea is inject it with some DI container, like Ninject.
Code Snippets
interface FormattingRemover
{
bool Match(string value);
string RemoveFormat(string value);
}public class RemoveBold : FormattingRemover
{
public bool Match(string value) {
// your condition here
return true;
}
public string RemoveFormat(string value) {
return RemoveBold(value);
}
}private removers = AllRemovers();
private string Fix(ChangeLogFixOperationPreferences preferences,string value)
{
foreach (var remover in removers)
{
if (remover.Match(value))
{
value = remover.RemoveFormat(value);
}
}
return value;
}Context
StackExchange Code Review Q#29444, answer score: 5
Revisions (0)
No revisions yet.