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

More JewishCalendar Tefila Rules / Special Days #198

Closed
CompuGenius-Programs opened this issue Dec 1, 2022 · 14 comments
Closed

More JewishCalendar Tefila Rules / Special Days #198

CompuGenius-Programs opened this issue Dec 1, 2022 · 14 comments

Comments

@CompuGenius-Programs
Copy link

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
Copy link
Owner

KosherJava commented Dec 1, 2022

@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
Copy link
Author

CompuGenius-Programs commented Dec 1, 2022

@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
Copy link
Owner

@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
Copy link
Author

@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 Change README gradle import syntax #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
Copy link
Owner

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
Copy link
Author

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

@CompuGenius-Programs
Copy link
Author

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

Closing this now, thanks!

@KosherJava
Copy link
Owner

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

@CompuGenius-Programs
Copy link
Author

CompuGenius-Programs commented Dec 1, 2022

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
Copy link
Author

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
Copy link
Owner

@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
Copy link
Author

CompuGenius-Programs commented Dec 1, 2022

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 added a commit that referenced this issue Dec 5, 2022
First part of #198
Fixes a bug that did not consider Hoshana Rabba as part of Chol Hamoed Succos.
@KosherJava
Copy link
Owner

@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
Copy link
Author

LGTM 😁

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

No branches or pull requests

2 participants