feat: decouple commands

[diomonogatari]

closes #64

[pxpm]

🧹

edit: to left a reference https://refactoring.guru/design-patterns/command

[pxpm]

Parece um bocado "meh" ter uma lista de comandos e um check para este comando.
Não há nada que se possa fazer quanto a isto ?

Se calhar os comandos em ver de estarem num .json, serem classes com um execute() a la middleware style ? E termos um mapa dos comandos para as respectivas classes ?

[diomonogatari]

ya, tb fiquei um bocado "meh" porque teres um .json só é fixe para os commands com message definida.

Vou procurar uma melhor opção, tal como mencionaste.

[diomonogatari]

Done.

commands.json removido e command pattern implementado.

Acabei por remover o CodewarsLeaderboard Use case e converter num command.

Please review :^)

[pxpm]

Não acho necessário estar a passar estas deps. aqui.
Elas são utilizadas apenas nos comandos, portanto cada comando pode importar as deps. que precisa não ?

Isto pode ser parte da "aquitectura hexagonal" e eu não estou a par, nesse caso ignora 👍

[diomonogatari]

No fundo, como não há um DI container (que eu saiba) no projeto, acabei por optar criar instancias de comandos que vão viver no scope do commandUseCaseResolver.ts. Para criar as instancias, preciso de passar as deps necessarias.

A alternativa seria ter constructores vazios e passar os deps no .execute() como sugeriste.

Para ser seguro, teria que validar que as deps passadas são as necessárias para cada Command, em jeito de:

async execute(context: Context, deps: { chatService: ChatService; kataService: KataService }): Promise<void> {
    if (!deps.chatService || !deps.kataService) {
      throw new Error('Required dependencies not provided');
    }

    const { chatService, kataService } = deps;
    //...
}

já que temos classes, mais vale usar.
Se fosse tudo clean FP, aí sim acho que faria sentido a tua sugestão.

Lemme know what you think!
Cheers,

P.S.:

Kiwi aprova! :D

[Alf0nso]

O kiwi aprovar efectivament é um ponto muito forte honestamente...
em que ficamos @pxpm @diomonogatari ? aceitamos e merge?

[pxpm]

Por mim tudo bem, vocês decidem.
Eu no meu POV não acho necessário passar as deps para os comandos. Agora é o chat e o kata, depois vem outro comando com o 3rdPartyApi, depois vem o 4thPartyApi.

Mais uma vez, não sou super entendido na arquitetura, portanto perdoem-me se estiver errado, mas diria que cada comando sabe das dependências que precisa e faz load delas:

// dontAskToAskCommand.ts

import chatService from "../../domain/service/chatService";

private chatService chatService;

constructor() {
    this.chatService = chatService;
  }

  async execute(context: Context): Promise<void> {
    await this.chatService.sendMessageToChannel(this.message, context.channelId);
  }

// commandResolver.ts
const commandInstance = this.commands.find((cmd) => cmd.name === command);
await commandInstance.execute(context);

Se quisermos adicionar outra dep. ao dontAskToAsk só mudamos 1 ficheiro, em vez de 2 🤷. E caso precisemos de usar o comando noutro lado qualquer diretamente sem necessidade do command resolver estamos garantidos que a "inicialização" se mantem.

Your call 👍

[diomonogatari]

// dontAskToAskCommand.ts

import chatService from "../../domain/service/chatService";

private chatService chatService;

constructor() {
    this.chatService = chatService; // error
  }

  async execute(context: Context): Promise<void> {
    await this.chatService.sendMessageToChannel(this.message, context.channelId);
  }

Infelizmente, para termos instancias de qualquer command em algum sítio vamos ter que passar os serviços necessários para o command funcionar.

Com o sistema atual, ou vêm do constructor ou são passados como argumento do execute().
Mas já vi que posso melhorar na mesma.

O CommandUseCaseResolver poderá receber a coleção de commands que são possíveis.
Ou seja:

• Criar as instancias necessárias de ICommands no index.ts
• Passar as mesmas para o `CommandUseCaseResolver

Algo do estilo:

// index.ts
const commands: Command[] = [
    new CodewarsLeaderboardCommand(chatService, kataService),
    new DontAskToAskCommand(chatService),
    new OnlyCodeQuestionsCommand(chatService)
  ]
const useCaseResolver = new CommandUseCaseResolver({
  commands,
  loggerService,
});

// commandUseCaseResolve.ts
constructor({
    commands,
    loggerService,
  }: {
    commands: Command[];
    loggerService: LoggerService;
  }) {
    this.loggerService = loggerService;

    this.commands = commands;
  }

@pxpm e @Alf0nso é isto que irei fazer, mas ainda preciso de dar fix nos tests antes de fazer commit deste final cleanup :D

Cheers

[diomonogatari]

nvm lol, tests foram fáceis

[pxpm]

Boas @diomonogatari aquilo era meio pseudo-code, não podes dar assign de uma interface. Era mais para dar uma ideia geral do que eu estava a pensar.
Indo mais concretamente á solução que eu imaginava, seria alguma coisa deste gênero:

1) mover o que é relacionado com o discord para um service na application/, eg:

// application/services/discordService.ts

import ChatService from "../../domain/service/chatService";
import DiscordChatService from "../../infrastructure/service/discordChatService";
import { Client, Intents } from "discord.js";

const discordClient = new Client({
  intents: [Intents.FLAGS.GUILDS, Intents.FLAGS.GUILD_MEMBERS, Intents.FLAGS.GUILD_MESSAGES],
});

export const 
    chatService: ChatService = new DiscordChatService(discordClient),
    client: any = discordClient;

2) Remover as dependências do construtor dos comandos :

// application/command/dontAskToAskCommand.ts

import { Command, Context } from "../../types";
import { chatService } from "../services/discordService";

export default class DontAskToAskCommand implements Command {
  readonly name = "!ja";

  private readonly message: string =
    "Olá! Experimenta fazer a pergunta diretamente e contar o que já tentaste! Sabe mais aqui :point_right: https://dontasktoask.com/pt-pt/";
  
  async execute(context: Context): Promise<void> {
    await chatService.sendMessageToChannel(this.message, context.channelId);
  }
}

// no index.js

import { chatService, client } from "./application/services/discordService";

// remover os inits que foram movidos para o discordService.ts

E depois lá ter os comandos nalgum array:

[
    new dontAskToAskCommand(),
    new otherCommand(),
]

quando chamas o command.execute(context) ele lá usa as dependências que precisa, importando as instâncias previamente criadas.

Mais uma vez, leva isto com um grain of salt, não sei se isto vai contra alguma regra da arquitetura 👍