Stop Watch Application 2.1

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

• Separation of Concerns

• Use Delegates

I'm asking you guys if this looks good. Other then separate out the FileButton_Click(), WriteResultsToFile(), and PromtSaveAsFileName() functions to another class would you recommend any other things to improve?

```
public partial class StopWatchForm : Form
{
WatchState state;
Stopwatch stopWatch;
TimeSpan timeSpan;
List times = new List();

public event Action OnStart = () => { };
public event Action OnStop = () => { };
public event Action OnLap = () => { };

public StopWatchForm()
{
InitializeComponent();
state = WatchState.Stopped;
stopWatch = Stopwatch.StartNew();
stopWatch.Stop();
ConfigureStopWatchBehavior();
ConfigureTimerBehavior();
ConfigureTimesBehavior();
ConfigureViewBehavior();
ConfigureViewStateBehavior();
}

private void StartAndLapButton_Click(object sender, EventArgs e)
{
if (state == WatchState.Stopped)
{
OnStart();
} else if (state == WatchState.Started)
{
OnLap();
}
}

private void StopButton_Click(object sender, EventArgs e)
{
if (state == WatchState.Started)
{
OnStop();
}
}

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 ConfigureStopWatchBehavior()
{
OnStart += stopWatch.Restart;
OnStop += stopWatch.Stop;
}

void ConfigureTimesBehavior()
{
Action storeElapsedtime = () => times.Add(stopWatch.Elapsed);
OnLap += storeElapsedtime;
OnStop += storeElaps

stopWatch = Stopwatch.StartNew();
stopWatch.Stop();

If you want to create a Stopwatch that's stopped, use the constructor instead of StartNew() and then immediately stopping it.

timer1

The autogenerated WinForms names don't make much sense. If you have only one timer and there is no need to name it, call it just timer.

Also, consider creating the Timer in code, so that I don't have to look at the designer to find out for example what the period of timer is (or to change it).

if (state == WatchState.Stopped)
{
    OnStart();
} else if (state == WatchState.Started)
{
    OnLap();
}

This is an unusual bracing style. The normal .Net style would be to have the else if on a new line.

TimeSpan newitem = new TimeSpan(item.Hours, item.Minutes, item.Seconds);
timesToString.Add(newitem.ToString());

What is the purpose of this code? Is it to get rid of the "milliseconds" part in the output? Are you aware that it also removes the "days" part? If you're doing this just to get the format you want, consider using a custom format string (.Net 4+ only).

string elapsedTime;

There is no reason to declare this variable early.

String.Format("{0:00}:{1:00}:{2:00}", timeSpan.Hours, timeSpan.Minutes, timeSpan.Seconds)

Why are you using two different approaches to get TimeSpans formatted in the same way (here and using the new TimeSpan() approach above)? Considering encapsulating the formatting into a common method.

StackExchange Code Review Q#70147, answer score: 3