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

Time conversion program optimization

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

Problem

I have just finished working on a problem on hackerrank dealing with time conversion in C++.

I'm still fairly new to programming as I only have a few semesters of programming classes under my belt and had a pretty challenging time with this program.

Below is a link to the problem itself and my code.
I solved the problem but I feel like my time complexity is unnecessarily high. It works to solve all the test cases the site provides but I am looking to become a better developer so I want to learn to write more efficient and effective code.

Input Format


A single string containing a time in 12-hour clock format (i.e.:hh:mm:ssAM or hh:mm:ssPM), where 01 <= hh <= 12.

Output Format


Convert and print the given time in 24-hour format, where 00 <= hh <= 23.

Please tell show me how I can optimize this code for a better run time.

Link to the problem

```
#include
#include
#include
#include
#include
#include
#include
using namespace std;

int main(){
string time;
cin >> time;
char delim = ':'; // delimiter for extracting colons from input
stringstream newTime; // stream to read string that holds time info
newTime > hours;
newTime >> delim;
newTime>> minutes;
newTime >> delim;
newTime>> seconds;
newTime>> ampm;
if(ampm == "PM"){ // for changing hours to 0-23 scale
switch(hours){
case 1:hours = 13;
break;
case 2:hours = 14;
break;
case 3:hours = 15;
break;
case 4:hours = 16;
break;
case 5:hours = 17;
break;
case 6:hours = 18;
break;
case 7:hours = 19;
break;
case 8:hours = 20;
break;
case 9:hours = 21;
break;
case 10:hours = 22;
break;
case 11:hours = 23;
break;
}
}
else if(ampm == "AM" && hours == 12 ){ // for changing 12

Solution

If you were writing this for real world use1, I think the right way would be to use std::get_time and std::put_time, something on this order:

#include 
#include 
#include 

int main() {
    struct std::tm t;
    std::cin >> std::get_time(&t, "%I:%M:%S%p");
    std::mktime(&t);
    std::cout << std::put_time(&t, "%H:%M:%S");
}


(Note: it's open to question whether the call to mktime is really necessary here, but at worst it's still pretty harmless).

Switch Statement

Looking more specifically at your code, the thing that probably sticks out the most is the big switch statement. I'd eliminate that in favor of a tiny bit of math:

if (ampm == "PM")
    hours += 12;


Reading Delimiters

Right now, the code to initially read the data is made somewhat ugly and unreadable (and arguably failure-prone) by it's somewhat poor handling of delimiters. It currently just reads a character and assumes it's a delimiter. In real life, you probably want to at least compare what you read to what you expected so ensure that the input is correctly formatted (and react appropriately if it's not).

Personally, I think I'd write a little operator overload like this:

std::istream &operator >> (std::istream &is, char const *pat) {

    char ch;
    while (isspace(static_cast(is.peek())))
        is.get(ch);

    while (*pat && is && *pat == is.peek() && is.get(ch)) 
        ++pat;    

    // if we didn't reach the end of the pattern, matching failed (mismatch, premature EOF, etc.)
    if (*pat) {
        is.setstate(std::ios::failbit);
    }

    return is;
}


At least to me, this supports much cleaner code to read the data:

cin >> hour >> ":" >> minute >> ":" >> second >> ampm;


...and the operator overload takes care of reading data, comparing to what was passed, and setting the stream's failbit if it didn't match. As it stands right now, I've also included skipping any leading white-space like normal extraction operators do, but it would be trivial to eliminate that if you didn't want it (but it shouldn't make any difference for solving the HackerRank problem).

  1. HackerRank is apparently using an old/broken standard library, so it won't accept this, even when you try to tell it you want to use C++14.

Code Snippets

#include <iostream>
#include <iomanip>
#include <ctime>

int main() {
    struct std::tm t;
    std::cin >> std::get_time(&t, "%I:%M:%S%p");
    std::mktime(&t);
    std::cout << std::put_time(&t, "%H:%M:%S");
}
if (ampm == "PM")
    hours += 12;
std::istream &operator >> (std::istream &is, char const *pat) {

    char ch;
    while (isspace(static_cast<unsigned char>(is.peek())))
        is.get(ch);

    while (*pat && is && *pat == is.peek() && is.get(ch)) 
        ++pat;    

    // if we didn't reach the end of the pattern, matching failed (mismatch, premature EOF, etc.)
    if (*pat) {
        is.setstate(std::ios::failbit);
    }

    return is;
}
cin >> hour >> ":" >> minute >> ":" >> second >> ampm;

Context

StackExchange Code Review Q#129771, answer score: 5

Revisions (0)

No revisions yet.