Skip to content

Conversation

@bcowdery
Copy link
Contributor

Fix for Issue #674,

Moves DateTime and TimeSpan extensions into the "FluentAssertions.Extensions" namespace. I've also renamed the TimeSpanConversionExtensions to FluentTimeSpanExtensions to follow the naming conventions used in other "helper" extensions.

@dennisdoomen dennisdoomen merged commit 7f9f912 into fluentassertions:release-5.0 Oct 16, 2017
public static TimeSpan Nanoseconds(this int nanoseconds)
{
return ((long)Math.Round(nanoseconds * TicksPerNanosecond)).Ticks();
return ((long)Math.Round((double)(nanoseconds * TicksPerNanosecond))).Ticks();
Copy link
Member

Choose a reason for hiding this comment

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

Why this cast?

public static TimeSpan Nanoseconds(this long nanoseconds)
{
return ((long)Math.Round(nanoseconds * TicksPerNanosecond)).Ticks();
return ((long)Math.Round((double)(nanoseconds * TicksPerNanosecond))).Ticks();
Copy link
Member

Choose a reason for hiding this comment

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

Why this cast?

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 used ReSharpers refactor tools to move the class into a new package, I think it got a bit overzealous and decided to add casts. I'll back out the change.

Sorry about that!

Copy link
Member

Choose a reason for hiding this comment

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

No need to feel sorry :)
I just read through the diff and noticed they were introduced, so i spun up VS to see they were marked as redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. Marked as redundant as on mine as well. Not sure why VS introduced it, I might have fat-fingered my new keyboard this morning and done it by accident or maybe ReSharper was trying to be cleaver.

In either case, I've removed the casts and created new pull request #676. If it's easier to just make the change in the code base just kill the PR and I'll merge the change back into my fork later.

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants