quickstep-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hakanmemisoglu <...@git.apache.org>
Subject [GitHub] incubator-quickstep pull request #98: QUICKSTEP-53 DateLit improvements
Date Wed, 07 Sep 2016 01:57:31 GMT
Github user hakanmemisoglu commented on a diff in the pull request:

    --- Diff: types/DatetimeLit.hpp ---
    @@ -51,53 +54,70 @@ struct DateLit {
             + 1   // -
             + 2;  // Day
    +  // Years should be between [-kMaxYear, +kMaxYear] inclusive both end.
    +  static constexpr std::int32_t kMaxYear = 99999;
    +  static constexpr std::uint8_t kBitsNeededForDay = 5u;
    +  static constexpr std::uint8_t kBitsNeededForMonth = 4u;
       static DateLit Create(const std::int32_t _year,
                             const std::uint8_t _month,
                             const std::uint8_t _day) {
         DateLit date;
    -    date.year = _year;
    -    date.month = _month;
    -    date.day = _day;
    +    // Normalize year by adding kMaxYear value, because we try to
    +    // encode signed year value into an unsigned integer.
    --- End diff --
    Hi @hbdeshmukh 
    In the code, the leap year check and other checks such whether the year is valid or not,
are done within `printValueToString() `and `parseValueFromString() `. These methods use `yearField()`,
`monthField()`, `dayField()` to decode the real number from the representation. I think it
should be OK. 
    I did not understand actually why it should provide:
    > if _year is a leap year, is there a guarantee that _year + kMaxYear is also a leap
    Or do I miss some other leap year checks done in the code?

If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.

View raw message