Member Avatar for Gadgetman_53

Hey,
I'm still fairly new to C++, and even more new to this forum. Any help would be greatly appreciated. Could someone look at this segment of code for me and tell me what is wrong with the "if" statements to change the time. I'm not getting the output that I want; though, it is pretty close.

void ChangeTimeToMil(int hours, int minutes, string AMPM, int& time)
{
	if((AMPM == "AM" || AMPM == "am")&& hours == 12)
	{
		time = minutes;
		cout << "  " << setfill('0') << setw(4) << time;
	}
	else if((AMPM == "AM" || AMPM == "am") && (hours >= 0 && hours <= 9))
	{
		time = (hours*100) + minutes;
		cout << "  " << setfill('0') << setw(4) << time;
	} 
	else if((AMPM == "AM" || AMPM == "am") && (hours >= 10 || hours <= 11))
	{
		time = (hours*100) + minutes;
	}
	else if(AMPM == "PM" || AMPM == "pm" && (hours > 12))
	{
		time = (hours + 12)*100 + minutes;
		cout << "  " << setfill('0') << setw(4) << time;
	} 
	else if((AMPM == "PM" || AMPM == "pm") && (hours == 12))
	{
		time = 1200 + minutes;
		cout << "  " << setfill('0') << setw(4) << time;
	} 
}

I'm sure it's something quite simple, but I can't find it.

Thanks again,
Gadgetman_53

  • 3 Contributors
  • 5 Replies
  • 894 Views
  • 17 Hours Discussion Span
  • Latest Post Latest Post by Gadgetman_53

Dani AI

Generated

Two quick causes explain the weird outputs seen in the thread: a logical-condition bug and inconsistent output. The condition that was intended to match "10 or 11" used || in a way that makes the test effectively always true, and some branches set time but never print it. Also mixing && and || without explicit parentheses can make an expression evaluate differently than intended because && binds tighter than ||. Fix the boolean logic and make the conversion single-purpose (compute and return the numeric HHMM) so printing happens in one place.

Practical checklist to harden the function:

  • Validate inputs (hours in 1..12, minutes in 0..59).
  • Normalize the AM/PM text (case-insensitive) before testing.
  • Replace ambiguous range tests with explicit ranges (e.g., hours >= 10 && hours <= 11) or explicit equality checks.
  • Convert once (handle the 12 AM / 12 PM special cases) and return the result; format/print at the caller so you cannot forget a cout in a branch.
  • Trim file input for stray whitespace or punctuation before parsing.

A robust alternative is to let the standard library parse the 12-hour string and yield a tm with the 24-hour hour value. Example using std::get_time (keeps the conversion logic out of manual Boolean juggling):

#include <iomanip>
#include <sstream>
#include <ctime>
#include <string>

int toMilitary(const std::string& s) {
    std::tm tm = {};
    std::istringstream ss(s);            // e.g. "10:00AM" or "11:59 PM"
    ss >> std::get_time(&tm, "%I:%M%p");
    if (ss.fail()) throw std::runtime_error("parse failed");
    return tm.tm_hour * 100 + tm.tm_min; // returns HHMM in 24-hour form
}

If your toolchain lacks std::get_time, follow the checklist above: normalize AM/PM, treat 12 specially (12 AM -> 00, 12 PM -> 12), and always print the formatted 4-digit result at the end. Combine ' pointer about the logical operator and 's suggestion to normalize the AM/PM string for a clean, reliable conversion.

Recommended Answers

All 5 Replies

Member Avatar for Narue

Can you provide an input to this function that produces an incorrect result?

[edit]
Also include what the correct result should be.
[/edit]

Member Avatar for Gadgetman_53

Input would be "10:00AM"

and I get "1"
also when the input is "11:59AM"
I get nothing at all

The rest works fine...


Thanks

Member Avatar for vmanes

Well, consider your line:

else if((AMPM == "AM" || AMPM == "am") && (hours >= 10 || hours <= 11))

the Boolean operator should be AND not OR, same as you did in the 0 to 9am block.
or you could simple test for equality to 10 OR equality to 11

else if((AMPM == "AM" || AMPM == "am") && (hours >= 10 && hours <= 11))
//or
else if((AMPM == "AM" || AMPM == "am") && (hours == 10 || hours == 11))

I'm not seeing how the erroneous outputs you just described occur. Are you sure they come from the code posted?
Val

Member Avatar for Narue

What's the actual input to the function? hours and minutes are integers, yet when I pass 10 and 0 with "AM" as the third parameter, it works just fine. 11 and 59 with "AM" works as well. Though you have a lot of redundancy and unnecessary code. Try this:

void ChangeTimeToMil(int hours, int minutes, string AMPM, int& time)
{
  // Normalize the string so we don't have to test combinations
  for ( string::size_type i = 0; i < AMPM.size(); i++ )
    AMPM[i] = toupper ( (unsigned char)AMPM[i] );

  if ( AMPM == "AM" && hours == 12 )
    hours = 0;
  else if ( AMPM == "PM" )
    hours += 12;

  time = hours * 100 + minutes;
}

Of course, you might also want to validate the input to make sure that it's not already in military time and act accordingly.

Member Avatar for Gadgetman_53

Thanks for the help. This is for a project in my c++ class that I turned in this past Friday night. It was just bugging me that I couldn't get the time to output correctly. The time that is input is ALWAYS in 12 hour time, it is read from a text file.

Thanks for all the help!

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.