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

[Refactor] Extract DB access logic from functionalities and views #378

Closed
2 tasks done
damien-rivet opened this issue Oct 31, 2023 · 3 comments
Closed
2 tasks done
Assignees
Labels
feature New feature or request help wanted Extra attention is needed refactor Refactor code to improve quality

Comments

@damien-rivet
Copy link
Contributor

Terms

Description

DB access is done through a app level queryDBRow function located in the LoadData.swift file.

The whole logic to load and access the local databases should be moved to a dedicated service that exposes only the required functions with a set of very limited parameters. This refactor could also be used to add unit tests for those accesses.

At the App level, there is also no error management regarding this database logic, unless the App is plugged to Xcode there is no way to know if something has gone wrong.

Contribution

No response

@damien-rivet damien-rivet added the feature New feature or request label Oct 31, 2023
@andrewtavis andrewtavis added help wanted Extra attention is needed refactor Refactor code to improve quality labels Oct 31, 2023
@flumaves
Copy link
Contributor

flumaves commented Nov 2, 2023

There are some exposed methods in loadData.swift. May need a class (could consider whether it is a singleton) or structure similar to DataManager to organize these methods, contact the database and provide API for external use. At the same time, loadData.swift must be changed to a more suitable name.

@andrewtavis
Copy link
Member

Hey @flumaves! Your profile image fits in well here 😅😊 Suggestions sound really good! Let me know if you think a singleton would be better or not. I'm happy to have you work on this! Please let me know if there's anything I can do to support :)

@andrewtavis
Copy link
Member

Closed by #381 and 3ffa3da to clean up the implementation :) Again, @flumaves, really sorry for the long and messy merge process for the PR. There were a couple of minor problems with the conversions that I was able to figure out. Specifically changing over the word that's in any given query to lowercase by default doesn't always work for proper nouns or specifically German nouns that are always capitalized. There were also a couple of cases where verbToConjugate wasn't used when it should have been.

Overall though everything is much much nicer now 😊 Really appreciate the care you put into all this. Such an improvement to how we access the language databases! Thanks again for your contribution 🙏

@github-project-automation github-project-automation bot moved this from In Progress to Done in Scribe Board Apr 3, 2024
SaurabhJamadagni pushed a commit to marekviktor/Scribe-iOS that referenced this issue Apr 18, 2024
SaurabhJamadagni pushed a commit to marekviktor/Scribe-iOS that referenced this issue Apr 18, 2024
SaurabhJamadagni pushed a commit to marekviktor/Scribe-iOS that referenced this issue Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request help wanted Extra attention is needed refactor Refactor code to improve quality
Projects
Archived in project
Development

No branches or pull requests

3 participants