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

Prettify ToString return value #10

Open
AlexanderButs opened this issue May 26, 2016 · 5 comments
Open

Prettify ToString return value #10

AlexanderButs opened this issue May 26, 2016 · 5 comments

Comments

@AlexanderButs
Copy link

Hi.

Thanks for great library and Your work.

Just want to discuss some minor issue with ToString implementation. If I parse '*/2 * * * *' schedule value and then want to make a string from this object via ToString I'll get '0,2,4,6,8,10,12,14,16,18,20,22,24,26,28,30,32,34,36,38,40,42,44,46,48,50,52,54,56,58 * * * *' value. Which is a little bit annoying. Can You fix this or I can make a pull request for this minor issue. Just need to add original expression value to ctor, set it to the internal class member and then return from ToString method.
What do You think about that? Thanks in advance.

@atifaziz
Copy link
Owner

@AlexanderButs I think it's a reasonable request/expectation though to keep things KISS for the library, the responsibility of maintaining the originally parsed expression was left up to the user. For example, why not introduce a simple type in your code along the lines of the following?

partial class ParsedCrontabSchedule
{
    public string Expression { get; }
    public CrontabSchedule Schedule { get; }

    ParsedCrontabSchedule(string expression, CrontabSchedule schedule)
    {
        Expression = expression;
        Schedule = schedule;
    }

    public static ParsedCrontabSchedule Parse(string expression) =>
        new ParsedCrontabSchedule(expression, CrontabSchedule.Parse(expression));

    public override string ToString() => Expression;
}

It's simple to introduce and gives you more control, don't you think?

@AlexanderButs
Copy link
Author

Hi, again.

Off course I can do this. I can do any thing inside my own code. But when I parse "2" value I'll get 2 and when call ToString I'll get not "1+1" and not "3-1", I'll get exactly "2". So I expect the same behavior from any library I use. Does it make sense? May I expect such behavior from NCrontab library?

@atifaziz
Copy link
Owner

atifaziz commented Jun 1, 2016

May I expect such behavior from NCrontab library?

As I said, it's a reasonable expectation so let's go ahead with it but…

I can make a pull request for this minor issue

…if you can make a PR (with unit tests, please) that I can review and merge then things would go quicker. If it's up to me alone then I'll eventually get round to it but I'm busy on other projects at the moment so this would be a low priority considering that a reasonable workaround exists that doesn't require much effort on the user of the library.

@AlexanderButs
Copy link
Author

Ok, deal.
I've made a fix in my own code as you suggested earlier.
About PR: If I find a time slot for this activity I'll do it for sure. With unit test of course :)
Thanks for Your collaboration.

@AlexanderButs
Copy link
Author

Please, review pull request 26ffc8b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants