Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

Inconsistent handling of negative values in HeaderUtilities.TryParseInt64/FormatInt64 #760

Closed
halter73 opened this issue Jan 24, 2017 · 6 comments
Assignees

Comments

@halter73
Copy link
Member

HeaderUtilities.FormatInt64 will create a string for a negative number even though HeaderUtilities.TryParseInt64 will throw trying to parse it.

@JunTaoLuo
Copy link
Contributor

Yea, we can make it consistent and get TryParseInt64 to parse negative values. Is there any use case for it currently?

@Tratcher
Copy link
Member

Other way around, we don't want negatives in either API

@JunTaoLuo
Copy link
Contributor

JunTaoLuo commented Jan 24, 2017

Maybe we should change it to TryParseUInt64 and FormatIntUInt64 and parse to/format ulong instead long? Might be a dramatic change though with updates in several other repos.

@muratg
Copy link

muratg commented Feb 3, 2017

We don't use unsigned integer types in the public API.

One idea HeaderUtilities.TryParseInt64 to HeaderUtilities.TryParseNonNegative (yuck!) or something. Backlogging for now.

@muratg muratg added this to the Backlog milestone Feb 3, 2017
@halter73
Copy link
Member Author

halter73 commented Feb 3, 2017

@muratg Can we reconsider? This is a new API, and I don't think we need to rename anything. Just make HeaderUtilities.FormatInt64 throw for negative values like HeaderUtilities.TryParseInt64. Is there a real use case for putting negative longs from response headers? More real than parsing negative values from request headers?

@muratg muratg removed this from the Backlog milestone Feb 3, 2017
@JunTaoLuo
Copy link
Contributor

This is not a new API, it existed in previous versions. It was just using long.Parse() instead. We don't need to parse/format negative numbers so throwing is fine. However, since we will throw for negative numbers, we were considering renaming it to be more consistent with what it does hence {Format/Parse}NonNegativeInt64 but that naming looks bad. We could just mention it in the doc comments though.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants