patterncsharpMinor
Stop Watch Application 2.0
Viewed 0 times
stopwatchapplication
Problem
After the tips from my previous review. I have come up with these changes:
I'm not entirely sure I have used the namespace correctly. If it's wrong, can you please give me a push in the right direction? Any other comments would be appreciated.
```
namespace Difficult.StopWatch0002
{
public partial class StopWatchForm : Form
{
StateEnum state;
Stopwatch stopWatch;
TimeSpan timeSpan;
string elapsedTime;
List times = new List();
public StopWatchForm()
{
InitializeComponent();
state = StateEnum.Stoped;
}
private void StartAndLapButton_Click(object sender, EventArgs e)
{
if (state == StateEnum.Stoped)
{
stopWatch = Stopwatch.StartNew();
state = StateEnum.Started;
ChangeButtonText(StartAndLapButton, "Lap");
ChangeFormTitle("Started");
timer1.Enabled = true;
} else if (state == StateEnum.Started)
{
times.Add(elapsedTime);
}
}
private void StopButton_Click(object sender, EventArgs e)
{
if (state == StateEnum.Started)
{
ChangeButtonText(StartAndLapButton, "Start");
ChangeFormTitle("Stop Watch");
stopWatch.Stop();
times.Add(elapsedTime);
state = StateEnum.Stoped;
timer1.Enabled = false;
}
}
private void FileButton_Click(object sender, EventArgs e)
{
if (!times.Any())
{
return;
}
var filename = PromtSaveAsFileName();
if (filename == null)
{
return;
}
WriteResultsToFile(filename);
times.Clear();
}
private void WriteResultsToFile(string filename)
{
System.IO.File.WriteAllLines(filename, tim
- Changed my braces to start on a new line
- Used the Single Responsibility Principle
- Return early whenever possible
- Use a namespace that doesn't start with an underscore
I'm not entirely sure I have used the namespace correctly. If it's wrong, can you please give me a push in the right direction? Any other comments would be appreciated.
```
namespace Difficult.StopWatch0002
{
public partial class StopWatchForm : Form
{
StateEnum state;
Stopwatch stopWatch;
TimeSpan timeSpan;
string elapsedTime;
List times = new List();
public StopWatchForm()
{
InitializeComponent();
state = StateEnum.Stoped;
}
private void StartAndLapButton_Click(object sender, EventArgs e)
{
if (state == StateEnum.Stoped)
{
stopWatch = Stopwatch.StartNew();
state = StateEnum.Started;
ChangeButtonText(StartAndLapButton, "Lap");
ChangeFormTitle("Started");
timer1.Enabled = true;
} else if (state == StateEnum.Started)
{
times.Add(elapsedTime);
}
}
private void StopButton_Click(object sender, EventArgs e)
{
if (state == StateEnum.Started)
{
ChangeButtonText(StartAndLapButton, "Start");
ChangeFormTitle("Stop Watch");
stopWatch.Stop();
times.Add(elapsedTime);
state = StateEnum.Stoped;
timer1.Enabled = false;
}
}
private void FileButton_Click(object sender, EventArgs e)
{
if (!times.Any())
{
return;
}
var filename = PromtSaveAsFileName();
if (filename == null)
{
return;
}
WriteResultsToFile(filename);
times.Clear();
}
private void WriteResultsToFile(string filename)
{
System.IO.File.WriteAllLines(filename, tim
Solution
Use Correct Types in the Model
This is your model:
This is an example of stringly typed programming. Store data in meaningful types and convert them to other types (string, byte[] etc) for transfer, display etc. as necessary. For example storing
Looking from another perspective, you model should depend on the requirements, and as little else as possible. reading the problem description, and not bothering with menial details, thus not putting the cart before the horse, you would come up with some model like this:
Separation of Concerns
We see here several consequences of a user action bunched up:
Suppose you decided to change one aspect, say how
Another way of organizing things is the OO way, group data and related behavior together:
Which causes form event handlers to look like these:
This is your model:
Stopwatch stopWatch;
TimeSpan timeSpan;
string elapsedTime;
List times = new List();This is an example of stringly typed programming. Store data in meaningful types and convert them to other types (string, byte[] etc) for transfer, display etc. as necessary. For example storing
times as List you can convert this to a list of running totals or display a graph etc without parsing, or worse trying to parse, the strings back to a meaningful type.Looking from another perspective, you model should depend on the requirements, and as little else as possible. reading the problem description, and not bothering with menial details, thus not putting the cart before the horse, you would come up with some model like this:
Stopwatch stopWatch; // from (start lap stop) requirement
List times = new List(); // from (write to file) requirementSeparation of Concerns
We see here several consequences of a user action bunched up:
stopWatch = Stopwatch.StartNew();
state = StateEnum.Started;
ChangeButtonText(StartAndLapButton, "Lap");
ChangeFormTitle("Started");
timer1.Enabled = true;Suppose you decided to change one aspect, say how
times are stored, then you will have to look for every code block that refers to it, and try to judge whether the change you will make affect the behavior of the code in the same code block below the changes.Another way of organizing things is the OO way, group data and related behavior together:
event Action OnStart = () => {};
event Action OnLap = () => {};
event Action OnStop = () => {};
void ConfigureStopWatchBehavior()
{
OnStart += stopWatch.Restart;
OnStop += stopWatch.Stop;
}
void ConfigureTimesBehavior()
{
Action storeElapsedTime = () => times.Add(stopWatch.Elapsed);
OnLap += storeElapsedTime;
OnStop += storeElapsedTime;
}
void ConfigureViewBehavior()
{
OnStart += () => {
StartAndLapButton.Text = "Lap";
this.Text = "Started";
};
OnStop += () => {
StartAndLapButton.Text = "Start";
this.Text = "Stop Watch";
};
}
void ConfigureViewStateBehavior()
{
OnStart += () => state = StateEnum.Started;
OnStop += () => state = StateEnum.Stoped;
}
void ConfigureTimerBehavior()
{
OnStart += timer1.Start;
OnStop += timer1.Stop;
}Which causes form event handlers to look like these:
private void StartAndLapButton_Click(object sender, EventArgs e)
{
if (state == StateEnum.Stoped) OnStart();
else if (state == StateEnum.Started) OnLap();
}
private void StopButton_Click(object sender, EventArgs e)
{
if (state == StateEnum.Started) OnStop();
}Code Snippets
Stopwatch stopWatch;
TimeSpan timeSpan;
string elapsedTime;
List<string> times = new List<string>();Stopwatch stopWatch; // from (start lap stop) requirement
List<TimeSpan> times = new List<TimeSpan>(); // from (write to file) requirementstopWatch = Stopwatch.StartNew();
state = StateEnum.Started;
ChangeButtonText(StartAndLapButton, "Lap");
ChangeFormTitle("Started");
timer1.Enabled = true;event Action OnStart = () => {};
event Action OnLap = () => {};
event Action OnStop = () => {};
void ConfigureStopWatchBehavior()
{
OnStart += stopWatch.Restart;
OnStop += stopWatch.Stop;
}
void ConfigureTimesBehavior()
{
Action storeElapsedTime = () => times.Add(stopWatch.Elapsed);
OnLap += storeElapsedTime;
OnStop += storeElapsedTime;
}
void ConfigureViewBehavior()
{
OnStart += () => {
StartAndLapButton.Text = "Lap";
this.Text = "Started";
};
OnStop += () => {
StartAndLapButton.Text = "Start";
this.Text = "Stop Watch";
};
}
void ConfigureViewStateBehavior()
{
OnStart += () => state = StateEnum.Started;
OnStop += () => state = StateEnum.Stoped;
}
void ConfigureTimerBehavior()
{
OnStart += timer1.Start;
OnStop += timer1.Stop;
}private void StartAndLapButton_Click(object sender, EventArgs e)
{
if (state == StateEnum.Stoped) OnStart();
else if (state == StateEnum.Started) OnLap();
}
private void StopButton_Click(object sender, EventArgs e)
{
if (state == StateEnum.Started) OnStop();
}Context
StackExchange Code Review Q#69520, answer score: 3
Revisions (0)
No revisions yet.