2
\$\begingroup\$

I just submitted this code to solve the "When to take medicine" challenge on CodeChef:

#include <iostream>
#include <string>
#include <sstream>

int countDays(int dd, int subtract)
{
    return ((subtract - dd) / 2) + 1;
}

int main()
{
    int t {0};
    std::cin >> t;
    std::cin.ignore();
    while (t--)
    {
        std::string date {};
        std::getline(std::cin, date);
        std::stringstream ss {date};
        int yy {0}, mm {0}, dd {0};
        std::string temp {};
        std::getline(ss, temp, ':');
        yy = std::stoi(temp);
        std::getline(ss, temp, ':');
        mm = std::stoi(temp);
        std::getline(ss, temp, ':');
        dd = std::stoi(temp);
        int subtract {0};
        bool leap {false};
        if (yy % 4 == 0 && (!(yy % 100 == 0) || yy % 400 == 0))
        leap = true;
        switch (mm)
        {
        case 2:
            if (leap)
                subtract = 29;
            else
                subtract = 59;
                break;
        case 4: case 6: case 9: case 11:
            subtract = 61;
        break;
        default:
            subtract = 31;
        }   
        std::cout << countDays(dd, subtract) <<'\n';
    }
}

In short, it is a program to tell how many days a guy has took his pill correctly, considering that he has to take a pill every 2 days and only takes it in odd or even days, depending on the day he started to take the pill. We provide a number of test cases in the first line and then we provide a date in a single line in the format yyyy:mm:dd. Check the link above if you're interested in reading the whole problem.

The thing is that I am not really satisfied with the way I am parsing the string separated by the char :

I also want to mention that I want to stay away from any C code like the scanf() function. It would make things a lot easier, but I don't want to use C in C++.

So, how could I redesign my string parse code to make it look nicer? I am just a newbie with C++ so all feedback / criticism is greatly appreciated.

\$\endgroup\$
1
\$\begingroup\$
  1. You don't check whether the input is invalid.

  2. Try to extract useful well-named functions, like int days(int month, int year) and bool is_leap_year(int year).

  3. You can extract the numbers directly from std::cin, the colon will be left behind.
    And the colon can thereafter be extracted like any other single character.

  4. As an aside, using scanf() would probably be easier than juggling streams.

  5. break should be indented like any other Statement. It does not deserve more indentation (first occurrence), nor less (second occurrence).

\$\endgroup\$
  • \$\begingroup\$ 1, 2, 3 - all good tips! \$\endgroup\$ – msmilkshake Jul 11 at 7:25
  • \$\begingroup\$ 4 - I just wanted to avoid using C code. \$\endgroup\$ – msmilkshake Jul 11 at 7:25
  • \$\begingroup\$ 5 - I think that indentation was a copy / paste mistake. Overall, thanks for your review! \$\endgroup\$ – msmilkshake Jul 11 at 7:26
  • \$\begingroup\$ Re 4: There are projects doing their best to mate the usability of format-string driven printf/scanf with the safety, flexibility and potential performance of templates. It's just that this code shows <iostream> at its most cumbersome. \$\endgroup\$ – Deduplicator Jul 11 at 15:06
1
\$\begingroup\$

It might be overkill for just the task at hand, but I've found it useful to write an overload of operator>> to read and match a string literal:

std::istream &operator>>(std::istream &is, char const *s) { 
    while (*s && *s == is.peek()) {
        ++s;
        is.ignore(1);
    }
    if (*s != '\0')
        is.setstate(std::ios_base::failbit);
    return is;
}

With this, reading in your data becomes much simpler, and (at least IMO) the intent becomes much more apparent:

int yy;
int mm;
int dd;

std::cin >> yy >> ":" >> mm >> ":" >> dd;

Depending on what you want/need, you can make this a bit more elaborate. For one example, it can be useful to have the literals act a little like scanf format strings, so any white space in the string matches an arbitrary amount of white space in the stream. You might also want to turn that behavior on/off, depending on whether the stream's skipws flag is set.

\$\endgroup\$
  • \$\begingroup\$ What does the *s && *s mean? Not null? \$\endgroup\$ – msmilkshake Jul 11 at 7:29
  • \$\begingroup\$ @msmilkshake: You have to look at the whole thing: *s && *s == is.peek(). The first *s is equivalent to *s != 0, so with a C-style string, it means "we haven't reached the end of the string yet". the second part is *s == is.peek(), which means the character in the string passed matches with the next character to be read from the stream. \$\endgroup\$ – Jerry Coffin Jul 11 at 7:46
  • \$\begingroup\$ oh yea, operator precedence. i was not considdering it lol. \$\endgroup\$ – msmilkshake Jul 11 at 9:51

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.