Does this make sense to be a subclass?

I'm trying to create a subclass to implement some custom functionality to a parent class. I'm making a discord bot that has a class

Discord

Discord

. This

Discord

Discord

class has a constructor that takes a token.

I've created a subclass

Bot

Bot

that has a constructor that calls the parent function. I've done this so I can implement environment variables.

class Bot extends Discord
{

    ...

    public function __construct()
    {
        $dotenv = Dotenv::createImmutable(dirname(__DIR__, 1));
        $dotenv->load();
        $dotenv->required('UMPIRE_TOKEN')->notEmpty();

        parent::__construct([
            'token' => $_ENV['UMPIRE_TOKEN'],
            'intents' => Intents::getDefaultIntents(),
        ]);
    }

    ...

}

class Bot extends Discord
{

    ...

    public function __construct()
    {
        $dotenv = Dotenv::createImmutable(dirname(__DIR__, 1));
        $dotenv->load();
        $dotenv->required('UMPIRE_TOKEN')->notEmpty();

        parent::__construct([
            'token' => $_ENV['UMPIRE_TOKEN'],
            'intents' => Intents::getDefaultIntents(),
        ]);
    }

    ...

}

Everything runs fine but for some reason, this just looks really off to me. I'm not sure if I'm overcomplicating it or creating an antipattern.

Ἔ

ἔρως•6/25/24, 9:55 PM

honestly, this looks a bit overcomplicated

Ἔ

ἔρως•6/25/24, 9:55 PM

you're still going to have discord-specific code on the

bot

bot

class

Ἔ

ἔρως•6/25/24, 9:56 PM

the

bot

bot

makes sense if it is an abstract class that has something generic that can be used for bots that are for other services, like teams or slack or twitch or something else

Ἔ

ἔρως•6/25/24, 10:02 PM

for example, if you need to send a message from the bot, you can have something like this:

abstract class Bot {
  ...
  abstract public function sendMessage(string $channelID, string $message);
  ...
}

class DiscordBot extends Bot {
  ...
  public function sendMessage(string $channelID, string $message) {
    ...
  }
  ...
}

abstract class Bot {
  ...
  abstract public function sendMessage(string $channelID, string $message);
  ...
}

class DiscordBot extends Bot {
  ...
  public function sendMessage(string $channelID, string $message) {
    ...
  }
  ...
}

so, this forces the bot to have specific methods implemented, which can be used on any other bot

Ἔ

ἔρως•6/25/24, 10:03 PM

now, imagine your bot does some processing, like checking the user

Ἔ

ἔρως•6/25/24, 10:10 PM

you can do something like this:

abstract class Bot {
  ...
  abstract public function sendMessage(string $channelID, string $message);
  
  abstract public function userExists(string $email);
  abstract public function userExistsByID(string $id);
  
  protected function userExistsInDB(string $email) {
    ...
  }
  ...
}

class DiscordBot extends Bot {
  ...
  public function sendMessage(string $channelID, string $message) {
    ...
  }

  public function userExists(string $email) {
    // pretend there's some validation of the email
    if(!parent::emailValid($email)) {
      return null;
    }
    
    return parent::userExistsInDB($email);
  }
  
  public function userExistsByID(string $id) {
    $user = $this->getUser();
    $email = $user->email;
    
    return self::userExists($email);
  }
  ...
}

abstract class Bot {
  ...
  abstract public function sendMessage(string $channelID, string $message);
  
  abstract public function userExists(string $email);
  abstract public function userExistsByID(string $id);
  
  protected function userExistsInDB(string $email) {
    ...
  }
  ...
}

class DiscordBot extends Bot {
  ...
  public function sendMessage(string $channelID, string $message) {
    ...
  }

  public function userExists(string $email) {
    // pretend there's some validation of the email
    if(!parent::emailValid($email)) {
      return null;
    }
    
    return parent::userExistsInDB($email);
  }
  
  public function userExistsByID(string $id) {
    $user = $this->getUser();
    $email = $user->email;
    
    return self::userExists($email);
  }
  ...
}

Ἔ

ἔρως•6/25/24, 10:13 PM

now, if you decide to implement a bot for something else, you can just do the same and extend from

Bot

Bot

, keep the same contract, guarantee that the same functions exist everywhere and have all the generic code in a generic place that's not repeated

Ἔ

ἔρως•6/25/24, 10:13 PM

for example, doesn't make sense to have discord-specific code in

Bot

Bot

, and doesn't make sense to have database access in

DiscordBot

DiscordBot

Hashi•6/25/24, 10:19 PM

