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

Simplify code #153

Merged

Conversation

BehindTheMath
Copy link
Contributor

@BehindTheMath BehindTheMath commented Apr 19, 2020

  • Simplify code
  • Set masechta names to final, since they're private and there's no mechanism to change them.

@BehindTheMath BehindTheMath changed the title Simplify JewishDate.compareTo() Simplify code Apr 19, 2020
@@ -25,13 +25,13 @@
private int masechtaNumber;
private int daf;

private static String[] masechtosBavliTransliterated = { "Berachos", "Shabbos", "Eruvin", "Pesachim", "Shekalim",
private static final String[] masechtosBavliTransliterated = { "Berachos", "Shabbos", "Eruvin", "Pesachim", "Shekalim",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BehindTheMath, I have a ToDo (sadly not in the code) to allow users to set different names for transliterated (such as Sephardi transliteration. While I do not yet have the set() methods, I will add them. Do you see any need for anyone to ever change the Hebrew ones? I can't see a reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you see any need for anyone to ever change the Hebrew ones? I can't see a reason.

I don't either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want me to remove this change? As the code stands now, there's no point in having them private and not final without setters.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BehindTheMath ,
Can you please rebase this against the latest master? I do ask that you remove the final on the masechtosBavliTransliterated and masechtosYerushalmiTransliterated ? Since I plan to add a setter/getter, I want to avoid adding a final just to revoke it a drop later.
Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@BehindTheMath BehindTheMath force-pushed the fix/simplify-jewish-date-compareto branch from 8e14fc8 to 4b242a7 Compare October 13, 2020 01:24
This is in preparation for adding getters and setters to allow
overriding the default transliteration.

This partially reverts 4b242a7.
Copy link
Owner

@KosherJava KosherJava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @BehindTheMath

@KosherJava KosherJava merged commit 810b79a into KosherJava:master Oct 13, 2020
@BehindTheMath BehindTheMath deleted the fix/simplify-jewish-date-compareto branch October 14, 2020 01:25
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

Successfully merging this pull request may close these issues.

2 participants