KosherJava / zmanim

KosherJava Zmanim API / Library
https://kosherjava.com
GNU Lesser General Public License v2.1
106 stars 48 forks source link

Begin JewishCalendar day At Sunset or Nightfall #139

Open ghost opened 5 years ago

ghost commented 5 years ago

Being a Jewish calendar I would expect it to begin at nightfall. However, from looking at the code it seems that it always uses the regular Calendar date, which begins at midnight. This can result in incorrect results.

Consider the following:

JewishCalendar jcal = new JewishCalendar();
if (jcal.isAssurBemelacha())
    System.out.print("Please don't use on Shabbat or Jewish Holidays");

This would consider even after nightfall on Saturday as if it is Shabbat (and therefore isAssurBemelacha() would evaluate to true), while it should not consider it to be Shabbat (and isAssurBemelacha() should evaluate to false).

zachweix commented 5 years ago

You are indeed correct that it should be based on the user's location, but you can use the function at https://github.com/KosherJava/zmanim/blob/master/src/net/sourceforge/zmanim/ZmanimCalendar.java#L672 to get what you are looking for

KosherJava commented 5 years ago

@plonibarploni The current design of the library has no concept of time. Adding it would seriously complicate things. As @zachweix pointed out, there is support for what you are looking for in the Zmanim portion of the API that is very time aware. While I will leave this open, I do not see a resolution to this item in the near future.

ghost commented 5 years ago

@KosherJava While JewishDate states very prominently that it has no concept of time, JewishCalendar does not. Regardless, it seems to be very misleading to include this function isAssurBemelacha() in a class that has no concept of time, being as it very specifically requires time to be taken into consideration. At the very least, a note should be added to the Javadoc of the function to state that it doesn't factor in the time.

CompuGenius-Programs commented 1 year ago

I created a JewishDate() constructor that takes sunset into account, and the dev replied with some ideas of how to properly implement it. See #193