-2
\$\begingroup\$

Take an integer representing a day of the year and translate it to a string consisting of the month followed by day of the month. For example, Day 32 would be February 1. I've already submitted my first attempt, but after looking at ctime library, I came up with this method. So, the function just takes a single parameter, a day number in a year, e.g. "60" and returns a month and day string.

#include <iostream>
#include <string>
#include <ctime>
using namespace std;

string convertDayOfYear(int num);

int main()
{
const int DAY_IN_YEAR = 60;
string time = convertDayOfYear(DAY_IN_YEAR);
cout << time << endl;
}

string convertDayOfYear(int num)
{
struct tm *date;
const long secPerday = 86400;
long days = (48 * 365) + 12;    // years since 1970 plus leap years

time_t time = days * secPerday;

date = gmtime(&time);

time += num * secPerday;

date = gmtime(&time);

char output[26];

strftime(output, 26, "%B %d", date);

string str(output);
return str;}  
\$\endgroup\$
  • \$\begingroup\$ It was pointed out in response to your previous question that the conversion depends on whether you have a day in a leap year or not: Day #61 can be March 1 or March 2. How does your new code address this concern? Is the year 2018 hard-coded in long days = (48 * 365) + 12; ? \$\endgroup\$ – Martin R Nov 2 '18 at 14:15
  • \$\begingroup\$ "For example, Day 32 would be February 1" – but your code converts 32 to February 2. \$\endgroup\$ – Martin R Nov 2 '18 at 14:24
  • \$\begingroup\$ So, the first day of the year e.g. Jan 01 is indexed as 0. You can add +1 to int num value. I somehow like beginning index at 0. \$\endgroup\$ – dino2018 Nov 2 '18 at 14:42
  • \$\begingroup\$ you can add two additional arguments to represent number of years since base year and number of leap days/years. \$\endgroup\$ – dino2018 Nov 2 '18 at 14:47
6
\$\begingroup\$

If you were doing this in real life under normal circumstances, I'd expect to use the standard library to do essentially the entire job. In particular, I'd avoid doing any of the computation myself, given that the library already has std::mktime and company to do date computations.

The only part of this that's tricky is how we get the library to convert a day number to a month/day. std::mktime will normalize a date for us, but it does pretty much the opposite of what we want. We want to give a day of the year as input, and get a month/day as output. std::mktime takes a month/day as input, and computes (among other things) the day of the year as an output.

There's still a way though: mktime also normalizes the date. So if (for example) we give it January 32nd as an input, it'll figure out that that's really February 1st.

In this case we put that to use: if we have an input of (say) 182, we fill in a struct tm saying it's the 182nd of January, then let std::mktime normalize that to give us the actual month and day (1 July in a non-leap year). First, however, we need to fill in the other fields so it'll know what year we're dealing with, so it knows whether it's a leap year.

That gives us code something like this:

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

int main(int argc, char **argv)
{
    if (argc != 2)
    {
        std::cerr << "Usage: " << argv[0] << " <daynumber>\n";
        return EXIT_FAILURE;
    }

    // get the current year, so we know whether it's a leap year:
    std::time_t current = time(nullptr);
    std::tm *now = std::localtime(&current);

    // we don't know if the day in question will be daylight savings time:
    now->tm_isdst = -1;

    // Every day is in January:
    now->tm_mon = 0;
    now->tm_mday = std::stoi(argv[1]);
    now->tm_hour = 12;    

    // Now have it normalize the date:
    std::mktime(now);

    // ...and we can print out our result:
    std::cout << std::put_time(now, "%m/%d\n");
}

Oh, I almost forgot to mention: I set the time of day to (approximately) noon, so we won't accidentally get anything funny if we happen to run it just as clocks are about to change. Likewise, we use localtime, to get the current time for the user, rather than the current time in England.

\$\endgroup\$
  • \$\begingroup\$ Great answer! Although I knew that we can pass the n th of January to be interpreted as any day of the year, I got lost in detail and forgot to use that! You might want to explain that you're using std::localtime() (rather than std::gmtime()) specifically to get the user's idea of "current year", as that detail might be missed. \$\endgroup\$ – Toby Speight Nov 5 '18 at 10:52
4
\$\begingroup\$

Avoid using namespace std;

It's dangerous to import lots of names into the global namespace - newly-added standard identifiers could easily collide with your own, causing hard-to-debug breakage in future. Don't be reluctant to write std:: where you need it - it's intentionally a very short prefix.

Think about presentation

It might be an artefact of how you've copied the code into the question, but I'm seeing code with no indentation. If this is how your code actually looks, consider using a tool such as indent (or perhaps your editor can auto-indent code?) and try to group related lines like you would group related sentences into paragraphs when writing prose.

Write main() at the end

If we define convertDayOfYear before main(), we don't need to forward-declare it.

Reject out of range values

Should we be accepting -1 or 365 as input?

Handle leap years

This code only works for days in non-leap years. With the <ctime> interface, it's not hard to work with arbitrary years.

