[Code Review] - Procuro pessoas para fazer CodeReview de meu primeiro projeto de faculdade em PHP · JohnVantas

Fala JV blz? Eu olhei só a parte msm do PHP, mas acho que já é suficiente pra tirar umas boas ideias

Receber parametros por variaveis de ambiente:

Nessa parte você devia ler parâmetros sensíveis por variáveis de ambiente, invés de deixar hard-coded no código. Além de tornar o processo de deploy mais dinâmico também evita vunerabilidade no seu projeto. Se o código vazar qualquer um vai conseguir logar na sua DB. Também já ouvi dizer que o melhor é usar PDO connection invés de mysqli

Instância para Database como variável global:

Ao invés de manualmente instânciar uma BD e dps fazer unset() para ativar o método desconstrutor, você deveria criar uma instância static e fazer new() no arquivo de entrypoint do seu projeto. Deixei um exemplo mais pro final de como poderia ser pra usar essa BD global.

Carregar as partes PHP dinamicamente:

Quase 90% do seu site você fica duplicando todo o html, invés disso deveria ter usado mais componentização - Como até fez em alguns lugares, deveria ter explorado mais esse recurso. Até porque essa é a diferença entre as páginas "estaticas e "dinâmicas" que devem ter abordado no curso.

Aqui vc usou "componentização": Deveria ter feito assim pra tudo, isso ia deixar muito mais simples de fazer manutenção das suas páginas e extender o projeto, além de evitar duplicação de código desnecessária.

Se teu prof. pedir pra tu mudar ai alguma coisa na navbar do site vai ter q sair procurando em todos os <html> que você fez.

Validar Sessão(Autentificação):

Antes de fazer esses delete ou qualquer outra operação de privilégios elevados você deve pelo menos verificar se o user tem $SESSION ou não.

Não sei oq sra é, eu to presumindo que seja o ID da coisa que ta apagando. Mas vc ta pegando isso via POST qualquer um consegue fazer request nesse arquivo .php e bypassar sua auth

Olha aqui oque eu falei vc ta metendo um input hidden com esse valor, dps aceita isso no lado do server. É mais facil abrir a $SESSIONlogo direto pelo server e carregar o valor de lá. Pq eu posso só inspecionar a página e mudar o conteudo desse input ai e apagar quem eu quiser do sistema.

Está confiando de mais que esse "Private/php/" seja realmente private viu 😂

Validar/Sanitização dos inputs do form:

Você só faz trim, mas nada impede de colocar "" e inserir com nome em branco!

Na sua TABLE a unica constraint é o NOT NULL

Também is_unregistered ta escrito ao contrário, por que siginifica "Não está registrado?" - dai quando olho o corpo do if eu vejo que a intenção era um is_already_registered ou is_duplicated_login. Ou seja, eu tive que olhar a implementação pra entender o que faz porque se tivesse que dar manutenção e só seguido o nome des-indicativo que você escolheu eu provavelmente teria adicionado um bug no sistema

Eu diria que projetos como esse com fins educacionais do seu curso o melhor seria usar PT BR mesmo. Deixa pra fazer Inglês só coisas pessoais ou em local de trabalho(dependendo da cultura da empresa)

Menos é mais!:

Eu sei que vcs tentaram montar umas abstrações pras queries mas nesse tipo de projeto eu acho que faz mas sentido você ter micro funções que já te retornam as queries montadas. Assim imagina, eu olhando o código saberia logo quais são os dados que ta buscando, inserindo etc...

Exemplo do que to falando:

Vamos ver esse código aqui, você ta praticamente tendo que escrever a query toda do mesmo jeito, e ainda tem suas abstrações(que se não forem bem testadas) podem acabar é criando pontos de vunerabilidade. Ainda por cima vc ta rodando duas queries que poderia m ser feitas numa só:

$db = new Database();

$db->update("student_tbl", "student_active", '0', "student_ra =", array($_POST["sra"]));
$db->update("student_tbl", "deleted_at", get_utc_time(), "student_ra =", array($_POST["sra"]));

unset($db);

// Nº de linha: 4 + as linhas internas que vc teve q implementar na Database é função de placeholder

Como poderia ser:


function set_student_as_deleted(int $student_id) {
    // Cria uma static property na classe database, dai vc faz `new` uma unica vez no arquivo entrypoint e atribui pra lá.
    $db = Database::get_instance();
    
    $sql = "
        UPDATE student_tbl SET
        student_active = 0,
        deleted_at = NOW(),
        WHERE student_ra = :student_id
    ";

    // Dá pra fazer um metodo `execute` na sua Database, para 
    // abstrair a parte de `prepare` e `close`. Assim você garante que 
    // a connection não fica aberta enquanto mantém a syntax simples.
    $db->execute($sql, 'student_id' => $student_id);
    
    // Nº de linhas: 8 - porém codigo ta muito mais explicito, evitei uma query desnecessária, não tive que me preocupar em fica instanciando e dropando a bd nem ter que fazer abstrações desnecessárias pro scopo do projeto.
}

😅 Eu não manjo muito de PHP mas espero ter ajudado! Recomendo que agora pra você chegar no próximo nível dessa linguagem, você aprender sobre o PHP Composer, dá uma estuda nisso e na PSR que seu código PHP vai ficar muito mais estruturado e também vai conseguir explorar muito mais os recursos de POO e arquitetura de projeto. Na minha época eu assisti essa playlist aqui: PHP tips

Se quiser ver um pouco dos projetos que faço ta aqui meu Github