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

Move the searching from Traits into Class based to allow for the dropping of the all() method and search functionality #10

Merged
merged 20 commits into from Oct 31, 2022

Conversation

RoadSigns
Copy link
Contributor

@RoadSigns RoadSigns commented Oct 29, 2022

Pull Request Description

Change to how we are structing the data

This Pull Request has changed the application from using Traits and using Classes, this allows us to create a tree like structure so when it comes to searching for specific locations we can go down the branches of the tree to location it as long it's lower down the tree.
Tree Data Structure

Reading of the JSON files

When it comes to getting the information this is done when creating the Uganda object for the first time and we store the information in memory. This allows us to access the information quickly since we won't be required to do multiple reads every time we want to access the information.

Since we aren't changing the state of the tree structure we are just creating copies of branches that we want. We only need to read the file when we create the object for the first time and we won't have to worry about the information since the Objects are immutable.

Testing

Since now we have Classes instead of Traits we can now test each section without the need to call the JSON data since they are just newly created objects with fake data.

PHPStan

Previously we was unable to go above PHPStan Level 1 when it came to static analyzing with the new changes we can now support Level 9.
PHPStan Level

The Ability to get specific types of location data from any level

When doing the refactor I saw the issue of #4 and made it possible to access all of the information for each branch of the data.

So for the issue ticket we are now able to complete this by doing the following $uganda->district('Bugweri')->subCounties();
This will return you an array of SubCounty objects that are from the District object of Bugweri.

The dropping of the all()

Previously all methods had to end with all() to get the information. Using this new approach we are getting the information from methods that are self descriptive.

Type (Feature, Fix, Config, etc)

  • Bug fix 🐞
  • New feature ✨
    • The ability to access all Geographical information for every specific type of Geographical location
  • Breaking change πŸ”©
    • Removal of the Geo class and now called Uganda
  • This change requires a documentation update πŸ“™
    • All of the new ways of accessing the information has been added to the README.md
  • Improvement 🌠
  • Refactor πŸ”„
  • Add Test Specs 🚦
    • All new tests have been added to cover the new classes

Tests

Tests have been written to match the new flow of the package

Tests configurations and running:

  • PHP Unit
vendor/bin/phpunit

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@RoadSigns RoadSigns mentioned this pull request Oct 29, 2022
@RoadSigns
Copy link
Contributor Author

@kusaasira Bit of a big Pull Request but if you have any questions please get in touch!

@kusaasira
Copy link
Owner

This is great @RoadSigns . You keep "deeply" refactoring on each PR! This is more than a PR though πŸ˜…. Great work there.

Copy link
Owner

@kusaasira kusaasira left a comment

Choose a reason for hiding this comment

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

Great work @RoadSigns . I see the positive impact of PHPStan though.

@kusaasira kusaasira merged commit 3dec45c into kusaasira:main Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants