Skip to content

[Request] More readable way to set polling interval in After constraint #1866

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

Closed
rvignesh89 opened this issue Oct 25, 2016 · 6 comments
Closed

Comments

@rvignesh89
Copy link

This issue is a follow up of #1837 (originally requested by @mbendtsen ) to work on improving the readability of setting a polling interval in the After constraint.

@rvignesh89
Copy link
Author

@CharliePoole I was thinking of using extensions to the integer class to simplify the readability, something like this. What do you think? This should be straightforward but adds extra braces which I don't like :)

   Assert.That(actual, Is.EqualTo(expected).After(3.Minutes()).PollyEvery(5.MilliSeconds());

@rvignesh89
Copy link
Author

One of the limitations that I see with the above piece of code is, how will the user of the library know that he can do something like this. because the signature of After would probably be like this,

 public DelayedConstraint After(Interval timeInterval)
        {
            ...
        }

But this does not expose to the user about how to create an Interval.

@CharliePoole
Copy link
Member

@rvignesh89 I gave this the design label to start with.

Our earlier discussion was that we would implement new modifiers:

  • PollEvery(int n) to set the polling interval amount
  • minute, second and millisecond to set the polling interval measurement type
    Taken together with the earlier modifiers, that's a total of seven modifiers that can appear after After. Unless we do something about it, they can appear in any order and even be duplicated. The intellisense will suggest all of them at each point, which is potentially confusing.

I think the best solution, if you can implement it, would be to allow only certain sequences at compile time. You could do that using nested classes, derived from AfterConstraint itself.

  • After(int, int) would give the base class, which would allow no further mods.
  • After(int) would give nested class 1, allowing entry a dimension (minutes, etc.) or PollEvery
  • Entering Minutes would give nested class 2, allowing only PollEvery
  • PollEvery would give nested class 3, allowing only a polling dimension (minute, etc.)
  • Minute would give the base class. Allowing no further modifiers.
  • A class with a polling value but no dimension
    This is completely provisional, of course. What do you think?

@CharliePoole
Copy link
Member

@rvignesh89 Our comments crossed.

I am suspicious of adding extensions to basic classes like int. If we do it here it's going to pop up in other places as a user option. And the resulting syntax, as you point out is not that great.

What do you think of the nested class idea?

@rvignesh89
Copy link
Author

@CharliePoole I like the idea, but at the moment I'm just not sure how I would do it :) The good part is, it still sticks to the syntax which @mbendtsen originally requested for. So I'm guessing it is more intuitive compared to my idea.

I'll give it a try first and see :)

@CharliePoole
Copy link
Member

Remember that you can always do a PR to get code review, even if the work is only partially done.

One approach to start would be to convert the existing code to use a nested class for the modifiers, thereby eliminating the problem of duplicate dimensions. Polling interval would then be a second commit.

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

No branches or pull requests

3 participants