mojidev-py / discmoji

A discord API wrapper made for fun
MIT License
5 stars 0 forks source link

Code Review #1 #4

Closed almostDemoPy closed 1 week ago

almostDemoPy commented 1 week ago

discmoji/bot.py | Lines 70 - 78

    async def get_guild(self,id: int) -> Guild:
        """Gets a guild from the bot's current joined guilds, through the id."""
        check = None
        for guild in self._guild_cache:
            if guild.id == id:
                check = True
                return guild
        if check is None:
            guild = Guild(asyncio.run(self._http.send_request('get',f"/guilds/{id}")).data)

so this is both cache and API call ? check variable is redundant here. return keyword exterminates the function's process and returns the value from whatever in the statement is. so this can be done with just :

for guild in self._guild_cache:
  if guild.id == id:
    return guild
return Guild(...)

discmoji/guild.py | Line 10

class Guild:
  def __init__(self, ...) -> None:
    self.invoked : Optional[Invoked] = inboked

a guild object will have an Invoked object ? assuming your Invoke command has a guild attribute, this is blunder


discmoji/guild.py | Line 41

class Guild:
  def get_member(self, username) -> Member:
    for member in self._member_cache:
      if member.nick == username:
        return member

nick attribute can be different, no ?


discmoji/http.py | Lines 22 - 51

class EndpointManager:
  async def send_request(
    self,
    method : Literal[...]
    route : str
  ) -> Payload:
    ...

what i had in mine was another enum lmao, but this one's good too. what i have is :

class GET(Enum):
  something = lambda var: return var

>>> test = GET.something("hello world")
>>> test
"hello world"

this allows me to just pass the variables and return the endpoint together with the variables, i don't have to type long :P


discmoji/types.py | Line 102

class PermissionsBits(Enum):
  CREATE_INSTANT_INVITE = ...

just use bitwise left shifts lol ( or 2 ** n )

XenonPy commented 1 week ago

Yeah, I wasn't too sure about the check = None either

mojidev-py commented 1 week ago

ok! ty for the advice, and I didn't want to use the id attr, since guild member objects (through message create events) don't provide the user's id, but i'll edit that, and today, i'm coming back to the lib after a long break

almostDemoPy commented 1 week ago

i'll probably do this on mine too but you can utilize the joined_at ISO 8601 timestamp and convert it into a UNIX timestamp, and use the UNIX timestamp to query from your cache ( assuming the one in your cache will also have a non-empty user attribute )

member.user : User = cache.query(joined_at = datetime.fromisoformat(joined_at)).user

References : datetime.datetime.fromisoformat()

mojidev-py commented 1 week ago

i'll probably do this on mine too but you can utilize the joined_at ISO 8601 timestamp and convert it into a UNIX timestamp, and use the UNIX timestamp to query from your cache ( assuming the one in your cache will also have a non-empty user attribute )

member.user : User = cache.query(joined_at = datetime.fromisoformat(joined_at)).user

References : datetime.datetime.fromisoformat()

thank you for the help! I'll use it when i am working on guild members that actually have User attributes.

almostDemoPy commented 1 week ago

by the way. i cloned the repository yesterday to test something out for your behalf, and I received an error regarding types.py file. It seems, that on my end, I may have another global types module, resulting in a circular import. Though I've fixed this by just basically renaming it toenums.py ( similar to what I had in mine )

Also the header text for an error message is too long and quite unnecessary