Tribler / anydex-core

The Universal Decentralized Exchange
GNU Lesser General Public License v3.0
9 stars 6 forks source link

Possible minor code improvements #63

Open Solomon1732 opened 3 years ago

Solomon1732 commented 3 years ago

In mind I have a small number of possible (minor) improvements:

Replace % string operators with f-strings

Using the % operator is somewhat brittle AFAIK, and can arguably be less readable. Example: https://github.com/Tribler/anydex-core/blob/master/anydex/core/tickentry.py#L144

Replace list comprehensions in str.join with generator expressions

Building a list for a str.join statement is an operation which can be skipped. Instead it can be replaced with a generator expression. Example: https://github.com/Tribler/anydex-core/blob/master/anydex/core/match_queue.py#L13

# This
return ' '.join([str(i) for i in self.queue])
# Can be replaced with this
return ' '.join(str(i) for i in self.queue)

Remove object as parent class

Since the default parent class of new classes in Py3 is object by default, such a declaration became redundant. Such declarations can be deleted with no adverse effects. Example: https://github.com/Tribler/anydex-core/blob/master/anydex/core/settings.py

Are points 1 and 3 artifacts from transitioning from Py2 to Py3?

I know these are all minor points. I don't mind creating one or more PRs for what I suggested, and in fact I'll be glad to make them since I am aware that such issues are of a low priority.

Edit:

Replace two argument form calls to super() with the zero argument form when appropriate

https://docs.python.org/3/library/functions.html#super https://stackoverflow.com/a/576183 Code such as

class Cls:
    def __init__(*args, **kwargs):
        super(Cls, self).__init__():

can be replaced with

class Cls:
    def __init__(*args, **kwargs):
        super().__init__():

The two are equivalent. Example: https://github.com/Tribler/anydex-core/blob/master/anydex/core/message.py#L13

Solomon1732 commented 3 years ago

Looking through the code I ran into MatchPriorityQueue. Internally it uses tuples as entries. It looks like the aspect of code readability it could benefit if typing.NamedTuples are used. As far as I am aware using them isn't significantly more expensive (performance-wise) than using plain tuples. Example:

# This
def contains_order(self, order_id):
    for _, _, other_order_id, _ in self.queue:
        if other_order_id == order_id:
            return True
    return False

# Might look like this
def contains_order(self, order_id):
    for other_order in self.queue:
        if other_order.id == order_id:
            return True
    return False
Solomon1732 commented 3 years ago

https://github.com/Tribler/anydex-core/blob/master/anydex/core/database.py#L128 The code in the link could be replaced with something like the following:

def get_order(self, order_id):
    """
    Return an order with a specific id.
    """
    db_result = next(
        self.execute(
            "SELECT * FROM orders WHERE trader_id = ? AND order_number = ?",
            database_blob(bytes(order_id.trader_id), str(order_id.order_number))
        ),
        # This is an optional default value which can be returned
        None,
    )
    return None if db_result is None else Order.from_database(db_result, self.get_reserved_ticks(order_id))

This prevents the need to set up a try-except for StopIteration

Edit:

Replace ValueError with TypeError when checking for types

When checking for types and the wrong type is found it might be more appropriate to raise a TypeError instead of a ValueError. Example: https://github.com/Tribler/anydex-core/blob/master/anydex/core/timestamp.py#L16

devos50 commented 3 years ago

Thanks for your suggestions!

Recently, the ideas implemented in this repository (low-risk, universal asset trading) have been accepted for publication. Also, the matchmaking logic has recently been published. I think further improvements can be made to the exchange mechanism but we consider the fundamental work as finished. As such, the future plans of this repository are not clear yet.

Even though we highly value any contribution, your time might be better spent working on one of our other projects, for example, Tribler, our mobile superapp or our accountability toolbox. This would also depend on how much time you would like to invest into one of our projects, and your interests/expertise 👍 . Do you have any preferences/ideas?

Solomon1732 commented 3 years ago

Gotcha. Happy to help!

I have just above beginner skills in python, and I can read C code, but that's it. Do you have any suggestions?

devos50 commented 3 years ago

It depends also on what you like to work on. You can, for example: