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

Extract lambdas into functions to improve readability and ease maintenance #7432

Closed
Solomon1732 opened this issue May 22, 2023 · 2 comments · Fixed by #7466
Closed

Extract lambdas into functions to improve readability and ease maintenance #7432

Solomon1732 opened this issue May 22, 2023 · 2 comments · Fixed by #7466

Comments

@Solomon1732
Copy link
Contributor

Solomon1732 commented May 22, 2023

Describe the bug

Extracting lambdas

I noticed there are three lambda declarations. They look to me like they would benefit from being extracted into functions (in the style of _function_name). Each suggestion is below each link. Note I gave placeholder names to extracted functions. They're not full-blown names.

circuit.ready.add_done_callback(lambda f, c=connection.udp_connection, r=request:

...
def select_circuit(self, connection, request):
    def _add_data_if_result(result_func, connection=connection_udp_connection, request=request):
        if not result_func.result():
            return None
        return self.on_socks5_udp_data(connection, request)

...
            circuit.ready.add_done_callback(_add_data_if_result)

RootEndpoint.add_endpoint = lambda self, path, ep: add_endpoint(self, path, ep) \

def _add_endpoint(add_endpoint):
    if path in ['/ipv8', '/market', '/wallets']:
        return None
    return add_endpoint(self, path, ep)
...
with patch_import(modules):
    from tribler.core.components.restapi.rest.root_endpoint import RootEndpoint

    add_endpoint = RootEndpoint.add_endpoint
    RootEndpoint.add_endpoint = _add_endpoint

callback=lambda addr, ih=info_hash: self.on_e2e_finished(addr, ih))

import functools
...
    def monitor_downloads(self, dslist):
...
                    self.join_swarm(info_hash, hops[info_hash], seeding=new_state == DownloadStatus.SEEDING,
                                    callback=functools.partial(self.on_e2e_finished, info_hash=info_hash))

Replacing lambdas with built-ins or library functions

These lambdas can be replaces with built-in functions or library ones, mainly from the python library operator module.

additional_information: dict = field(default_factory=lambda: {}, repr=False)

    additional_information: dict = field(default_factory=dict, repr=False)

self.plot_data = dict(sorted(self.plot_data.items(), key=lambda x: x[0]))

import operator
...
        self.plot_data = dict(sorted(self.plot_data.items(), key=operator.itemgetter(0))

sys_info_dict = defaultdict(lambda: [])

        sys_info_dict = defaultdict(list)

self.plot_data = dict(sorted(self.plot_data.items(), key=lambda x: x[0]))

import operator
...
        self.plot_data = dict(sorted(self.plot_data.items(), key=operator.itemgetter(0))

torrents_list.sort(key=lambda x: x["num_seeders"], reverse=True)

import operator
...
            torrents_list.sort(key=operator.itemgetter("num_seeders"), reverse=True)

removed_peer = min(channel_peers, key=lambda x: x.last_response)

import operator
...
            removed_peer = min(channel_peers, key=operator.attrgetter("last_response"))

return sorted(channel_peers, key=lambda x: x.last_response, reverse=True)[0:limit]

import operator
...
        return sorted(channel_peers, key=operator.attrgetter("last_response"), reverse=True)[0:limit]

for file in sorted(files, key=lambda x: x['index']):

import operator
...
    for file in sorted(files, key=operator.attrgetter("index")):```

            ltsession.pop_alerts = dict
  • Tribler's version [main branch]
@drew2a
Copy link
Contributor

drew2a commented May 23, 2023

@Solomon1732 thank you for your contributions to improving Tribler's code quality. If it's not too much trouble, could you kindly convert this issue into a pull request? This way, we could avoid duplication of work, as it would save us the step of implementing the string replacements that you've described. Plus, we could run tests on the modified code to ensure its correctness.
Thank you in advance for your consideration.

@Solomon1732
Copy link
Contributor Author

No problem. I would be glad to 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants