3
\$\begingroup\$

I wasn't happy with my last attempt to parse a cron expression so I simplified it and now I use regex instead. I'd like you to take a look at this solution whether I'm doing something terribly wrong.


Here the main class is the CronExpression with only two public APIs - From that splits a cron expression fields and call their factories - Contains that passes the timestamp to each field that evaluate each date-time-part

public class CronExpression : IEnumerable<CronSubexpression>
{
    private static readonly IEnumerable<CreateSubExpressionCallback> SubExpressionFactories = new CreateSubExpressionCallback[]
    {
        ranges => new CronSecond(ranges),
        ranges => new CronMinute(ranges),
        ranges => new CronHour(ranges),
        ranges => new CronDayOfMonth(ranges),
        ranges => new CronMonth(ranges),
        ranges => new CronDayOfWeek(ranges),
        ranges => new CronYear(ranges),
    };

    private readonly List<CronSubexpression> _subexpressions;

    private CronExpression(IEnumerable<CronSubexpression> subExpressions)
    {
        _subexpressions = subExpressions.ToList();
    }

    public static CronExpression From(string input)
    {
        if (string.IsNullOrEmpty(input)) { throw new ArgumentNullException(nameof(input)); }

        var subExpressions =
            Regex
                .Split(input, @"\s")
                .Where(f => !string.IsNullOrEmpty(f))
                .Zip(SubExpressionFactories, (field, factory) => new { field, factory })
                .Select(x => x.factory(Regex.Split(x.field, @",").Select(CronRange.From)));

        return new CronExpression(subExpressions);
    }

    public bool Contains(DateTime timestamp) => this.All(x => x.Contains(timestamp));

    public IEnumerator<CronSubexpression> GetEnumerator() => _subexpressions.GetEnumerator();

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

    public override string ToString() => string.Join(" ", this.Select(x => x.ToString()));
}

internal delegate CronSubexpression CreateSubExpressionCallback(IEnumerable<CronRange> ranges);

Most of the parsing work is done in the CronRange class by the From methods. They build the regexes and try to match the values either numeric or literal.