That’s pretty great answer but guessing from his previous question - “Discord” class is external/dependency project class, and you are meant to use it by calling its constructor with those 2 params.
Thus my guess is his goal is to take out the part of the code where he fetches the needed param(s) from the env file to a “function”/constructor to avoid repetition in case he needs the discord class instance in more than one place in the code

Ἔ

ἔρως•6/25/24, 10:21 PM

makes sense, but for different bots you will need different configurations

Ἔ

ἔρως•6/25/24, 10:22 PM

which can't be made in a generic sense

Ἔ

ἔρως•6/25/24, 10:23 PM

what you can do is create an abstract function in

Bot

Bot

that demands you to implement the fetching of the settings yourself

Ἔ

ἔρως•6/25/24, 10:23 PM

and, obviously, you can share the same instance of

DotEnv

DotEnv

between both classes

Ἔ

ἔρως•6/25/24, 10:27 PM

in this case, this:

abstract class Bot {
  public $dotenv = null;
  
  public function __construct() {
    $dotenv = Dotenv::createImmutable(...);
    $dotenv->load();
    
    $this->dotend = $dotenv;
  }
  ...
}

class DiscordBot extends Bot {
  public function __construct() {
    $this->dotend->required('UMPIRE_TOKEN')->notEmpty();
    
    $this->doSomething([
        'token' => $_ENV['UMPIRE_TOKEN'],
        'intents' => Intents::getDefaultIntents(),
    ]);
  }
  ...
}

abstract class Bot {
  public $dotenv = null;
  
  public function __construct() {
    $dotenv = Dotenv::createImmutable(...);
    $dotenv->load();
    
    $this->dotend = $dotenv;
  }
  ...
}

class DiscordBot extends Bot {
  public function __construct() {
    $this->dotend->required('UMPIRE_TOKEN')->notEmpty();
    
    $this->doSomething([
        'token' => $_ENV['UMPIRE_TOKEN'],
        'intents' => Intents::getDefaultIntents(),
    ]);
  }
  ...
}

vinceOP•6/25/24, 10:27 PM

Sorry guys I haven't gotten a full chance to read everything but I wanted to say I originally wanted to make this a static class. And instead of

Bot

Bot

I should probably rename this to

Umpire

Umpire

(my discord bot name). I don't really plan this to be reusable but instead an instance of my custom class that I can call static methods on and initialize with the env vars

Ἔ

ἔρως•6/25/24, 10:27 PM

this is how i would implement the basic steps

Vvince Sorry guys I haven't gotten a full chance to read everything but I wanted to say...

Ἔ

ἔρως•6/25/24, 10:28 PM

if you're not going to make it generic, doesn't make sense to separate both

vinceOP•6/25/24, 10:29 PM

My thinking is I want to abstract functions from the main file

vinceOP•6/25/24, 10:29 PM

Instead of having a messy main file I have 1 class that handles most of the methods that I can call statically in the main file

Ἔ

ἔρως•6/25/24, 10:29 PM

yeah, and the best way to implement it is how i explained it

Ἔ

ἔρως•6/25/24, 10:30 PM

this is because each class does it's own thing

Ἔ

ἔρως•6/25/24, 10:30 PM

what you had before, both classes do parts of the same thing

Ἔ

ἔρως•6/25/24, 10:30 PM

both have discord specific code and handling

Ἔ

ἔρως•6/25/24, 10:31 PM

if you decide to make it a slack bot later on, then you will have to do what i did anyways

Ἔ

ἔρως•6/25/24, 10:31 PM

unless you're okay with repeating code

vinceOP•6/25/24, 10:33 PM

Got it, reading everything now

Hashi•6/25/24, 10:34 PM

From what I understand what you want I would call it “DiscordBotBuilder”/ “DiscordUmpireBuilder” static class with static functions to tidy your main function that will use them

Hashi•6/25/24, 10:35 PM

With no goal of decoupling and enabling further extension

vinceOP•6/25/24, 10:36 PM

Yes, exactly that! I just wanted a nice way to call an object that contains methods I need (that would be extended from

Discord

Discord

) and a tidy way to initialize the

Discord

Discord

class (or some instance of it) with my env vars

vinceOP•6/25/24, 10:37 PM

There is no way I will ever use this for anything else in the future and if there comes a time then, I'll be better equipped to rewrite it anyway since I'll know more about oop / php

Ἔ

ἔρως•6/25/24, 10:37 PM

well, you can do that with my method

vinceOP•6/25/24, 10:37 PM

