alonsodomin / cron4s

Cross-platform CRON expression parsing for Scala
https://alonsodomin.github.io/cron4s
Apache License 2.0
120 stars 24 forks source link

README not up to date #15

Closed ouraios closed 7 years ago

ouraios commented 7 years ago

Hi, It seems like your example in your README are not up to date, so i cant see how to compare a cron string 0 12 " with localtime :/ Can you show a complete example of that ? Thanks in advance

alonsodomin commented 7 years ago

I would really make a better documentation effort than just the README but that would require a bit of time. Minding pointing out which is the bit that you think is out of date?

ouraios commented 7 years ago

Well you give this example :

val parsed = Cron("10-35 2,4,6 * * *")

but it seems like now it's more like :

val parsed = Cron.apply("10-35 2,4,6 * * *").right.get

Then this return a CronExpr so according to your example in your Readme i should be able to do that :

val cron : CronExpr = Cron.apply("10-35 2,4,6 * * *").right.get
val now = LocalDateTime.now
cron.allOf(now)

But the probleme is that the CronExpr object doesn't have the allOf method, so i cant verify if my cron expr match the actual datetime.

EDIT : I'm using the 0.2.0 version from maven for Scala.

alonsodomin commented 7 years ago

Ok, I get it now.

Cron("10-35 2,4,6 * * *") and Cron.apply("10-35 2,4,6 * * *") are equivalent in Scala so you should get the same result with either of them.

About the right.get... well, probably I should have been more explicit. The README itself assumes that the developer has some sort of familiarity working with the Either type and how to extract values out of it.

For me, my preferred way of doing is:

val now = LocalDateTime.now
val parsedCron = Cron("10-35 2,4,6 * * *")
val matchedResult = parsedCron.right.map(cron => cron.allOf(now))

which results in a type Either[cron4s.ParseError, Boolean]. Either in Scala 2.12 will be right biased and provide with first class map and flatMap methods, so the previous becomes Cron("10-35 2,4,6 * * *").map(cron => cron.allOf(now)) or Cron("10-35 2,4,6 * * *").map(_.allOf(now)) for short.

Another, more verbose, way of dealing with it is the following:

val parsedCron = Cron("10-35 2,4,6 * * *")
parsedCron match {
  case Left(error) => // deal with the error case
  case Right(cron) =>
    val now = LocalDateTime.now
    cron.allOf(now)
}

In the other hand, other people using Scalaz or Cats like to transform the Either to the enhanced types provided by those libraries... so there are different ways of dealing with it. The README ignores that and focuses on what can be done with the CronExpr type, which is the most interesting one.

Since I started already working in a better version of the documentation, I will include a bit more of detail there if that's ok.

P.S.: I would not recommend using .right.get in the Either type. If for some reason there has been a parse error initially, doing .right.get will throw a very generic exception, which is probably what you do not want...

ouraios commented 7 years ago

Okay i was using

var cron : Cron("10-35 2,4,6") instead of : var cron = Cron("10-35 2,4,6 * * *")

It's working better now thanks ! But only working better for the Cron object.

When i call allOf i still get an error : value allOf is not a member of cron4s.expr.CronExpr

With this code :

val now = LocalDateTime.now
val parsedCron = Cron("10-35 2,4,6 * * *")

      if (parsedCron.isRight) {
        if (parsedCron.right.get.allOf(now)) {
          log.info("Working");
        }
      } else {
         log.info("ParsedError !");
      }

Even in your CronExpr file i dont see your allOf method, maybe i mistaken somewhere but i think this is something from your code, if you can help that would be awesome ! Thanks for your already help ;)

alonsodomin commented 7 years ago

@ouraios you are getting the error because your are missing a couple of important imports. If you look at the current readme, in the section "Matching against time" you'll see it mentions these imports:

import java.time._
import cron4s.japi.time._

The missing methods will be added by the compiler after you import that

ouraios commented 7 years ago

Thanks it works like a charm !

The only weird thing is that IntelliJ tell me that the type mismatch on allOf method, because the method wait for Nothing and got LocalDateTime.

Anyway when i'm compiling my app it works great !

Thanks for your amazing job on this library and for your support ! :+1:

alonsodomin commented 7 years ago

yeah, IntelliJ has a bit of a hard time aligning the types. I use Ensime most of time, it tends to give less false negatives.

Thanks for finding the library useful, I'll leave this issue open until the new documentation is ready and live. In the meantime, feel free to create new issues if you find other problems with the lib, at the moment it's still a bit experimental so I would be very interested on knowing about any potential unexpected behaviour.

alonsodomin commented 7 years ago

A docs module has been added to the repo containing the new version of the documentation. This issue will be closed once 0.2.1 gets released and the new site published.

alonsodomin commented 7 years ago

Website published at https://alonsodomin.github.io/cron4s, version 0.2.1 will be soon available at Maven Central.