fix(type): validate DecimalType construction and drop the init setters - #19
Merged
Merged
Conversation
DecimalType.Numeric validated; the record's generated constructor and init
setters did not, so `new DecimalType(10, 30)` and
`new DecimalType(10, 2) { Scale = 30 }` both produced NUMERIC(10,30) with
IntegerDigits = -20 — a value that feeds DecimalTypeRules.Clamp and the width
selection rather than failing at construction.
Replace the positional record declaration with explicit get-only properties, a
validating constructor, and a hand-written Deconstruct. The constructor now
rejects precision outside 1..76 and scale above precision (two comparisons,
behind non-inlined throw helpers), and with no init setters the
object-initializer route is gone. Numeric keeps its int overload so
out-of-range values are rejected before the narrowing conversion to byte, and
is otherwise an alias for the constructor.
Fixes #17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #17.
DecimalType.Numericvalidated; the record's generated constructor andinitsetters did not, so both of these producedNUMERIC(10,30)withIntegerDigits = -20:That value is not inert — it feeds
DecimalTypeRules.Clampand theWidthtier selection, so it propagates into the arithmetic instead of failing at construction.What changed
Options 1 and 3 from the issue, together. The positional record declaration is replaced with explicit get-only
Precision/Scale, a validating constructor, and a hand-writtenDeconstruct:Numericis now an alias for it rather than the only validating door;initsetters, the object-initializer bypass is gone entirely;Numerickeeps itsintparameters and checks them first, soNumeric(300, 0)is still rejected rather than truncated to precision 44 by the conversion tobyte.Cost is two comparisons at construction, behind
NoInliningthrow helpers so the constructor stays inlinable. Nothing in the library constructs aDecimalTypeper element — the kernels take the types as arguments — and the only in-repo construction site isDecimalTypeRules.Clamp, once per operation.The remaining unvalidated value is
default(DecimalType), which a struct always permits; it is now documented on the type as behaving likeNUMERIC(0,0).Compatibility
Source- and binary-breaking for anyone who used the
initsetters (an object initializer orwith) — the removedset_Precision/set_Scaleare gone from the public surface. The positional constructor keeps its signature and now throws for inputs it previously accepted. Nothing in this repo used either route.AssemblyVersionis deliberately left at0.3.0.0— the csproj comment says to bump it to0.4.0.0at the next breaking release, and that call (plus whether this ships alone or with other breaks) is yours.Tests
Six new cases in
DecimalTypeTests: constructor rejection for scale > precision, precision 0, and precision 77 (each asserting theParamName), constructor/Numericagreement,Deconstruct, and an exhaustive sweep over all 1..76 precisions assertingIntegerDigits >= 0for every constructible type and thatscale = precision + 1throws. Full suite passes on net8.0, net10.0, and net472 (549 tests).