Ye not saying I can't, I'm reading abstract classes now

vinceOP•6/25/24, 10:37 PM

That's how little I know about oop lol

Ἔ

ἔρως•6/25/24, 10:37 PM

but i would make the generic

Bot

Bot

a singleton

Ἔ

ἔρως•6/25/24, 10:38 PM

or create a bot factory

Ἔἔρως but i would make the generic `Bot` a singleton

vinceOP•6/25/24, 10:38 PM

I was thinking that too but I felt that might be a bit overkill but idk anything

Ἔ

ἔρως•6/25/24, 10:38 PM

a singleton is what you described

Ἔ

ἔρως•6/25/24, 10:39 PM

a class you initialize just once, and use it multiple times and you share the same instance everywhere, with static method or not

Ἔ

ἔρως•6/25/24, 10:39 PM

but the idea is that there's ever just 1 instance, at most

Hashi•6/25/24, 10:39 PM

The problem with that is he has to write his abstract run() and abstract on() and decoupling those will produce some lines of code.

Ἔἔρως a class you initialize just once, and use it multiple times and you share the sa...

vinceOP•6/25/24, 10:40 PM

See I was thinking that I don't even need it to be initialized in an object -- I'd be happy if I was just calling it from the class whch is why I wanted it to be static but I was getting errors trying to do that

Hashi•6/25/24, 10:40 PM

Cuz he probably needs the on() and run() from Discord class

Ἔἔρως you can do something like this: ```php abstract class Bot { ... abstract pub...

vinceOP•6/25/24, 10:42 PM

oh okay I understand this now but yea not sure if this is what im looking for could be wrong tho

vinceOP•6/25/24, 10:44 PM

I was writing this whole thing out but realized that yea, definitely can't make this static

vinceOP•6/25/24, 10:45 PM

Or can I

vinceOP•6/25/24, 10:46 PM

I think I definitely need a singleton now that I'm writing it out and thinking about it more

vinceOP•6/25/24, 10:47 PM

Because the

Discord

Discord

class uses a constructor, so I can't get the instance of the

Discord

Discord

class on the static

Umpire

Umpire

class

Hashi•6/25/24, 10:51 PM

If you want umpire to be singleton you can’t make it static, but usually you hide the constructor by making it private if the language allows and you define static function like “getInstance()” that makes sure it creates it’s instance only once and then saves it in its fields and every following call to it just returns that same instance

vinceOP•6/25/24, 11:07 PM

Okay, I think I got it.

Here's what I have:

// UmpireSingleton.php

<?php

namespace Umpire;

use Dotenv\Dotenv;
use Discord\Discord;
use Discord\Parts\Channel\Message;

class UmpireSingleton extends Discord
{
    private static UmpireSingleton $instance;

    private function __construct()
    {
        $dotenv = Dotenv::createImmutable(dirname(__DIR__, 1));
        $dotenv->load();
        $dotenv->required(['UMPIRE_TOKEN'])->notEmpty();

        parent::__construct([
            'token' => $_ENV['UMPIRE_TOKEN'],
        ]);
    }

    public static function getInstance(): UmpireSingleton
    {
        if (!isset(self::$instance)) {
            self::$instance = new self();
        }

        return self::$instance;
    }

    public static function startTopicThread(Message $message, array $topic): void
    {
        ...
    }
}

// UmpireSingleton.php

<?php

namespace Umpire;

use Dotenv\Dotenv;
use Discord\Discord;
use Discord\Parts\Channel\Message;

class UmpireSingleton extends Discord
{
    private static UmpireSingleton $instance;

    private function __construct()
    {
        $dotenv = Dotenv::createImmutable(dirname(__DIR__, 1));
        $dotenv->load();
        $dotenv->required(['UMPIRE_TOKEN'])->notEmpty();

        parent::__construct([
            'token' => $_ENV['UMPIRE_TOKEN'],
        ]);
    }

    public static function getInstance(): UmpireSingleton
    {
        if (!isset(self::$instance)) {
            self::$instance = new self();
        }

        return self::$instance;
    }

    public static function startTopicThread(Message $message, array $topic): void
    {
        ...
    }
}

And then:

// main.php

<?php

include __DIR__ . '/vendor/autoload.php';

use Umpire\UmpireSingleton;

$umpire = UmpireSingleton::getInstance();

...

// main.php

<?php

include __DIR__ . '/vendor/autoload.php';

use Umpire\UmpireSingleton;

$umpire = UmpireSingleton::getInstance();

...

vinceOP•6/25/24, 11:07 PM

Looks / functions better?