HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Parsing cron expression

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
expressioncronparsing

Problem

I'm building my own scheduler and I took the first step by creating a cron expression parser.

To test it I used this expression to cover each case. (I guess the 52W is wrong but this doesn't matter yet - validation comes later).

"14,18,3-39/3,52 0/5 14,18,3-39,52W ? JAN,MAR,SEP MON-WED,FRI#3 2002-2010"


Core

It starts with the tokenizer that has only one method with a loop that builds each token. It requires special handling of extensions like W, L or #.

```
class CronExpressionTokenizer
{
private static readonly Dictionary TokenTypes = new Dictionary
{
[' '] = TokenType.FieldSeparator,
[','] = TokenType.ListItemSeparator,
['-'] = TokenType.RangeSeparator,
['/'] = TokenType.StepSeparator,
['*'] = TokenType.Blank,
['?'] = TokenType.Blank,
['L'] = TokenType.Extension,
['W'] = TokenType.Extension,
['#'] = TokenType.Extension,
};

public static IEnumerable Tokenize(string text)
{
var position = (int?)null;
var value = new StringBuilder();
var lastTokenType = TokenType.None;

var updateLastTokenType = new Func(t => { lastTokenType = t.Type; return t; });

for (var i = 0; i < text.Length; i++)
{
var c = text[i];

var tokenType = TokenType.None;
if (TokenTypes.TryGetValue(c, out tokenType))
{
// Special extension handling.
var isNotExtension =
// Parsed as extension...
tokenType == TokenType.Extension &&
// but a "W" not after a value or field-separator.
(c == 'W' && (lastTokenType != TokenType.Value || lastTokenType == TokenType.FieldSeparator));

if (isNotExtension)
{
lastTokenType = TokenType.Value;
position = position ?? i;
value.Append(c);
contin

Solution

There's not a single access modifier on any type, did you mean for them all to be internal? If so, specify that explicitly.

var position = (int?)null;


Really? In cases like this (casting null) don't use var, just specify the type.

var updateLastTokenType = new Func(t => { lastTokenType = t.Type; return t; });


For your use case, this function is not helpful. You update lastTokenType manually as many times as you update it with this function, and one of the updates you do with this function is undone by a manual update. Just swallow the bullet and replace this with manual updates instead.

private static readonly IReadOnlyDictionary 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);


Is MAI spelled like that? Not MAY?

Also, with what this and the property above it do, I would consider wrapping them in a function call that will do that conversion, that's a lot of code to have in an initializer.

private static IEnumerable> GroupTokens(IEnumerable tokens)


You should be returning IEnumerable there, since you are returning objects of that type anyway.

public static CronRange SetMin(this CronRange? range, int min) => range.Value.SetMin(min);

public static CronRange SetMax(this CronRange? range, int max) => range.Value.SetMax(max);

public static CronRange SetStep(this CronRange? range, int step) => range.Value.SetStep(step);

public static CronRange SetExtension(this CronRange? range, CronExtension extension) => range.Value.SetExtension(extension);


What happens when I provide null here? Easy fix: range?.Value.Set...

Overall, excellent work here. It's always nice to see your questions as they tend to be very good code-wise to begin with, the worst I can find here is a few nitpicks. :)

Code Snippets

var position = (int?)null;
var updateLastTokenType = new Func<Token, Token>(t => { lastTokenType = t.Type; return t; });
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);
private static IEnumerable<IGrouping<Type, Token>> GroupTokens(IEnumerable<Token> tokens)
public static CronRange SetMin(this CronRange? range, int min) => range.Value.SetMin(min);

public static CronRange SetMax(this CronRange? range, int max) => range.Value.SetMax(max);

public static CronRange SetStep(this CronRange? range, int step) => range.Value.SetStep(step);

public static CronRange SetExtension(this CronRange? range, CronExtension extension) => range.Value.SetExtension(extension);

Context

StackExchange Code Review Q#147453, answer score: 4

Revisions (0)

No revisions yet.