Skip to content
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

ParseOptions should be an enum #51

Closed
mklement0 opened this issue May 22, 2019 · 4 comments

Comments

Projects
None yet
3 participants
@mklement0
Copy link

commented May 22, 2019

This API is very hard to use from PowerShell due to needing to construct a ParseOptions object in order to call Parse.

$parseOptions = [NCrontab.CrontabSchedule+ParseOptions]::new()
$parseOptions.IncludingSeconds = $true
[NCrontab.CrontabSchedule]::Parse('1 0 2 * * 6', $parseOptions)
@atifaziz

This comment has been minimized.

Copy link
Owner

commented May 23, 2019

The reason it's not an enum is that I can't say if future options will all be just Boolean flags and using an options object is a more extensible approach from forward and backward compatibility angle. In general, a .NET library will have to be wrapped for it to appear PowerShell-wise idiomatic. You could avoid introducing $parseOptions by re-writing the initialization as follows:

[NCrontab.CrontabSchedule]::Parse('1 0 2 * * 6', (
    New-Object NCrontab.CrontabSchedule+ParseOptions -Property @{ IncludingSeconds = $true }))

What would be even better is to introduce a small helper function if you'll be doing that often.

I also took a quick crack at what making NCrontab very friendly to use in PowerShell would look like and here's that experiment:

<#
MIT License

Copyright (c) 2019 Atif Aziz

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
#>

function Get-CrontabSchedule() {

    [CmdletBinding()]
    param(
        [Parameter(Mandatory = $true)]
        [string]$Expression,
        [datetime]$Start,
        [datetime]$End,
        [int]$Count,
        [switch]$NoOccurrences = $false)

    # Assume NCrontab has been loaded before:
    # Add-Type -Path NCrontab.dll

    $options = [NCrontab.CrontabSchedule+ParseOptions]@{
        IncludingSeconds = ($expression -split ' +', 6).Count -gt 5
    }
    $schedule = [NCrontab.CrontabSchedule]::Parse($expression, $options)

    if (!$start) {
        $start = Get-Date
    }

    if (!$end) {
        $end = [datetime]::MaxValue
        if (!$count) {
            $count = 20
        }
    }

    if ($noOccurrences) {
        $schedule
    }
    else {
        $occurrences = $schedule.GetNextOccurrences($start - ((New-TimeSpan) - 1), $end)
        if ($count) {
            $occurrences | Select-Object -First $count
        } else {
            $occurrences
        }
    }
}

Given just an expression, it will dump the next 20 occurrences:

PS> Get-CrontabSchedule '1 0 2 * * 6'

25 May 2019 02:00:01
01 June 2019 02:00:01
08 June 2019 02:00:01
15 June 2019 02:00:01
22 June 2019 02:00:01
29 June 2019 02:00:01
06 July 2019 02:00:01
13 July 2019 02:00:01
20 July 2019 02:00:01
27 July 2019 02:00:01
03 August 2019 02:00:01
10 August 2019 02:00:01
17 August 2019 02:00:01
24 August 2019 02:00:01
31 August 2019 02:00:01
07 September 2019 02:00:01
14 September 2019 02:00:01
21 September 2019 02:00:01
28 September 2019 02:00:01
05 October 2019 02:00:01

You can change that with -Count:

PS> Get-CrontabSchedule '1 0 2 * * 6' -Count 5

25 May 2019 02:00:01
01 June 2019 02:00:01
08 June 2019 02:00:01
15 June 2019 02:00:01
22 June 2019 02:00:01

Use -Start to specify a start date/time:

PS> Get-CrontabSchedule '1 0 2 * * 6' -Start '2019-01-01' -Count 5

05 January 2019 02:00:01
12 January 2019 02:00:01
19 January 2019 02:00:01
26 January 2019 02:00:01
02 February 2019 02:00:01

Finally, if you just want to parse the expression without the evaluation of next occurrences, you can use the -NoOccurrences flag to get back the raw CrontabSchedule object:

PS> Get-CrontabSchedule '1 0 2 * * 6' -NoOccurrences | gm


   TypeName: NCrontab.CrontabSchedule

Name               MemberType Definition                                        
----               ---------- ----------                                        
Equals             Method     bool Equals(System.Object obj)                    
GetHashCode        Method     int GetHashCode()                                 
GetNextOccurrence  Method     datetime GetNextOccurrence(datetime baseTime), ...
GetNextOccurrences Method     System.Collections.Generic.IEnumerable[datetime...
GetType            Method     type GetType()                                    
ToString           Method     string ToString()                                 

Hope this helps.

I will close this issue now on the basis that PowerShell shouldn't be the driver of API design. That said, I'd be happy to discuss further (or even collaborate) if the idea of rendering NCrontab friendlier to use in PowerShell through, say, a PowerShell module would be interesting.

@atifaziz atifaziz closed this May 23, 2019

@mklement0

This comment has been minimized.

Copy link
Author

commented May 23, 2019

Thank you for your detailed and thoughtful response.

I'd actually forgotten about New-Object's ability to initialize instance properties as an alternative to calling a constructor with arguments.

You can even use syntactic sugar with casts:

[NCrontab.CrontabSchedule+ParseOptions] @{ IncludingSeconds = $true }

(Obviously, an [enum] is still much more convenient, because PowerShell allows you to simply pass strings ('IncludingSeconds'), but I understand your reasoning re extensibility.)

And thanks for the PowerShell function.

@atifaziz

This comment has been minimized.

Copy link
Owner

commented May 23, 2019

You can even use syntactic sugar with casts:

[NCrontab.CrontabSchedule+ParseOptions] @{ IncludingSeconds = $true }

Now you see, even I'd forgotten that terser method, but glad to see one suggestion leading to another. 😄

@jzabroski

This comment has been minimized.

Copy link

commented May 23, 2019

Very impressive, Atif.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.