4
\$\begingroup\$

I am new to C#. I just created a method where I can convert a string with the format HH:MM AM/PM to minutes. I was wondering if there is a better or more effective way to achieve the same result.

static void Main(string[] args)
    {

        string time1 = "11:15 AM";
        string time2 = "11:15 PM";

        calculateTimeInMinutes(time1);
        calculateTimeInMinutes(time2);

        void calculateTimeInMinutes(string currentTime)
        {
            if (currentTime.Contains("PM"))
            {
                int hours = Convert.ToInt32(currentTime.Substring(0, 2));
                int HoursInminutes = (12 + (hours % 60)) * 60;
                string minutesInString = currentTime.Split(':')[1];
                int minutes = Convert.ToInt32(minutesInString.Remove(2));
                int totalMinutes = HoursInminutes + minutes;
                Console.WriteLine(totalMinutes);
            }
            else
            {
                int hours = Convert.ToInt32(currentTime.Substring(0, 2));
                int HoursInminutes = (hours % 60) * 60;
                string minutesInString = currentTime.Split(':')[1];
                int minutes = Convert.ToInt32(minutesInString.Remove(2));
                int totalMinutes = HoursInminutes + minutes;
                Console.WriteLine(totalMinutes);
            }
        }

        Console.ReadKey();
    }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Could you explain the hours % 60 part? This baffles me completely: for any time between 00:00 and 23:59 it will do nothing. \$\endgroup\$ – VisualMelon Jul 7 '18 at 7:56
  • 1
    \$\begingroup\$ @highrun any reason to do not use datetime type? \$\endgroup\$ – Adriano Repetti Jul 7 '18 at 11:56
4
\$\begingroup\$

The code is easy to understand, and it seems to produce the correct result.


A couple of things:

1) The code for "AM" and "PM" are almost identical with the difference of 12 (the PM-addend). You should not repeat yourself.

2) You "split" the string three times: one for AM/PM, one for hours, and one for minutes. Instead you could use string[] parts = input.Split(':', ' '); to split to the useful parts in one operation.

3) You should maybe check the input for correct format and throw some meaningful exceptions in case of wrong format.

All in all my reviewed code would be something like:

int CalculateTimeInMinutes(string input)
{
  if (string.IsNullOrWhiteSpace(input))
    throw new ArgumentNullException(nameof(input));

  string[] parts = input.Split(':', ' ');

  if (parts.Length != 3)
    throw new ArgumentOutOfRangeException(nameof(input));

  if (parts[2].ToUpper() != "AM" && parts[2].ToUpper() != "PM")
    throw new ArgumentOutOfRangeException(nameof(input), "Missing AM/PM qualifier");


  if (!int.TryParse(parts[0], out int hours))
  {
    throw new ArgumentOutOfRangeException(nameof(input), "Invalid hours part.");
  }

  if (hours < 0 || hours > 12)
    throw new ArgumentOutOfRangeException(nameof(input), "Hours must be between 00 and 12 (inclusive)");

  // TODO: The same checks for minutes.
  int.TryParse(parts[1], out int minutes);

  int toPM = parts[2].ToUpper() == "PM" ? 12 : 0;
  int hoursInminutes = (toPM + (hours % 60)) * 60;
  int totalMinutes = hoursInminutes + minutes;
  return totalMinutes;
}

That said, there are well known objects and apis that can do the same operation in a few lines of codes:

int CalculateTimeInMinutes(string input)
{
  DateTime time = DateTime.Parse(input, new CultureInfo("en-US"));
  return (int)(time - time.Date).TotalMinutes;
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ new CultureInfo("en-US", false) would be even better \$\endgroup\$ – dfhwze Jun 3 '19 at 19:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.