patterncsharpMinor
Separation of concern on micro lvl
Viewed 0 times
microconcernlvlseparation
Problem
The task is to create a DataTable with 1 column + header and 7 rows with content. The content will never change
Implementation:
Also, my
A method should only do one thing, right? How would you improve this and do you think there is a line between clean coding and adding overhead with no need?
```
public class Wochenuebersicht
{
public int KW { get; set; }
public DateTime BeginDate { get; set; }
public DateTime EndDate { get; set; }
public List WochenMenuListe { get; set; }
public DataTable Gesamt { get; set; }
public string Wochenangebot { get; set; }
public Wochenuebersicht()
{
DisplayTitle ="Wochenübersicht";
WochenMenuListe = new List();
WochenMenuListe.Add(createTage());
}
private WochenMenu createTage()
{
var table = new DataTable();
table.Columns.Add(new DataColumn("Wochentage", typeof(string)));
createRowAndAdd(table, "Montag");
createRowAndAdd(table, "Dienstag");
createRowAndAdd(table, "Mittwoch");
createRowAnd
Implementation:
private DataTable createTable()
{
var table = new DataTable();
table.Columns.Add(new DataColumn("Wochentage", typeof(string)));// Wochentage=> WeekDays
createRowAndAdd(table, "Montag");
createRowAndAdd(table, "Dienstag");
createRowAndAdd(table, "Mittwoch");
createRowAndAdd(table, "Donnerstag");
createRowAndAdd(table, "Freitag");
createRowAndAdd(table, "Samstag");
createRowAndAdd(table, "Sonntag");
return table;
}
private void createRowAndAdd(DataTable table,string text)
{
DataRow newRow = table.NewRow();
newRow[0] = text;
table.Rows.Add(newRow);
}createRowAndAdd does 2 things:- create a row (and add the text)
- add the row to the Table
Also, my
createTable does more than just creating the table.A method should only do one thing, right? How would you improve this and do you think there is a line between clean coding and adding overhead with no need?
Wochenuebersicht```
public class Wochenuebersicht
{
public int KW { get; set; }
public DateTime BeginDate { get; set; }
public DateTime EndDate { get; set; }
public List WochenMenuListe { get; set; }
public DataTable Gesamt { get; set; }
public string Wochenangebot { get; set; }
public Wochenuebersicht()
{
DisplayTitle ="Wochenübersicht";
WochenMenuListe = new List();
WochenMenuListe.Add(createTage());
}
private WochenMenu createTage()
{
var table = new DataTable();
table.Columns.Add(new DataColumn("Wochentage", typeof(string)));
createRowAndAdd(table, "Montag");
createRowAndAdd(table, "Dienstag");
createRowAndAdd(table, "Mittwoch");
createRowAnd
Solution
You're correct, in general methods should try to do just one thing.
It doesn't seem difficult to alter your code to make that so.
You'll note I've made some other changes too.
An additional change I'd make? I'd make the column header and the days parameters of
It doesn't seem difficult to alter your code to make that so.
private List days = new List()
{
"Montag",
"Dienstag",
"Mittwoch",
"Donnerstag",
"Freitag",
"Samstag",
"Sonntag"
};
private const string ColumnHeader = "Wochentage";
private DataTable CreateTable()
{
var table = new DataTable();
table.Columns.Add(new DataColumn(ColumnHeader, typeof(string)));
foreach(var day in days)
{
table.Rows.Add(CreateRow(table, day));
}
return table;
}
private DataRow CreateRow(DataTable table,string text)
{
var row table.NewRow();
row[0] = text;
return row
}You'll note I've made some other changes too.
- Refactored your list of days into a, well, list. This lets us iterate over it and reduces duplicate code.
- Removed your magic string "Wochentage" into a const.
- Used
varwhen declaring local variables when the right-hand side of declaration makes the type obvious
- Used PascalCase when defining methods (as you should)
An additional change I'd make? I'd make the column header and the days parameters of
CreateTable. Then you have a more generic function that could be usable elsewhere. I know the data is apparently not going to change, ever, but in the real world you can basically never guarantee that.Code Snippets
private List<string> days = new List<string>()
{
"Montag",
"Dienstag",
"Mittwoch",
"Donnerstag",
"Freitag",
"Samstag",
"Sonntag"
};
private const string ColumnHeader = "Wochentage";
private DataTable CreateTable()
{
var table = new DataTable();
table.Columns.Add(new DataColumn(ColumnHeader, typeof(string)));
foreach(var day in days)
{
table.Rows.Add(CreateRow(table, day));
}
return table;
}
private DataRow CreateRow(DataTable table,string text)
{
var row table.NewRow();
row[0] = text;
return row
}Context
StackExchange Code Review Q#80340, answer score: 3
Revisions (0)
No revisions yet.