Consider C++ library

Instead of going through the C library, we might find it easier to use the std::put_time() I/O manipulator to format our output (particularly if we're to write it to an output stream). You could even use a std::stringstream to write values, and extract using std::get_time().

Use sizeof rather than repeating magic numbers

Avoid repeating 26 here, as it's easy to update one and miss the other.

char output[26];

strftime(output, 26, "%B %d", date);

Instead, we can use sizeof, so it's always consistent:

std::strftime(output, sizeof output, "%B %d", date);

We should check the return value - we've only guessed that 26 bytes will be sufficient for conversion in all languages. I'm inclined to increase the buffer size somewhat - 26 is cutting it a little fine in Greek, for instance, and is inadequate for Thai.

Unused value

We have

date = gmtime(&time);

time += num * secPerday;

date = gmtime(&time);

We never use the first assignment to date, so that line can simply be deleted:

time += num * secPerday;
date = std::gmtime(&time);

Improved code

#include <ctime>
#include <stdexcept>
#include <string>

std::string convertDayOfYear(int num, int year)
{
    const long secPerDay = 86400;

    std::tm date = { 0, 0, 0,    // 00:00:00
                     1, 0, year-1900, // first of January
                     0, 0, 0 };  // no DST

    // add the number of days, using seconds
    auto year_start = std::mktime(&date);
    auto day_start = year_start + num * secPerDay;
    date = *std::gmtime(&day_start);

    if (date.tm_year != year-1900) {
        throw std::range_error("Invalid day number");
    }

    char output[64];
    if (!std::strftime(output, sizeof output, "%B %d", &date)) {
        throw std::runtime_error("Date conversion failed - buffer overrun");
    }

    return std::string(output);
}
#include <iostream>
int main()
{
    for (auto i: { -1, 0, 1, 31, 32, 58, 59, 60, 365, 366 }) {
        try {
            auto s = convertDayOfYear(i, 2018);
            std::cout << i << " -> " << s << '\n';
        } catch (std::exception& e) {
            std::cout << i << ": " << e.what() << '\n';
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ This takes for granted that a time_t is an integer count of seconds since an epoch. That's required by POSIX, but not by either the C or C++ standard (though in fairness, nearly all libraries conform with POSIX in this regard). \$\endgroup\$ – Jerry Coffin Nov 2 '18 at 16:17
  • \$\begingroup\$ @TobySpeight I didn't notice but you have a bug I think. tm_year should have an offset of -1900. (conforming to docs) \$\endgroup\$ – Calak Nov 2 '18 at 20:34
  • \$\begingroup\$ Did you notice the bug fixed in the rejected edit? \$\endgroup\$ – Calak Nov 4 '18 at 19:25
  • \$\begingroup\$ Thanks for the correction @Calak. I'd completely forgotten the mad offset of tm_year! \$\endgroup\$ – Toby Speight Nov 5 '18 at 8:58
  • \$\begingroup\$ Nobody (except me) is perfect (even you). I'm joking, you make an amazing job here, little oversight it happens \$\endgroup\$ – Calak Nov 5 '18 at 10:36
0
\$\begingroup\$

What's the purpose of your multiples implements iteration?

If it's for learning, the previous post was a better solution, since he only depend on C++ standard features (IMHO).

  • If it's for using in real code, why not using this class who led to the voted one into the C++20 working draft.

Now, about your code:

Don't use using namespace std; until you don't understand it. Don't take bad habit from the beginning - In C++ you don't have to prefix variable declaration with struct; - You use a int to store the number of day from 1rst january (and all other count). Be care that this number of day can't be negative. Using an unsigned int would be more logic. - When your constants never ever change (secPerDay), you can declare it static const. - Don't use std::endl until you don't want to flush the stream. If you just want a newline, use '\n'. - Don't declare local variable in ALL_CAPS - You writed date = gmtime(&time); twice, I guess it's a "typo". You an even delete the second and place it directly as an argument to strftime. - Declare your variables in the closest scope possible - Don't hardcode the counts of day from 1970. Use standard C++ (if possible) or C function. - Here you presume that days must be counted until the current year. Putting a date in 2nd parameters would be a good idea.


Version without hard-coded calculations

There is many other things we can say, but there is a way, in C++, using this C lib, to do what you want, using same header than you:

std::string convertDayOfYear(unsigned int day_number, unsigned int year = 1900)
{
    auto t = time(nullptr);
    auto info = localtime(&t);

    if (!year || year < 1900) {
        year = info->tm_year + 1900;
    }
    info->tm_sec = 0;
    info->tm_min = 0;
    info->tm_hour = 0;
    info->tm_mday = day_number;
    info->tm_mon = 0;
    info->tm_wday = 0;
    info->tm_yday = 0;
    info->tm_year = year - 1900;
    info->tm_isdst = 0;

    mktime (info);

    char output[26];
    strftime(output, 26, "%B %d", info);
    return {output};
}  
\$\endgroup\$

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.