Cada vez mais está caindo o conceito de código limpo. e aqui vou fazer alguma... · Pilati

Cada vez mais está caindo o conceito de código limpo. e aqui vou fazer algumas críticas ao seu código:

/**
 * Calcula o valor total de um pedido com base nos seus itens.
 * @param items - Um array de itens do pedido.
 * @returns O valor total calculado.
 */
function calculateOrderTotal(items: OrderItem[]): number {
  return items.reduce((sum, item) => sum + item.price * item.quantity, 0);
}

Esse comentário é totalmente desnecessário.

function calculateOrderTotal(items: OrderItem[]): number {
  return items.reduce((sum, item) => sum + item.price * item.quantity, 0);
}

Somente o código sozinho já deixa claro o que o método está fazendo.

A assinatura da função dispensa completamente o uso do comentário, dá pra entender completamente o que é feito.

Em um projeto eu vou perder 5 a 10 minutos comentando todas as funções? Jamais.

Agora trago exemplos de casos reais:

 public SignContractService(
    SignContractRepository scRepository,
    ValidationResponseTransformer transformer,
    OracleObjectStorageService oracleOsService,
    UploadService uploadService,
    ContractRenderPdfService contractRenderPdfService,
    CompanySignatureRepository companySignatureRepository,
    AppDbContext dbContext)
{
    this.scRepository = scRepository;
    this.transformer = transformer;
    this.oracleOsService = oracleOsService;
    this.uploadService = uploadService;
    this.contractRenderPdfService = contractRenderPdfService;
    this.companySignatureRepository = companySignatureRepository;
    this.dbContext = dbContext;
}

public async Task<DefaultResponseDTO> SignContract(
    Guid eventId,
    Guid eventRoleCollaboratorId,
    Guid contractId,
    SignContractDTO signContractDTO)
{
    var roleInvalid = await dbContext.CollaboratorEventRoles
        .Where(cer => dbContext.EventRoleCollaborators
            .Where(erc => erc.CollaboratorId == cer.CollaboratorId)
            .Where(erc => erc.EventEventRole.EventRoleId == cer.EventRoleId)
            .Any(erc => erc.Id == eventRoleCollaboratorId)
        ).AnyAsync(cer => cer.Status != Enums.Collaborator.CollaboratorEventRoleStatus.Active);

    if (roleInvalid) return transformer.GenerateDefaultErrorResponse("Cargo com pendências, não é possível assinar o contrato");

    var contract = await scRepository.GetContractAsync(eventRoleCollaboratorId, contractId);
    if (contract == null) return transformer.GenerateDefaultErrorResponse("Contrato não encontrado");

    var signaturePath = $"event/{eventId}/collaborator/{eventRoleCollaboratorId}/contract/{contractId}/signature.png";
    var contractPath = $"event/{eventId}/collaborator/{eventRoleCollaboratorId}/contract/{contractId}/contract.pdf";
    await oracleOsService.MoveFile(
        uploadService.GetUploadPath(signContractDTO.signatureId),
        signaturePath
    );

    var collaboratorSignature = await oracleOsService.DownloadFile(signaturePath);
    if (collaboratorSignature == null) return transformer.GenerateDefaultErrorResponse("Erro ao baixar a assinatura do colaborador");

    var currentCompanySignature = await companySignatureRepository.GetCurrentSignature();
    if (currentCompanySignature == null) return transformer.GenerateDefaultErrorResponse("A assinatura da empresa não está cadastrada");

    var companySignature = await oracleOsService.DownloadFile(currentCompanySignature.Path);
    if (companySignature == null) return transformer.GenerateDefaultErrorResponse("Erro ao baixar a assinatura da empresa");

    var pdfBytes = await contractRenderPdfService.RenderContract(contract, eventRoleCollaboratorId, collaboratorSignature, companySignature);
    if (pdfBytes == null) return transformer.GenerateDefaultErrorResponse("Erro ao gerar o PDF do contrato");

    var uploaded = await oracleOsService.UploadFile(contractPath, pdfBytes, "application/pdf");
    if (!uploaded) return transformer.GenerateDefaultErrorResponse("Erro ao salvar o contrato assinado");

    var signedContractModel = new EventRoleCollaboratorContract
    {
        EventRoleCollaboratorId = eventRoleCollaboratorId,
        ContractModelId = contractId,
        signatureImgPath = signaturePath,
        signedContractPath = contractPath,
    };
    await scRepository.AddEventRoleCollaboratorContract(signedContractModel);

    return new DefaultResponseDTO();
}

Tamanho Ideal: Idealmente, uma função não deve ultrapassar 15-20 linhas. Se uma função está ficando longa, provavelmente está fazendo demais.

Essa função está gigante: Validando entrada do usuário, comunicando com 3 serviços, comunicando com 2 repositórios, fazendo query, salvando.

Te desafio a refatorar ela com Clean Code!

Vai ficar péssimo! ela está legível, cumpre o que faz, você que nunca viu o código sabe o que ela faz!

Considerações pessoais

É bom saber clean code? SIM!

Devemos levar ao pé da letra? Não!

Devemos saber quando quebrar a regra!

Concordo e gostaria de complementar: detesto essas regras que determinam um limite para a quantidade de linhas de uma classe, método, ou seja lá o que for.

Pra isso vou contar um caso real. Uma vez uma função ultrapassou esse limite, não lembro o número exato mas vamos dizer que era 30 linhas e ela ficou com 31. Ou seja:

class blabla {
    // vários métodos...

    function fazAlgo() {
        // 31 linhas aqui dentro
    }

    // outros métdodos
}

Pra satisfazer a regra, tive que pegar um trecho qualquer (digamos que foi de 10 linhas) e jogar para outra função:

class blabla {
    // vários métodos...

    function fazAlgo() {
        // 21 linhas aqui dentro
        novaFuncao(); // nova função com 10 linhas tiradas daqui
    }

    function novoMetodo() {
        // 10 linhas que antes estavam em fazAlgo
    }

    // outros métodos...
}

Ou seja, o método original tinha 31 linhas e fiz o seguinte:

joguei 10 linhas para outro método (original fica com 21)
adicionei a chamada do novo método (original fica com 22)

Mas olha só, pra fazer isso eu tive que adicionar também a linha function novoMetodo() { e o respectivo fechamento }. E uma linha em branco antes e depois (regras de formatação do projeto). Ou seja, a classe como um todo ficou com mais linhas.

E ao fazer isso com vários métodos, o limite de linhas por classe também foi ultrapassado, ou seja, tive que criar uma classe auxiliar e mover alguns métodos para lá.

E no fim o código ficou pior porque não fazia sentido quebrar em métodos menores, já que tudo era parte da mesma operação. O mesmo vale para a classe auxiliar, que só ajudou a aumentar a complexidade desnecessariamente.

Tudo isso pra satisfazer uma regra arbitrária envolvendo um número mágico que alguém definiu.

Não me entenda mal, algum conjunto mínimo de regras e padrões devem existir para não virar bagunça. E tem vezes que faz todo sentido quebrar em métodos menores ou separar em outras classes (mas se vc fez isso somente por causa da quantidade de linhas, aí complicou à toa, na minha opinião).

E a partir do momento em que essas regras começam a atrapalhar, deve-se questionar se os benefícios compensam os problemas causados. Eu particularmente detesto essas regrinhas do tipo "máximo de linhas", "nunca use tal coisa", "quantidade máxima de parâmetros", etc. Tem que sempre avaliar caso a caso e não seguir cegamente só porque algum guru/influencer/livro famoso falou.