T-baby / pondpond

Pond is a high performance object-pooling library for Python
https://qin.news/pond
Apache License 2.0
51 stars 2 forks source link

Bug: Obvious issues in pooled_object.py #10

Open rwperrott opened 2 weeks ago

rwperrott commented 2 weeks ago
  1. The class properties declared at lines 23 to 25 are useless and may deceptively cause bugs, so need deleting.

    • I've made this mistake too, which made hard to figure out, mistakenly using class properties, bugs!
  2. All the object self properties are public, which makes them too easy to externally modify! All of them need to be made private, with some exposed read-only via @property self methods.

  3. A destroy self method should be added, to at least set self.__keeping_object to None, for compulsory use by the PooledObjectFactory destroy self method, to sabotage reuse of a Pond dereferenced PoolObject. The pooled_object, is del(eted) at line 213 in pond_class.py, which should be enough, so I removed my extra code for this.

  4. No object function can directly refer to its class name, so has to use 'class-name' or preferably Self, from from typing import Self.

    • PyCharm informed me of this. I think it's because the class doesn't exist yet in the namespace!

A validation Callable probably need to be injected transiently to allow validation of the object, to avoid exposing it to potential misuse via a @property self method for it.

Therefore, lines 19 to 37 of pooled_object.py should probably be replaced by:

from typing import Any, Callable, Self

class PooledObject:
    def __init__(self, obj: Any) -> None:
        self.__create_time = time.time()
        self.__last_borrow_time = time.time()
        self.__kept_object = obj

    def use(self) -> Any:
        return self.keeped_object

    def update_brrow_time(self) -> Self:
        self.__last_borrow_time = time.time()
        return self

    @property
    def create_time(self) -> float:
        return self.__create_time

    @property
    def last_borrow_time(self) -> float:
        return self.__last_borrow_time

    def is_valid(self, test: Callable[[Any], bool]) -> bool:
        return test(self.__kept_object)
rwperrott commented 2 weeks ago

You may find my pond_skim.py gist interesting.