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

Add a file for each PDO class #3301

Merged
merged 1 commit into from Dec 28, 2020
Merged

Conversation

aledeg
Copy link
Member

@aledeg aledeg commented Dec 28, 2020

Changes proposed in this pull request:

  • split Minz pdo file to have a class per file

How to test the feature manually:

  1. Use the application, as the modification changes the database access, you'll notice right away if it's not working.

Pull request checklist:

  • clear commit messages
  • code manually tested
  • unit tests written (optional if too hard)
  • documentation updated

Additional information can be found in the documentation.

Before, we had 5 classes in the ModelPdo file. It was bad for 2 reasons.
The first reason is that it is considered bad practice to have multiple
class in one file. This is especially true when using autoloading. On top
of that it is less readable considering the size of the file. The second
reason is that so far we were lucky. Everytime we needed to access the
database, it was through the ModelPdo class which loads all the other
classes. If we want to access directly the connection, it wont be loaded.
On top of that, the system is configured to work on a single database,
but as we have every connection definition in a single file, all classes
were loaded at the same time. Thus using memory and processing time for
nothing.
Now, we have a file for each class. To work with autoloading, classes
were slightly renamed to match autoloading rules.

Before, we had 5 classes in the ModelPdo file. It was bad for 2 reasons.
The first reason is that it is considered bad practice to have multiple
class in one file. This is especially true when using autoloading. On top
of that it is less readable considering the size of the file. The second
reason is that so far we were lucky. Everytime we needed to access the
database, it was through the ModelPdo class which loads all the other
classes. If we want to access directly the connection, it wont be loaded.
On top of that, the system is configured to work on a single database,
but as we have every connection definition in a single file, all classes
were loaded at the same time. Thus using memory and processing time for
nothing.
Now, we have a file for each class. To work with autoloading, classes
were slightly renamed to match autoloading rules.
@aledeg aledeg added this to the 1.18.0 milestone Dec 28, 2020
@Alkarex Alkarex merged commit 3f4c86f into FreshRSS:master Dec 28, 2020
@aledeg aledeg deleted the enhance/pdo-objects branch December 28, 2020 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants