Stringify, possible stepped range improvements

[bazineta]

I believe we can get better results from stringify() on stepped ranges by a couple of condition changes:

cron-parser/lib/field_stringify.js

Line 13 in f36281a

if (step !== 1 && singleRange.start === min && singleRange.end === max - step + 1) {

I think that changing singleRange.end === max - step + 1 to step > max - singleRange.end will yield more opportunities to export ranges as */<step>.

Likewise, here:

cron-parser/lib/field_stringify.js

Line 27 in f36281a

if (range.end === max - step + 1) {

I think that changing range.end === max - step + 1 to step > max - range.end will yield more opportunities to export ranges as <start>/<step>.

Basically, in both cases, don't emit a range unless necessary. I think I've got the boundary conditions right there, but I'm kind of famous for fencepost errors, fair warning.

A few test cases would be:

• '1/5 5/5 * * * *'
• '4-59/7 * * * *'
• '1/5 * * * *'
• '*/7 * * * *'

I should also note that there are some entertainingly pathological results possible from stringify(). For example, '1/5,6/4 * * * *' is a valid (arguably, stupid, but nevertheless valid) expression, but the stringify() result is pretty fun.

[bazineta]

@regevbr Those two conditional changes seem to produce better results for any test case I hand them, and so far as I can see don't break anything.

'1/5 * * * *': 1-56/5 * * * * -> 1/5 * * * *
'*/7 * * * *': 0-56/7 * * * * -> */7 * * * *

Also to be clear, the '1/5,6/4 * * * *' thing isn't an issue to me, more just a fun result where stupid input equals stupid output.

[regevbr]

@bazineta seems like a legit change to me. Can you work on a PR so @harrisiirak will be able to review it? I just submitted the change and I'm not the maintainer of the library. Of course, add more unit tests to support your case and make sure there is nothing else that can go wrong here.
About the funny case, indeed there is probably no way to cover such cases, or at least it is not worth investing the time into them.

[bazineta]

@regevbr Sure, I can do that; I've got a decent stable of unit tests to cover it now.

I figure that you are using this code live and will likely quickly see any issues from the change if you try it, so how about we do that for a few days, and then I'll do the PR?

And yeah, the funny case I just thought was neat. Your adjacency algorithm is very slick and works amazingly well at running the meat grinder backwards; we should not think poorly of it when, in certain ugly situations, it didn't reconstruct the pig from the sausage in quite the way we'd expect.

[regevbr]

@bazineta great!
Sadly we had to suspend the feature that initiated this addition so I can't help out with testing...
Thanks for the compliments :-) Indeed I don't think there is something wrong with it, but there is always room for improvement in every piece of code :-)