Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #101: Draft for precision-based calculator #114

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/Humanizer.Tests/AmbientCulture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ namespace Humanizer.Tests
{
public class AmbientCulture : IDisposable
{
private readonly CultureInfo _culture;
private readonly CultureInfo _callerCulture;

public AmbientCulture(CultureInfo culture)
{
_culture = Thread.CurrentThread.CurrentUICulture;
_callerCulture = Thread.CurrentThread.CurrentUICulture;
Thread.CurrentThread.CurrentCulture = culture;
Thread.CurrentThread.CurrentUICulture = culture;
}
Expand All @@ -22,7 +22,8 @@ public AmbientCulture(string cultureName)

public void Dispose()
{
Thread.CurrentThread.CurrentUICulture = _culture;
Thread.CurrentThread.CurrentCulture = _callerCulture;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. Thanks for fixing this :)

Thread.CurrentThread.CurrentUICulture = _callerCulture;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ public class CasingExtensions

public class Configurator
{
public double DefaultPrecision { get; }
public Humanizer.DistanceOfTimeCalculators.IDistanceOfTimeInWords DistanceOfTimeInWords { get; set; }
public Humanizer.Localisation.IFormatter Formatter { get; }
}

Expand All @@ -73,6 +75,33 @@ public class DateHumanizeExtensions
public string Humanize(System.DateTime input, bool utcDate, System.Nullable<System.DateTime> dateToCompareAgainst) { }
}

public class DefaultDistanceOfTime
{
public DefaultDistanceOfTime() { }
public string Calculate(System.DateTime date1, System.DateTime date2) { }
}

public class DistanceOfTime
{
public DistanceOfTime() { }
public int Count { get; set; }
public Humanizer.Localisation.TimeStructure Structure { get; set; }
public Humanizer.Localisation.TimeUnitTense Tense { get; set; }
public Humanizer.Localisation.TimeUnit Unit { get; set; }
public Humanizer.DistanceOfTimeCalculators.DistanceOfTime Create(Humanizer.Localisation.TimeUnit timeUnit, Humanizer.Localisation.TimeUnitTense timeUnitTense, Humanizer.Localisation.TimeStructure timeStructure, int count) { }
}

public interface IDistanceOfTimeInWords
{
string Calculate(System.DateTime date1, System.DateTime date2);
}

public class PrecisionBasedDistanceOfTime
{
public PrecisionBasedDistanceOfTime() { }
public string Calculate(System.DateTime date1, System.DateTime date2) { }
}

public class EnumDehumanizeExtensions
{
public TTargetEnum DehumanizeTo(string input) { }
Expand Down Expand Up @@ -151,6 +180,7 @@ public class DefaultFormatter
public DefaultFormatter() { }
public string DateHumanize(Humanizer.Localisation.TimeUnit timeUnit, Humanizer.Localisation.TimeUnitTense timeUnitTense, int unit) { }
public string DateHumanize_Now() { }
public string Humanize(Humanizer.DistanceOfTimeCalculators.DistanceOfTime distanceOfTime) { }
public string TimeSpanHumanize(Humanizer.Localisation.TimeUnit timeUnit, int unit) { }
public string TimeSpanHumanize_Zero() { }
}
Expand All @@ -159,6 +189,7 @@ public interface IFormatter
{
string DateHumanize(Humanizer.Localisation.TimeUnit timeUnit, Humanizer.Localisation.TimeUnitTense timeUnitTense, int unit);
string DateHumanize_Now();
string Humanize(Humanizer.DistanceOfTimeCalculators.DistanceOfTime timeUnit);
string TimeSpanHumanize(Humanizer.Localisation.TimeUnit timeUnit, int unit);
string TimeSpanHumanize_Zero();
}
Expand All @@ -173,6 +204,13 @@ public class Resources
public string GetResource(string resourceKey) { }
}

public enum TimeStructure
{
DateTime,
TimeSpan,
value__,
}

