KosherJava / zmanim

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

More `JewishCalendar` Tefila Rules / Special Days #198

Closed CompuGenius-Programs closed 1 year ago

CompuGenius-Programs commented 1 year ago
public boolean isTishaBav() {
    int holidayIndex = getYomTovIndex();
    return holidayIndex == TISHA_BEAV;
}

public boolean isRoshChodesh() {
    int holidayIndex = getYomTovIndex();
    return holidayIndex == ROSH_CHODESH;
}

public boolean isYaalehVyavo() {
     return isRoshChodesh() || isCholHamoedPesach() || isCholHamoedSuccos();
}

public boolean isPurim() {
    int holidayIndex = getYomTovIndex();
    return holidayIndex == PURIM;
}

public boolean isAlHanissim() {
    return isPurim() || isChanukah();
}

All the above functions will work except isRoshChodesh(). For some reason, the const ROSH_CHODESH does not seem to be used. Why is that?

KosherJava commented 1 year ago

@CompuGenius Thanks for the suggestions.

  1. isTishaBav() is overkill since we do not have an isTzomGedalia(), isShivaasarBetamuz(), IsTaanisEsther etc. We do have isTaanis(). If you can make a good case for it, I will reconsider.
  2. JewishCalendar.isRoshChodesh() exists
  3. IsPurim() needs adjustments and setting such as isMukafChoma() for this to work for Yerushalayim and other cities.
  4. isYaalehVyavo() should return true for first and last days of Sukkos as well as Shavuos.
CompuGenius-Programs commented 1 year ago

@KosherJava thanks for your prompt reply.

  1. Regarding isTishaBav(), I created it for the Nacheim prayer by mincha.
  2. I have version 2.4.0 implemented in gradle and there is no isRoshChodesh() function - only a const named ROSH_CHODESH that is never used (from what I can tell).
  3. That is a good point - forgot. How exactly would I go about validating which cities keep SHUSHAN_PURIM or not?
  4. I specifically did not implement the days of Sukkos or Shavuos because, although I considered that there may be use cases for it, in my circumstance - a dynamic weekday siddur - it is not necessary, C"V.
KosherJava commented 1 year ago

@CompuGenius ,

  1. Understood the need
  2. See isRoshChodesh(). I am not sure how it is missing in your version. Indeed the constant is not in use. I will review it.
  3. No way to validate. It is up to the developer to have a setting for it. I already coded it but it is not in GitHub yet. One thing I am not sure of is how to deal with places that have a safek. It is probably too much to deal with at this point, and the implementer can address it without the API.
  4. It would make sense for many non eSiddurim. An example would be a large shul display showing what part of davening is recited such as Parsha, Mashiv Haruach and Yaaleh Veyavo makes sense.
CompuGenius-Programs commented 1 year ago

@KosherJava,

  1. Obviously up to you, just think it would be helpful.
  2. That it indeed very strange. I just removed the gradle command as listed in the README and instead put what is suggested in #197, which caused gradle to rebuild due to different code, and now it is there. Strange indeed.
  3. Yeah, figured as much. Thanks
  4. Yeah, i figured these as use cases, I just did not bother implementing it myself. Since I knew this one would possibly need to be changed, I created an issue instead of bothering with a PR.
KosherJava commented 1 year ago

As far as the constant ROSH_CHODESH, the main use of these constants are in getYomTovIndex(). That returns an int I am not sure that it is worth an effort in returning. I may change my mind down the line.

CompuGenius-Programs commented 1 year ago

@KosherJava I understand the usage of the constants, but ROSH_CHODESH is never used. Just wanted to point it out.

CompuGenius-Programs commented 1 year ago

Alright, the only dangling suggestion is isTishaBav() for Nacheim, up to you to add.

Closing this now, thanks!

KosherJava commented 1 year ago

I would have left it open until I actually addressed the items that can be addressed.

CompuGenius-Programs commented 1 year ago

Oh, I had figured that you had decided to not implement isTishaBav().

I also just realized I had forgotten about isAlHanissim() and isYaalehVyavo() lol.

CompuGenius-Programs commented 1 year ago
public boolean isPesach() {
    int holidayIndex = getYomTovIndex();
    return holidayIndex == PESACH;
}

public boolean isSuccos() {
    int holidayIndex = getYomTovIndex();
    return holidayIndex == SUCCOS || holidayIndex == HOSHANA_RABBA || holidayIndex == SHEMINI_ATZERES || holidayIndex == SIMCHAS_TORAH;
}

public boolean isShavuos() {
    int holidayIndex = getYomTovIndex();
    return holidayIndex == SHAVUOS;
}

public boolean isPesachHoliday() {
    return isPesach() || isCholHamoedPesach();
}

public boolean isSuccosHoliday() {
    return isSuccos() || isCholHamoedSuccos();
}

public boolean isYaalehVyavo() {
    return isRoshChodesh() || isPesachHoliday() || isSuccosHoliday() || isShavuos();
}

Here's a proper isYaalehVyavo() and helper functions related. I made a separate one for isPesach() or isSuccos() to match isShavuos() and in case there ever is a want to just get those days.

KosherJava commented 1 year ago

@CompuGenius , isCholHamoedPesach() and isCholHamoedSuccos() already exist. I will add isPesach() etc that will cover the entire YT and you can just call isCholHamoedPesach() or isCholHamoed() to determine things. Too many methods will confuse people. As far as Yaaleh Veyavo, you missed RH, YK, Shmini Atzers and Simchas Torah. I will deal with it.

CompuGenius-Programs commented 1 year ago

isCholHamoedPesach() and isCholHamoedSuccos() already exist

I know that. I did not create those functions, rather just called them from my own.

I extracted each boolean to it's own function, which is definitely overboard, but you never know what people need 🤷

As far as Yaaleh Veyavo, you missed RH, YK, Shmini Atzers and Simchas Torah. I will deal with it.

I missed Rosh Hashanah and Yom Kippur, but if you look at my isSuccos() function, it includes Shmini Atzeres and Simchas Torah.

KosherJava commented 1 year ago

@CompuGenius , as far as I can tell, this is all done with the recent commits. Please let me know if anything was missed.

CompuGenius-Programs commented 1 year ago

LGTM 😁