Tribler / tribler

Privacy enhanced BitTorrent client with P2P content discovery
https://www.tribler.org
GNU General Public License v3.0
4.73k stars 445 forks source link

Refactor magnet link copy function #8017

Closed drew2a closed 1 month ago

drew2a commented 2 months ago

The function for copying the magnet link has been refactored. The code now is a bit safier.

Fixes #8016

drew2a commented 1 month ago

Yes, it is more verbose, but that is not necessarily a bad thing. More lines of code do not always equate to worse code.

It is more verbose because:

  1. The logic was extracted into the function create_magnet, which makes it possible to properly test it. The previous version did not allow this.
  2. The line:
        trackers = [
            tk['url'] for tk in self.current_download['trackers'] if 'url' in tk and tk['url'] not in ['[DHT]', '[PeX]']
        ]

was rewritten to:

        invalid_urls = {'[DHT]', '[PeX]'}
        urls = (t.get('url') for t in trackers)
        valid_urls = [u for u in urls if u and u not in invalid_urls]

This makes it more understandable by splitting a large and complex expression into two expressions with clear purposes:

  1. Mapping the sequence
  2. Filtering the sequence
drew2a commented 1 month ago

@kozlovsky, thank you for the review. I've addressed most of your comments except for the verbosity. For the explanation, please see above.