583 years and months #604
583 years and months #604
Conversation
Fixes string resolution of a tiespan including years and months
| months += monthCount; | ||
|
|
||
| return months; | ||
| } |
There was a problem hiding this comment.
I think it would be better to write 142-162 as:
for (var monthCount = 0; daysRemaining > _daysInMonths[monthCount]; monthCount++)
daysRemaining -= _daysInMonths[monthCount];
return isTimeUnitToGetTheMaximumTimeUnit
? months + monthCount
: daysRemaining;I didnt test it. So feel free to change anything else ;)
| return timespan.Days % _daysInAWeek; | ||
| } | ||
| return (int)(timespan.TotalDays%_daysInAYear); | ||
| } |
There was a problem hiding this comment.
Also
{
return (int)(isTimeUnitToGetTheMaximumTimeUnit
? Math.Floor(timespan.TotalDays / _daysInAYear)
: timespan.TotalDays % _daysInAYear);
}There was a problem hiding this comment.
You can benefit of ternary operator further 👍
| } | ||
| private static List<string> CreateTimePartsWithNoTimeValue(string noTimeValue) | ||
| { | ||
| return new List<string>() |
| } | ||
| private static bool IsContainingOnlyNullValue(IEnumerable<string> timeParts) | ||
| { | ||
| return (timeParts.Count(x => x != null) == 0); |
| @@ -8,190 +8,235 @@ | |||
|
|
|||
| namespace Humanizer | |||
| { | |||
| /// <summary> | |||
There was a problem hiding this comment.
Doesn't seem to obey the .editorconfig, stating 4 spaces.
This makes the diff much larger than it's supposed to.
|
@reinhardtb Thanks for the contribution. This has been sitting here idly for about a year now. Could you please address the comments, do a rebase and push force? Please also fix the whitespace diffs as they make it hard to review the code. Please leave us a comment. Thanks. |
|
Actually I will close this for now. Please submit a new PR and we'll review. |
Fixes string resolution of a timespan including years and months