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

Bug: Obvious issues in pooled_object.py #10

Open
rwperrott opened this issue Aug 24, 2024 · 1 comment
Open

Bug: Obvious issues in pooled_object.py #10

rwperrott opened this issue Aug 24, 2024 · 1 comment

Comments

@rwperrott
Copy link

rwperrott commented Aug 24, 2024

  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)
  • The above was typed in a separate file in PyCharm (Community Edition), so should be fully valid code.

  • I've written my own Generics typed sub-class/wrapper classes for the key Pond classes, which I may put somewhere on my GitHub account, after I've proved that they work well. When I do, I may update you.

@rwperrott
Copy link
Author

You may find my pond_skim.py gist interesting.

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

No branches or pull requests

1 participant