smartystreets / smartystreets-java-sdk

The official client libraries for accessing SmartyStreets APIs from Java (and JRE-based languages)
https://smartystreets.com/docs/sdk/java
14 stars 18 forks source link

refactor: change type ArrayList for fields and input parameters to List #23

Closed eduard-romaniuk closed 3 years ago

eduard-romaniuk commented 3 years ago

Hey guys!

Decided to propose you such a change because it will more convenient way of using lists and will easier life for library users (I hope). Let me know if you are interested in code quality improvement PRs because I have few more improvements in my mind.

eduard-romaniuk commented 3 years ago

@MouaYing @DuncanBeutler any response?

DuncanBeutler commented 3 years ago

We are reviewing the changes. The ArrayList was used in previous iterations to leverage its methods but current implementations may have rendered it obsolete. We just need to confirm that the change is sound. Thank you for your contribution!

eduard-romaniuk commented 3 years ago

@DuncanBeutler Oh, sorry for my impatience. Thank you too!

DuncanBeutler commented 3 years ago

@eduard-romanyuk Looks like there are a couple of failing tests, specifically in the LicenseSenderTest file. Could you make sure these tests are passing? Thanks!

eduard-romaniuk commented 3 years ago

Tests passed successfully on my computer 😅

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.smartystreets.api.CustomHeaderSenderTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.035 sec
Running com.smartystreets.api.GoogleSenderTest
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.036 sec
Running com.smartystreets.api.GoogleSerializerTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.031 sec
Running com.smartystreets.api.international_street.CandidateTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.006 sec
Running com.smartystreets.api.international_street.ClientTest
Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.01 sec
Running com.smartystreets.api.LicenseSenderTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running com.smartystreets.api.RequestTest
Tests run: 11, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec
Running com.smartystreets.api.RetrySenderTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 sec
Running com.smartystreets.api.SharedCredentialsTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running com.smartystreets.api.SigningSenderTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running com.smartystreets.api.StaticCredentialsTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running com.smartystreets.api.StatusCodeSenderTest
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.004 sec
Running com.smartystreets.api.URLPrefixSenderTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec
Running com.smartystreets.api.us_autocomplete.ClientTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 sec
Running com.smartystreets.api.us_autocomplete.SuggestionTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec
Running com.smartystreets.api.us_autocomplete_pro.ClientTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 sec
Running com.smartystreets.api.us_autocomplete_pro.SuggestionTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec
Running com.smartystreets.api.us_extract.ClientTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 sec
Running com.smartystreets.api.us_extract.ResultTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.003 sec
Running com.smartystreets.api.us_reverse_geo.ClientTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.008 sec
Running com.smartystreets.api.us_street.BatchTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec
Running com.smartystreets.api.us_street.CandidateTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.003 sec
Running com.smartystreets.api.us_street.ClientTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 sec
Running com.smartystreets.api.us_zipcode.BatchTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec
Running com.smartystreets.api.us_zipcode.ClientTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 sec
Running com.smartystreets.api.us_zipcode.ResultTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec

Results :

Tests run: 94, Failures: 0, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  2.290 s
[INFO] Finished at: 2021-09-09T23:28:00+03:00
[INFO] ------------------------------------------------------------------------
eduard-romaniuk commented 3 years ago

@DuncanBeutler Can you provide more information? Running environment, maybe logs from failed tests, etc. I tried to run tests on 8 and 7 versions of Java just in case but have the same result.

DuncanBeutler commented 3 years ago

@eduard-romanyuk Apologies, it was a dependency on my end that looked like a missing import. The changes look good.

DuncanBeutler commented 3 years ago

Thank you for your contributions. They will be reflected in the next release.