[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class CronRange
{
    private static readonly IReadOnlyDictionary<string, int> DaysOfWeek = new[]
    {
        "SUN", "MON", "TUE", "WED", "THU", "FRI", "SAT"
    }
    .Select((dayOfWeek, index) => new { dayOfWeek, index })
    .ToDictionary(x => x.dayOfWeek, x => x.index + 1, StringComparer.OrdinalIgnoreCase);

    private static readonly IReadOnlyDictionary<string, int> Months = new[]
    {
        "JAN", "FEB", "MAR", "APR", "MAI", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC"
    }
    .Select((month, index) => new { month, index })
    .ToDictionary(x => x.month, x => x.index + 1, StringComparer.OrdinalIgnoreCase);

    public CronRange(int? min, int? max, int? step, CronExtension extension)
    {
        Min = min;
        Max = max ?? min;
        Step = step;
        Extension = extension;
        if (min.HasValue && max.HasValue && min > max) { throw new ArgumentException("min must be less then max."); }
        if (step.HasValue && step < 1) { throw new ArgumentException("step must be positive."); }
    }

    private static CronRange Empty => new CronRange(null, null, null, CronExtension.None);

    public static IEqualityComparer<CronRange> Comparer { get; } = new CronRangeEqualityComparer();

    public bool IsEmpty => !Min.HasValue;

    public int? Min { get; }

    public int? Max { get; }

    public int? Step { get; }

    public CronExtension Extension { get; }

    private const string Pattern = @"(?<Min>{0})(?:-(?<Max>{0}))?(?:\/(?<Step>{0}))?((?:#(?<nth>[1-5]))|(?<Ext>L|W))?";

    public static CronRange From(string input)
    {
        if (string.IsNullOrEmpty(input)) { throw new ArgumentNullException(nameof(input)); }

        input =
            input
                .NormalizeCronString()
                .CleanUpCronString();

        if (input == "*") { return Empty; }

        return
            FromNumeric(input) ??
            FromLiteral(input) ??
            throw new ArgumentException(nameof(input));
    }

    private static CronRange FromNumeric(string input)
    {
        var match = Regex.Match(input, string.Format(Pattern, @"\d+"), RegexOptions.ExplicitCapture);

        if (!match.Success) { return null; }

        var range = Empty;

        if (int.TryParse(match.Groups["Min"].Value, out var min)) range = range.WithMin(min);
        if (int.TryParse(match.Groups["Max"].Value, out var max)) range = range.WithMax(max);
        if (int.TryParse(match.Groups["Step"].Value, out var step)) range = range.WithStep(step);
        if (int.TryParse(match.Groups["nth"].Value, out var nth)) range = range.WithExtension((CronExtension)nth);
        if (Enum.TryParse<CronExtension>(match.Groups["Ext"].Value, out var ext)) range = range.WithExtension(ext);

        return range;
    }

    private static CronRange FromLiteral(string input)
    {
        foreach (var literals in new[] { DaysOfWeek, Months })
        {
            var match = Regex.Match(input, string.Format(Pattern, string.Join("|", literals.Keys)));

            if (!match.Success) { continue; }

            var range = Empty;

            if (literals.TryGetValue(match.Groups["Min"].Value, out var min)) range = range.WithMin(min);
            if (literals.TryGetValue(match.Groups["Max"].Value, out var max)) range = range.WithMax(max);
            if (literals.TryGetValue(match.Groups["Step"].Value, out var step)) range = range.WithStep(step);

            return range;
        }
        return null;
    }

    private string DebuggerDisplay => ToString();

    public override string ToString()
    {
        if (IsEmpty) { return "*"; }

        return
            new StringBuilder()
                .Append(Min)
                .Append(Max.HasValue && Max > Min ? $"-{Max}" : string.Empty)
                .Append(Step.HasValue ? $"/{Step}" : string.Empty)
                .ToString();
    }
}

After the parsing the data is stored in a few classes derived from CronSubexpression. They mainly provide the valid range for a field. The base class also implements the Contains method that requries a date-time-part that the derived classes need to provide. (In future some of them will additionaly evaluate the cron-expression extensions but I don't need them yet so they are "missing".)

[DebuggerDisplay("{DebuggerDisplay,nq}")]
public abstract class CronSubexpression : List<CronRange>, IGrouping<CronField, CronRange>
{
    /// <remarks>Duplicate entries are ignored.</remarks>
    protected CronSubexpression(CronField field, IEnumerable<CronRange> ranges) : base(ranges.Distinct(CronRange.Comparer))
    {
        Key = field;
        if (ranges == null) { throw new ArgumentNullException(nameof(ranges)); }
        if (this.Count(r => r.IsEmpty) > 1) { throw new ArgumentException(paramName: nameof(ranges), message: $"{Key} can have only one empty range."); }
        if (this.Any(r => r.IsEmpty) && this.Count() > 1) { throw new ArgumentException(paramName: nameof(ranges), message: $"{Key} must not have other ranges if there is an empty one."); }
        if (this.Any(r => r.Min < Min || r.Min > Max)) { throw new ArgumentOutOfRangeException(paramName: nameof(ranges), message: $"{Key} must be between {Min}-{Max}."); }
        if (this.Any(r => r.Max < Min || r.Max > Max)) { throw new ArgumentOutOfRangeException(paramName: nameof(ranges), message: $"{Key} must be between {Min}-{Max}."); }
    }

    public CronField Key { get; }

    public abstract int Min { get; }

    public abstract int Max { get; }

    private string DebuggerDisplay => ToString();

    public bool Contains(DateTime timestamp)
    {
        if (this.Any(x => x.IsEmpty))
        {
            return true;
        }

        var value = GetDatePart(timestamp);

        foreach (var range in this)
        {
            if (range.Min <= value && value <= range.Max)
            {
                if (range.Step.HasValue)
                {
                    var step = range.Step.Value;
                    while (step <= value)
                    {
                        if (value == step) { return true; }
                        step += range.Min.Value;
                    }
                    return false;
                }
                return true;
            }
        }

        return false;
    }

    protected abstract int GetDatePart(DateTime timestamp);

    public override string ToString() => string.Join(",", this.Select(se => se.ToString()));
}

The concrete implementations for each field:

public class CronSecond : CronSubexpression
{
    public CronSecond(IEnumerable<CronRange> ranges) : base(CronField.Seconds, ranges) { }

    public override int Min => 0;

    public override int Max => 59;

    protected override int GetDatePart(DateTime timestamp) => timestamp.Second;
}

public class CronMinute : CronSubexpression
{
    public CronMinute(IEnumerable<CronRange> ranges) : base(CronField.Minutes, ranges) { }

    public override int Min => 0;

    public override int Max => 59;

    protected override int GetDatePart(DateTime timestamp) => timestamp.Minute;
}

public class CronHour : CronSubexpression
{
    public CronHour(IEnumerable<CronRange> ranges) : base(CronField.Hours, ranges) { }

    public override int Min => 0;

    public override int Max => 23;

    protected override int GetDatePart(DateTime timestamp) => timestamp.Hour;
}

public class CronDayOfMonth : CronSubexpression
{
    public CronDayOfMonth(IEnumerable<CronRange> ranges) : base(CronField.DayOfMonth, ranges) { }

    public override int Min => 1;

    public override int Max => 31;

    protected override int GetDatePart(DateTime timestamp) => timestamp.Day;
}

public class CronMonth : CronSubexpression
{
    public CronMonth(IEnumerable<CronRange> ranges) : base(CronField.Month, ranges) { }

    public override int Min => 0;

    public override int Max => 11;

    protected override int GetDatePart(DateTime timestamp) => timestamp.Month;
}

public class CronDayOfWeek : CronSubexpression
{
    public CronDayOfWeek(IEnumerable<CronRange> ranges) : base(CronField.DayOfWeek, ranges) { }

    public override int Min => 1;

    public override int Max => 7;

    protected override int GetDatePart(DateTime timestamp) => (int)timestamp.DayOfWeek;
}

public class CronYear : CronSubexpression
{
    public CronYear(IEnumerable<CronRange> ranges) : base(CronField.Year, ranges) { }

    public override int Min => DateTime.MinValue.Year;

    public override int Max => DateTime.MaxValue.Year;

    protected override int GetDatePart(DateTime timestamp) => timestamp.Year;
}

There are also a few other supporting classes and extensions that I didn't include in order to not make it a wall-of-code (let me know if I should have).


I've also written a few unit tests but I'll just post some for the Seconds part as they are all quite similar.

[TestMethod]
[TestCategory("Seconds")]
public void Contains_ExactSecond_True()
{
    Assert.IsTrue(CronExpression.From("2 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 2)));
}

[TestMethod]
[TestCategory("Seconds")]
public void Contains_ExactSecond_False()
{
    Assert.IsFalse(CronExpression.From("2 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 3)));
}

[TestMethod]
[TestCategory("Seconds")]
public void Contains_SecondRange_True()
{
    Assert.IsTrue(CronExpression.From("2-8 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 2)));
    Assert.IsTrue(CronExpression.From("2-8 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 5)));
    Assert.IsTrue(CronExpression.From("2-8 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 8)));
}

[TestMethod]
[TestCategory("Seconds")]
public void Contains_SecondRange_False()
{
    Assert.IsFalse(CronExpression.From("2-8 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 1)));
    Assert.IsFalse(CronExpression.From("2-8 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 9)));
}

[TestMethod]
[TestCategory("Seconds")]
public void Contains_SecondRangeWithStep_True()
{
    Assert.IsTrue(CronExpression.From("2-8/2 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 2)));
    Assert.IsTrue(CronExpression.From("2-8/2 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 6)));
    Assert.IsTrue(CronExpression.From("2-8/2 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 8)));
}

[TestMethod]
[TestCategory("Seconds")]
public void Contains_SecondRangeWithStep_False()
{
    Assert.IsFalse(CronExpression.From("2-8/2 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 3)));
    Assert.IsFalse(CronExpression.From("2-8/2 * * * * *").Contains(new DateTime(2017, 5, 1, 8, 0, 7)));
}

I'm going to use this for a scheduler (that I'm going to write after this is ready) so I'd like to hear from you whether I made any major mistakes that will make it less robust and or have any downsides?

\$\endgroup\$
  • 1
    \$\begingroup\$ @dfhwze I completely forgot about this one. I'll try to use my brad new tokenizer for that and see how this works. \$\endgroup\$ – t3chb0t Sep 4 at 17:35
  • 1
    \$\begingroup\$ Typo: MAI should be MAY. That's probably missing from your test cases. :) \$\endgroup\$ – Roland Illig Sep 4 at 19:25
  • \$\begingroup\$ @RolandIllig ooops, I must have been thinking in German while writing it ;-] \$\endgroup\$ – t3chb0t Sep 5 at 9:45
2
\$\begingroup\$

overall looks like a great job. Still, I wonder if you could save some CPU cycles with replacing Regex in favour of string methods

var subExpressions =
            input.Split(' ', StringSplitOptions.RemoveEmptyEntries)
                .Zip(SubExpressionFactories, (field, factory) => new { field, factory })
                .Select(x => x.factory(x.field.Split(',').Select(CronRange.From)));
\$\endgroup\$
  • \$\begingroup\$ I think this possible but I try to avoid string.split like the plague ;-] \$\endgroup\$ – t3chb0t Sep 5 at 9:45
  • 2
    \$\begingroup\$ @t3chb0t I'd be interested to hear why \$\endgroup\$ – VisualMelon Sep 5 at 9:47
  • \$\begingroup\$ @VisualMelon they're so not cool. Everyone can split and by doing regex you always learn something new ;-P \$\endgroup\$ – t3chb0t Sep 5 at 9:50
  • 1
    \$\begingroup\$ @VisualMelon btw, I've heard you like tuples? lol Have you seen this question already? \$\endgroup\$ – t3chb0t Sep 5 at 9:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.