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

Removing circular references #3896

Closed
ichorid opened this issue Sep 21, 2018 · 1 comment
Closed

Removing circular references #3896

ichorid opened this issue Sep 21, 2018 · 1 comment

Comments

@ichorid
Copy link
Contributor

ichorid commented Sep 21, 2018

Circular references in objects structure is bad practice for many reasons. The worst result of this practice is a God-object anti-pattern. We actually have a whole Trinity of God-objects in LaunchManyCore, Session, and DownloadManager.

These anti-patterns significantly complicate mocking, reduce testing efficiency, and prevent us from achieving a truly modular design, that is the first step to the microservice architecture.

In general, functions/methods should never extract information from its arguments. For example, we want the class __init__ arguments list to represent the minimal set of things that this class requires to be instantiated and do its thing.

In short, we must refactor code like this:

class SomeModule():
    def __init__(self, session):
        self.session = session
   
    def do_something(self): 
        print(self.session.some_property)
        print(self.session.another_property)

to

class SomeModule():
    def __init__(self, property, another_property):
        self.property = property
        self.another_property = another_property
   
    def do_something(self): 
        print(self.some_property)
        print(self.another_property)

We are already going to refactor the libtorrent wrapper, so this is a suitable moment to add this rule of no parent referencing to Tribler coding standards.

@devos50
Copy link
Contributor

devos50 commented Oct 14, 2021

This issue has been sufficiently addressed by the components refactoring 👍

@devos50 devos50 closed this as completed Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants