Ensure.That icon indicating copy to clipboard operation
Ensure.That copied to clipboard

Validation proposals

Open ndrwrbgs opened this issue 6 years ago • 15 comments

Paths

make sure work here includes thoughts about how to handle the case where Directory method is called but a File exists at the path

  • [ ] DirectoryExists
  • [ ] DirectoryDoesNotExist
  • [ ] FileExists
  • [ ] FileDoesNotExist

Object

  • [ ] IsNull (though Is(null) probably suffices)
  • [ ] Implements/Extends
  • [ ] IsAnyOf (analogous to Is)
  • [ ] IsNotAnyOf

Number

  • [ ] IsApproximately(accuracy/within)
  • [ ] IsPositive
  • [ ] IsNegative
  • [ ] IsNotNegative

Collection

  • [ ] SizeIsGt/Lt/Gte/Lte
  • [ ] SizeIsNot
  • [x] Contains
  • [ ] DoesNotContain
  • [ ] ^variation of above | HasNoNull
  • [ ] ContainsExactly(2, [optional] where: item => item == "foo")
  • [ ] IsEmpty (.SizeIs(0))
  • [ ] IsInAscendingOrder (IComparable only)
  • [ ] IsInDescendingOrder (IComparable only)

Dictionary

  • [ ] DoesNotContainKey
  • [ ] ContainsValue
  • [ ] DoesNotContainsValue

String

  • [ ] StartsWith #97
  • [ ] EndsWith
  • [ ] Contains
  • [ ] DoesNotContain
  • [ ] IsNullOrWhiteSpace
  • [ ] IsNullOrEmpty
  • [ ] IsEmpty
  • [ ] SizeIsGt/Lt/Gte/Lte
  • [ ] SizeIsNot
  • [ ] DoesNotMatch

ndrwrbgs avatar Nov 15 '17 06:11 ndrwrbgs

For the String and Collection SizeIs methods, they seem redundant since users could use Ensure.That(str.Length).IsGt(4). However I would suggest either feature parity (if exposing SizeIs, also expose Gt/Lt/!=), or not exposing any of them and forcing access via string.Length

ndrwrbgs avatar Nov 15 '17 06:11 ndrwrbgs

@danielwertheim I'm game to work on any/all of these if/as you approve of them, just let me know if you think they'd be valuable contributions.

ndrwrbgs avatar Nov 15 '17 22:11 ndrwrbgs

Haven't had time to look through this yet. Want to finalize 7.1 first so that it gets merged to master.

Also, before doing this, a decision to revoke the Ensure.That(...,...).Foo() or not need to be taken.

danielwertheim avatar Nov 16 '17 06:11 danielwertheim

Hi...I'm interested in contributing to this, specifically working on the numbers (IsApproximately, IsPosition, etc.). I didn't see any PR guidelines in the repo, so I was just wondering, would it be preferred that each implementation has its own PR or could I implement all four methods under the Number category and submit a single pull request for all of them?

dmarciano avatar Dec 05 '19 16:12 dmarciano

@dmarciano the smaller the better. Preferably one PR per feature if OK.

danielwertheim avatar Dec 06 '19 13:12 danielwertheim

I have implemented the IsPositive, IsNegative, IsNotNegative, and IsApproximately methods and extension methods, along with 16 new unit tests to cover these methods (all of which have passed successfully).

The project's coding style is a little different to how I usually code, but I tried to match the pattern/style as closely as possible with my new code. If there is anything off, or you would prefer I change, please let me know.

dmarciano avatar Dec 06 '19 18:12 dmarciano

Most of these implemented in the -Experimental library RE my comment in #125 I worked around it by allowing the user to specify how to handle 0, if you wanted to do similarly @dmarciano

ndrwrbgs avatar Dec 08 '19 22:12 ndrwrbgs

@ndrwrbgs Yes...I saw your comment but haven't had a chance to review the link you provided yet as I'm traveling today. I should be able to look at it first thing tomorrow morning and either give feedback or start work on a change for it. I'll keep you posted.

dmarciano avatar Dec 08 '19 22:12 dmarciano

Linked in the other convo (was trying to avoid 'advertising' as that isn't the intention here) but will link just for your ease since you're traveling :-P I implemented samples here: https://github.com/ndrwrbgs/Ensure.That.Experimental

ndrwrbgs avatar Dec 08 '19 22:12 ndrwrbgs

@ndrwrbgs what's in experimental that can't be in the public repo?

danielwertheim avatar Dec 09 '19 07:12 danielwertheim

@ndrwrbgs I have implemented the solution similar to experiemental repo you provided by creating a new enum ZeroSignMode and adding optional parameters which defaults to ZeroSignMode.IsNeither (other values possible are ZeroSignMode.IsPositive, ZeroSignMode.IsNegative, and ZeroSignMode.IsBoth).

I am currently working on updating the unit tests to confirm that everything still works as well as the new logic for zeros. I should have a commit ready in the next few hours.

dmarciano avatar Dec 09 '19 17:12 dmarciano

I think I mentioned in another thread -- absolutely nothing :P. But having it separate meant I didn't have to wait for PR, ensure parity in all 3 API forms, or (cough cough) add tests :P

I can move over any you think do belong here, this issue 71 was meant to get your feelings on which you wanted to see before sending a PR.

Part of the purpose though was also to show how Ensure.That is super extensible. So I put things in experimental to unblock some of my work and will move over anything you want, just let me know!

On Sun, Dec 8, 2019, 11:17 PM Daniel Wertheim [email protected] wrote:

@ndrwrbgs https://github.com/ndrwrbgs what's in experimental that can't be in the public repo?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/danielwertheim/Ensure.That/issues/71?email_source=notifications&email_token=ACSHCOS4DGA3W6INTZTW4ELQXXWJ5A5CNFSM4EDZYO3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGIDEUQ#issuecomment-563098194, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACSHCORP2J3BPJQWEHAGRRTQXXWJ5ANCNFSM4EDZYO3A .

ndrwrbgs avatar Dec 09 '19 17:12 ndrwrbgs

@ndrwrbgs Out of curiosity, did you implement the SizeIsNot, SizeIsGt/Lt/Gte/Lte, IsAscendingOrder/IsDescendingOrder methods in your experimental library? Just wondering because I created a new branch in my repo and I just implemented these; currently working on just adding all the unit tests. But if you've already implemented these methods, along with other ones on this list, I guess it doesn't make sense for me to re-do it.

dmarciano avatar Dec 09 '19 18:12 dmarciano

On another note, for the IsInAscendingOrder/IsInDescendingOrder , for the IDictionary interface, I'm assuming the ascending/descending order is for the keys?

dmarciano avatar Dec 09 '19 20:12 dmarciano

Hm, I wouldn't have implemented order for dictionary, semantically it doesn't make much sense. I didn't actually implement the SizeIsX because I felt it was a bit leaky, instead I exposed a .Count() property that extends an existing Param{Collection} but then moves it to Param{int} to be able to reuse all the existing assertions on int.

On Mon, Dec 9, 2019, 12:29 PM dmarciano [email protected] wrote:

On another note, for the IsInAscendingOrder/IsInDescendingOrder , for the IDictionary interface, I'm assuming the ascending/descending order is for the keys?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/danielwertheim/Ensure.That/issues/71?email_source=notifications&email_token=ACSHCORPUBL7MYGSF6RZX7DQX2TCNA5CNFSM4EDZYO3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGKSR6Y#issuecomment-563423483, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACSHCOW3OW5COAIFFDZR3GLQX2TCNANCNFSM4EDZYO3A .

ndrwrbgs avatar Dec 10 '19 05:12 ndrwrbgs