Migrate to nullsafety

[Anas35]

issue: #14

I made a slight change, In the event of invalid coordinates, we used to return null instead, I changed it to throw ArugumentError. The reason for this is every time the User called prayer time, they have to check for null.

If you don't like that, please let me know I will change it back.

PS: will migrate Example projects to null safety after this PR merged.

[iamriajul]

We already threw Invalid Coordinates Exception If coordinates are invalid.

[iamriajul]

Currently, I have two solutions in mind:

1. Force Coordinates class validation. So Coordinates don't become a reason for PrayerTimes initialization to fail.
2. Throw exception on PrayerTimes initialization fails. So the user doesn't reach the line prayerTimes.fajr

[Anas35]

I have some idea which can make PrayerTime non-nullable. https://github.com/iamriajul/adhan-dart/blob/758a71fcb0263afd12cbcd825f0f3431add0e409/lib/src/data/time_components.dart#L12 throwing ArugumentError instead of null here might make it non-nullable. I haven't tested it yet.

[iamriajul]

I have some idea which can make PrayerTime non-nullable.

https://github.com/iamriajul/adhan-dart/blob/758a71fcb0263afd12cbcd825f0f3431add0e409/lib/src/data/time_components.dart#L12

throwing ArugumentError instead of null here might make it non-nullable. I haven't tested it yet.

I agree, by throwing exceptions/errors on an individual part, the user will have easy debugging.

[Anas35]

@iamriajul can u take a look at this?

[iamriajul]

@iamriajul can u take a look at this?

test_coverage library is failing CI tests, looking for alternative test coverage library, do you have information on that?

[Anas35]

Is the test failing due to the test code is in with both null-safety and non-null-safety code?

[iamriajul]

I think it's because, the test_coverage library generated file does not have the opt-out @dart 2.9 annotation.

[codecov[bot]]

Codecov Report

Merging #16 (4d73c97) into master (758a71f) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            master       #16    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           22         9    -13     
  Lines          568       348   -220     
==========================================
- Hits           568       348   -220     

Impacted Files	Coverage Δ
test/src/calculation_method_test.dart	100.00% <ø> (ø)
test/src/calculation_parameters_test.dart	100.00% <ø> (ø)
test/src/qibla_test.dart	100.00% <ø> (ø)
test/src/madhab_test.dart	100.00% <100.00%> (ø)
test/src/prayer_times_test.dart	100.00% <100.00%> (ø)
lib/src/internal/qibla_util.dart
lib/src/coordinates.dart
lib/src/internal/shadow_length.dart
lib/src/calculation_method.dart
lib/src/prayer.dart
... and 17 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 758a71f...4d73c97. Read the comment docs.

[iamriajul]

I think the Madhab should be non-nullable.

On Sat, Mar 13, 2021 at 12:13 PM Anas35 @.***> wrote:

@Anas35 commented on this pull request.

In test/src/madhab_test.dart https://github.com/iamriajul/adhan-dart/pull/16#discussion_r593643925:

@@ -10,7 +10,7 @@ void main() { expect(hanafiMadhab1 == hanafiMadhab2, isTrue); expect(hanafiMadhab1 == shafiMadhab1, isFalse);

• Madhab madhab;
• Madhab? madhab; expect(() => madhab.getShadowLength(), throwsFormatException);

We either can make this Non-nullable and remove the format exception and add one of the Madhab as a default value or leave it as nullable.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/iamriajul/adhan-dart/pull/16#discussion_r593643925, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFY7BG7ZNVDYDPBIMNDRNPLTDL67BANCNFSM4YXMCSKA .

-- Riajul Islam Senior Applications Developer More Info: riajul.dev https://riajul.dev

[Anas35]

If that the case what should be the default value?

[iamriajul]

madhab is non-nullable in CalculationParameters. And I some opinions on CalculationParameters:

• We should not set default madhab but rather require the user to pass a madhab.
• We should not set default method but rather require the user to pass a method.

[Anas35]

If we remove the default value in CalculationParameter, getParameter in CalculationMethodExtensions throw error As none of the mahab have been assigned

[iamriajul]

We can accept madhab on getParameter method! should we do this? whats you thought on this?

[Anas35]

If we do that, It doesn't change much... it just an extra code for us and the user as well.

[iamriajul]

I'm confused 😵, If we should that or not.

[Anas35]

We shouldn't, It will just result in more code

[iamriajul]

Yes

On Sat, Mar 13, 2021 at 1:52 PM Anas35 @.***> wrote:

@Anas35 commented on this pull request.

In CHANGELOG.md https://github.com/iamriajul/adhan-dart/pull/16#discussion_r593716864:

@@ -1,3 +1,8 @@ +## 2.0.0-nullsafety.0

Are you planning to publish this package with null safety as beta?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/iamriajul/adhan-dart/pull/16#pullrequestreview-611577202, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFY7BGYSMHMVC2YPLNVQEM3TDMKT5ANCNFSM4YXMCSKA .

-- Riajul Islam Senior Applications Developer More Info: riajul.dev https://riajul.dev

[iamriajul]

Madhab is required for PrayerTime calculation.

On Mon, Mar 15, 2021 at 2:25 PM Anas35 @.***> wrote:

@Anas35 commented on this pull request.

In test/src/madhab_test.dart https://github.com/iamriajul/adhan-dart/pull/16#discussion_r594131022:

@@ -10,7 +10,7 @@ void main() { expect(hanafiMadhab1 == hanafiMadhab2, isTrue); expect(hanafiMadhab1 == shafiMadhab1, isFalse);

• Madhab madhab;
• Madhab? madhab; expect(() => madhab.getShadowLength(), throwsFormatException);

@iamriajul https://github.com/iamriajul what's the plan for this? If we change it non-nullable, we need to assign a default value and remove the Exception.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/iamriajul/adhan-dart/pull/16#discussion_r594131022, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFY7BG3KX2FHCEIXHJGRSI3TDXAATANCNFSM4YXMCSKA .

-- Riajul Islam Senior Applications Developer More Info: riajul.dev https://riajul.dev

[Anas35]

Is there anything left now?

[iamriajul]

I think we should enforce Madhab passing with getParameters method, https://github.com/iamriajul/adhan-dart/issues/17#issue-833526025

[Anas35]

I don't understand... can you elaborate more?

[iamriajul]

The person who raised the issue was from Karachi/Pakistan (as his issue title indicate) mostly Karachi/Pakistani people follow Hanafi Madhab but our app default to Shafi Madhab, I'm suspecting which is why he had Asr time issue as Asr times differ very by Shafi and Hanafi Madhab.

[Anas35]

I don't think that's the case. I looked into his repo and found out, he is setting his madhab to Hanafi https://github.com/muhammadsaad-ak/prayer_location_finder_app/blob/d8edb07151c74170e5ccd5153ac69dafe8608cb1/lib/homescreen.dart#L302

If he is using that project :)

[Anas35]

Without his more info... we don't know what might his issue

[iamriajul]

Yes

On Tue, Mar 23, 2021 at 9:06 AM Anas35 @.***> wrote:

Without his more info... we don't know what might his issue

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/iamriajul/adhan-dart/pull/16#issuecomment-804547955, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFY7BG2SJEP4PTORHDKKGK3TFAAUJANCNFSM4YXMCSKA .

-- Riajul Islam Senior Applications Developer More Info: riajul.dev https://riajul.dev

[Anas35]

I think we can merge this. Regarding that issue, if we get the response we can look into it separately.

[iamriajul]

Published as prerelease: https://pub.dev/packages/adhan/versions/2.0.0-nullsafety.0 Try it out in your projects. and give feedback.

[musaffa]

@iamriajul When will the version 2.0.0 be released?