Separate database-related code from ServerLobby #5124
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I also attempted to reuse some code that checks if a peer is banned
easySQLQuery was generalized to return output rows if requested, which allowed to shorten the code for many queries. The code for binding values to sqlite statement was also repeated many times. Two auxiliary structures were introduced, so that it's possible to provide at the same time both those parameters which require and those which don't require binding, in a single StringUtils::insertValues() call.
|
I trust you. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here, I split the database-related code previously located in ServerLobby implementation into logic part, which stayed in ServerLobby, and database part (forming queries, all types of sqlite3_* functions), which moved to a new DatabaseConnector class. Apart from that, I reused some code previously repeated while making queries of same type and while binding values (by adding two auxiliary structures), and added some doxygen comments.
I felt like making a separate class for that purpose would do better than not making it, as there a few more variables to keep track of than just database handle, and passing database handle to each of many functions would probably be too long in terms of code.
The amount of changes is pretty big, so I suppose that despite I did some testing, it requires even more of it + I could miss something, feel free to ask me anything about it.
Agreement