##DateTime handling##
It's possible that you're prohibited from using the DateTime struct in this method since this is apparently homework. But, in case you're not aware, problems which are as common as parsing and formatting dates and times generally already have very sophisticated and well-designed solutions. In this case, it's .NET's System.DateTime, in particular the overload of ToString which takes a format string, and TryParse. You can find these both documented on the msdn site. Using that, I believe you would be able to condense all your code down to just a few lines.
##Naming##
By convention, variables local to methods are named in camelCase- i.e. the first letter should be lower case. So Hours should be hours, etc. This also applies for method parameters- Input should be input.
You seem to be using the default naming for your controls- label1, button1, etc. These should be given descriptive names, like all other variables.
You change conventions inside Converter. Why is something which was Hours in a different method now H? Prefer the more descriptive name (hours, once the casing is fixed).
Another convention is that classes should be named as nouns, and methods as verbs. So instead of Converter, you should use Convert. Often, something in the form VerbNoun is the most descriptive: ConvertTime, for example.
##Numeric types##
Looking at your code, it seems like your hours, minutes and seconds are expected to be integers. So why are you parsing them into doubles? This is going to lead to all sorts of potential issues- rounding can cause unexpected results when doing equality comparisons for example. If you want a variable which only stores whole numbers, it's very important you use an integer type- generally int unless you have a good reason to pick another.
##Cleaning up##
Okay, here's the big one. It's pretty unpleasant how button1_Click handles a bunch of special cases, then if it's not any of those, hands over to Converter, which itself has a whole lot of complex logic. Let's work out how we can do this better.
As I said earlier, the best way to do this is to use DateTime, but that's not really saying much useful about the code you wrote, and for all I know you may have a requirement not to use those in-built methods. So I'll work on the assumption you have to do all the tricky time handling yourself.
Probably the first thing to notice is that the handling of seconds, minutes and hours are almost completely independent from each other. Here's our aim for each one:
- Seconds: Decide whether we need to round up to the next minute
- Minutes: Add one if we've rounded up the seconds. Check for the special case that we went from 59 minutes to 60, in which case we need to tick over to the next hour.
- Hours: Add one if we rounded up a minute from 59. Decide whether it's AM or PM. If it's PM, subtract 12 from the number.
Let's address these one by one.
Seconds
As you can see from the description above. Seconds are really simple. We really only need to decide whether they're greater than or equal to 30, then we can forget about them. But look at how much complexity they have in the code:
if (M == 59 && S == 59)
and
if (S >= 30)
{
S = 1;
if (S==1)
{
M = S + M;
}
}
else
{
S = 0;
}
Plus the repeated individual checks in button1_click.
So forget all that, we want a super-simple way to ask "do I need to round up my minutes?":
private bool DoMinutesRequireRoundUp(int seconds)
{
return seconds >= 30;
}
There we go, that's all we need for seconds!
Minutes
Now we get a bit more complex. We have two questions to ask here:
Will rounding up minutes put us into the next hour? What will the minutes value be after we've done any necessary rounding of seconds?
In your code you've handled this with a bunch of special cases sprayed all over the place of minutes being 59. But again, there's no need for that!
Let's start with the first, again as its own method:
private bool DoMinutesRoundUpToNextHour(int minutes, bool roundUpMinutes)
{
return roundUpMinutes && minutes==59;
}
Now we have that, we're better equipped to tackle the second
private int GetRoundedMinutes(int minutes, bool roundUpMinutes)
{
if(DoMinutesRoundUpToNextHour(minutes, roundUpMinutes))
{
return 0;
}
if(roundUpMinutes)
{
minutes++;
}
return minutes;
}
See how we've separated out this question from any concern of what exactly the seconds are, or- even more irrelevant- what the hours are.
Hours
And finally, the hours! In your code, you try to do things in a very mixed-up way, in which AM and PM checking is all jumbled up with subtracting 12, rounding minutes, and so on. There are a few steps here, but actually we can address them quite separately as long as we do them in the right order.
Here's what we need to do, and note how each task may be dependent on tasks before it, but not after it:
- Add one if the minutes were rounded up from 59 to 00
- Subtract 24 if the hours are greater than 23
- Decide whether we should display "AM" or "PM"
- Subtract 12 if the hours are greater than 12
Not trivial, but taking them one at a time, they're not too bad either. First, numbers 1 and 2:
private int GetRoundedHours(int hours, bool incrementHours)
{
if(incrementHours)
{
hours++;
}
return hours % 24;
}
Then 3:
private string GetPeriod(int hours)
{
return hours < 12 ? "AM" : "PM";
}
And finally 4:
private string Get12HourTime(int hours)
{
return hours <= 12 ? hours : hours - 12;
}
Gluing it together
So that's a bunch of methods! But now putting it all together is very easy. Let's look at how they'd work in Convert.
First, I'm going to fix the Input thing. This is pretty confusing, as it's never used. Instead I'm going to have the time elements actually passed in and used.
private string ConvertTime(int seconds, int minutes, int hours)
{
var roundUpMinute = DoMinutesRequireRoundUp(seconds);
var incrementHour = DoMinutesRoundUpToNextHour(minutes, roundUpMinute);
var roundedHour = GetRoundedHours(hours, roundUpHour);
var displayMinutes = GetRoundedMinutes(minutes, roundUpMinute);
var displayHours = Get12HourTime(roundedHour);
var period = GetPeriod(roundedHour);
return displayHours.ToString() + ":" + displayMinutes.ToString() + " " + period;
}
One thing this doesn't address is the leading zero for minutes < 10, but this can very easily be done in a few extra lines.
It's hard to do a line-by-line comparison of this to your version, because it's quite structurally different, but the key is to see how we've separated out each individual question we need answered into its own piece of code, and decoupled those questions from each other. We don't do a check like:
if (Hours == 23 && Minutes == 59 && Seconds >= 30)
Because that's conflating 3 different questions ("Do we need to round up to the next minute?", "Do we need to round up to the next hour?", "Do we need to round up to the next day"?) which we should answer separately.