public enum TimeUnit
{
Day,
Expand Down
27 changes: 1 addition & 26 deletions src/Humanizer.Tests/DateHumanizeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,8 @@

namespace Humanizer.Tests
{
public class DateHumanizeTests
public class DateHumanizeTests : DateHumanizeTestsBase
{
static void VerifyWithCurrentDate(string expectedString, TimeSpan deltaFromNow)
{
var utcNow = DateTime.UtcNow;
var localNow = DateTime.Now;

// feels like the only way to avoid breaking tests because CPU ticks over is to inject the base date
Assert.Equal(expectedString, utcNow.Add(deltaFromNow).Humanize(utcDate: true, dateToCompareAgainst: utcNow));
Assert.Equal(expectedString, localNow.Add(deltaFromNow).Humanize(utcDate: false, dateToCompareAgainst: localNow));
}

static void VerifyWithDateInjection(string expectedString, TimeSpan deltaFromNow)
{
var utcNow = new DateTime(2013, 6, 20, 9, 58, 22, DateTimeKind.Utc);
var now = new DateTime(2013, 6, 20, 11, 58, 22, DateTimeKind.Local);

Assert.Equal(expectedString, utcNow.Add(deltaFromNow).Humanize(dateToCompareAgainst: utcNow));
Assert.Equal(expectedString, now.Add(deltaFromNow).Humanize(false, now));
}

static void Verify(string expectedString, TimeSpan deltaFromNow)
{
VerifyWithCurrentDate(expectedString, deltaFromNow);
VerifyWithDateInjection(expectedString, deltaFromNow);
}

[Fact]
public void OneSecondFromNow()
{
Expand Down
43 changes: 43 additions & 0 deletions src/Humanizer.Tests/DateHumanizeTestsBase.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using System;
using Humanizer.Configuration;
using Humanizer.DistanceOfTimeCalculators;
using Xunit;

namespace Humanizer.Tests
{
public abstract class DateHumanizeTestsBase : IDisposable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good this is extracted out as we need this in #123.

{
private readonly IDistanceOfTimeInWords previousCalculator;

protected DateHumanizeTestsBase()
{
previousCalculator = Configurator.DistanceOfTimeInWords;
}

public void Dispose()
{
Configurator.DistanceOfTimeInWords = previousCalculator;
}

protected static void Verify(string expectedString, TimeSpan deltaFromNow)
{
VerifyWithCurrentDate(expectedString, deltaFromNow);
VerifyWithDateInjection(expectedString, deltaFromNow);
}

private static void VerifyWithCurrentDate(string expectedString, TimeSpan deltaFromNow)
{
Assert.Equal(expectedString, DateTime.UtcNow.Add(deltaFromNow).Humanize());
Assert.Equal(expectedString, DateTime.Now.Add(deltaFromNow).Humanize(false));
}

private static void VerifyWithDateInjection(string expectedString, TimeSpan deltaFromNow)
{
var utcNow = new DateTime(2013, 6, 20, 9, 58, 22, DateTimeKind.Utc);
var now = new DateTime(2013, 6, 20, 11, 58, 22, DateTimeKind.Local);

Assert.Equal(expectedString, utcNow.Add(deltaFromNow).Humanize(dateToCompareAgainst: utcNow));
Assert.Equal(expectedString, now.Add(deltaFromNow).Humanize(false, now));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
using System;
using Humanizer.DistanceOfTimeCalculators;
using Humanizer.Localisation;
using Xunit;

namespace Humanizer.Tests.DistanceOfTimeCalculators
{
public class DefaultPrecisionBoundaries
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I just had an aha moment. I never thought about the logic behind DateTime Humanize breakpoints; but looking at your breakpoints here made me realize it is calculated on a .75 precision! So I think having two separate strategies for DateTime calculation feels redundant as they're basically the same. The only difference is that in the default one the breakpoints are hardcoded while in the precision based one they're calculated.

This makes me wonder: do we even need the default one? I think we should just replace the default implementation with the precision based calculator, and we don't need to create the interface or make this configurable beyond providing the desired precision.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that the default one is .75 for all values. We can validate this by tests, just swap the calculator & run same tests ;) The point, I was thinking to introduce the change in a non-breaking way. So deliberately anyone wants to leverage the new precision-based calculator can do this, if not he's fine & nothing changed for him.

{
readonly PrecisionBasedDistanceOfTime _calculator = new PrecisionBasedDistanceOfTime();

private readonly string justNow = GetJustNow();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this had to get its separate method while other resources were fetched inline?! Maybe this should be inlined too.

private readonly string secondFromNow = GetResource(TimeUnit.Second, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact I am not sure why these are created here and not in the test methods they're used as each field is used only once.

private readonly string minuteFromNow = GetResource(TimeUnit.Minute, 1);
private readonly string hourFromNow = GetResource(TimeUnit.Hour, 1);
private readonly string dayFromNow = GetResource(TimeUnit.Day, 1);
private readonly string monthFromNow = GetResource(TimeUnit.Month, 1);
private readonly string yearFromNow = GetResource(TimeUnit.Year, 1);

[Fact]
public void MillisecondsBorder()
{
const int millisecondsBoder = 750;
VerifyBorder(TimeSpan.FromMilliseconds, millisecondsBoder, justNow, secondFromNow);
}

[Fact]
public void FourtyFiveSecondsBorder()
{
const int secondsBorder = 45;
VerifyBorder(TimeSpan.FromSeconds, secondsBorder,
GetResource(TimeUnit.Second, secondsBorder - 1),
minuteFromNow);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be easier to read the test in my opinion if we got the minuteFromNow resource here just like the line above. Same applies to all the other fields defined on the top.

}

[Fact]
public void FourtyFiveMinutesBorder()
{
const int minutesBorder = 45;
VerifyBorder(TimeSpan.FromMinutes, minutesBorder,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you break the argument list into lines, it's easier on eye if each argument appeared on a separate line. So I think this should ultimately be reformatted to:

VerifyBorder(
    TimeSpan.FromMinutes, 
    minutesBorder,
    GetResource(TimeUnit.Minute, minutesBorder - 1),
    GetResource(TimeUnit.Hour, 1));

GetResource(TimeUnit.Minute, minutesBorder - 1),
hourFromNow);
}

[Fact]
public void EighteenHoursBorder()
{
const int hoursBorder = 18;
VerifyBorder(TimeSpan.FromHours, 18,
GetResource(TimeUnit.Hour, hoursBorder - 1),
dayFromNow);
}

[Fact]
public void TwentyThreeDaysBorder()
{
const int monthBorderInDays = 23;
VerifyBorder(TimeSpan.FromDays, monthBorderInDays,
GetResource(TimeUnit.Day, monthBorderInDays - 1),
monthFromNow);
}

[Fact]
public void TwoHundredAndFourDaysBorder()
{
const int yearBorderInDays = 274;
VerifyBorder(TimeSpan.FromDays, yearBorderInDays,
GetResource(TimeUnit.Month, 9),
yearFromNow);
}

private void VerifyBorder(Func<double, TimeSpan> timeSpanFunc, int border, string beforeBorder, string afterBorder)
{
var date = DateTime.UtcNow;
var date1 = date.Add(timeSpanFunc(border - 1));
var date2 = date.Add(timeSpanFunc(border));
var date3 = date.Add(timeSpanFunc(border + 1));

var actual1 = _calculator.Calculate(date1, date);
var actual2 = _calculator.Calculate(date2, date);
var actual3 = _calculator.Calculate(date3, date);

Assert.Equal(beforeBorder, actual1);
Assert.Equal(afterBorder, actual2);
Assert.Equal(afterBorder, actual3);
}

private static string GetResource(TimeUnit timeUnit, int count)
{
string resourceKey = ResourceKeys.DateHumanize.GetResourceKey(timeUnit, TimeUnitTense.Future, count);
return Resources.GetResource(resourceKey).FormatWith(count);
}

private static string GetJustNow()
{
return Resources.GetResource(ResourceKeys.DateHumanize.Now);
}
}
}
Loading