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

Stop Watch Application 2.0

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

Problem

After the tips from my previous review. I have come up with these changes:

  • 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:

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) requirement


Separation 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) requirement
stopWatch = 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.