patterncsharpMinor
Parsing cron expression
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).
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
```
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
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
Really? In cases like this (casting
For your use case, this function is not helpful. You update
Is
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.
You should be returning
What happens when I provide
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. :)
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.