Moje konto na GitHubie.

pytanie zadane 16 grudnia 2017 w Nasze projekty przez amb00 Bywalec (2,000 p.)

Witam, zapraszam do zapoznania się z moimi dotychczasowymi projektami na GitHubie. 3 w Pythonie, 2 w C++ i jeden w PHP. Wszelkie uwagi/sugestie mile widziane :)

https://github.com/arkadiusz97

4 odpowiedzi

odpowiedź 16 grudnia 2017 przez Jedras Maniak (54,860 p.)

Rzuciłem tylko okiem i jest trochę rzeczy do poprawy, ale z drugiej strony nie ma tak naprawdę czego tutaj oceniać. Te wszystkie projekty to kilka godzin pracy. Może postarałbyś się napisać coś większego?

Lepiej napisać dwa lub trzy duże projekty przestrzegając dobrych praktyk i dbając o ogólną czystość kodu niż 150 skryptów, które da się klepnąć w 10 minut. Bo ilość na nikim wrażenia nie zrobi.

komentarz 17 grudnia 2017 przez amb00 Bywalec (2,000 p.)

Bardzo chciałbym zrobić choć jeden na prawdę konkretny projekt, ale nie mam za bardzo pomysłów.

odpowiedź 16 grudnia 2017 przez Arkadiusz Waluk Ekspert (287,950 p.)

Ja wypowiem się tylko o tym jednym repozytorium z kodem PHP, bo na nim się znam.

Ten projekt akurat jest bardzo mały, ale ogólnie z rozwiązania w stylu wywoływania konkretnego pliku pod daną akcję się odchodzi. Tworzy się routing, wszystko kieruje na jeden wejściowy plik .php i to routing zarządza tym, co należy wywołać w samej aplikacji.

$connection = mysqli_connect("localhost", "root", "", "webnotes");

Ręczne definiowanie danych do bazy w każdym pliku to kiepski pomysł, co jak nagle będzie potrzeba ich zmiany? Trzeba zrobić to w każdym pliku, a tak po prostu zmianę wykonałoby się raz. Poza tym nie widzę żadnej obsługi ewentualnego błędu, jak nie uda się połączyć z bazą to co?

"insert into notes(note, lastModification, userId) values('".$_POST['note']."',

Zrobienie zapytania tak umożliwia bez żadnego wysiłku wykonanie ataku SQL injection - ktoś może Ci np. usunąć dane z bazy, albo wszystkie wyświetlić. Należy użyć bindowania wartości, aby się przed tym zabezpieczyć. No i ktoś może nie przesłać tej wartości w $_POST, a wtedy pojawi się ostrzeżenie - lepiej to obsłużyć, np. sprawdzić przed użyciem czy dana wartość istnieje albo użyć filter_input().

mysqli_close($connection);

Jeśli już bardzo chcesz zamykać połączenie (ja tego nie robię, nie miałem nigdy takiej potrzeby) to czemu robisz to tylko gdy $_SESSION['id'] istnieje? Gdy nie, to już nie ma potrzeby go zamykać? Było tak samo otwarte.

		$result = mysqli_query($connection, 'select userId from users where user="'.$_POST['user'].'" and password="'.$_POST['password'].'"');

Haseł w bazie nie wolno przechowywać czystym tekstem - powinny być zahaszowane. Patrz: password_hash() i password_verify().

) ENGINE=InnoDB DEFAULT CHARSET=latin1;

W bazie jakieś dziwne kodowanie, pewnie będzie problem z polskimi znakami.

Do tego cały kod nie przestrzega standardu zapisu PSR-2 - nie jest to oficjalny standard PHP, ale jest na tyle popularny, że spotkasz go prawie wszędzie.

Nie wiem w jakim celu wrzucasz ten kod na GitHuba - czy to tak po prostu aby coś było, czy żeby się pochwalić, czy co, ale osobiście to repozytorium bym usunął, gdyż kod zawiera poważne błędy jak niehaszowane hasła i jest podatny na ataki jak SQL injection. Poza tym jest bardzo prosty i niewiele robi, do chwalenia więc niespecjalnie się nadaje. Jeśli chcesz się chwalić to też nie bardzo rozumiem takiego zakresu: Python, C++ i PHP? Pokazując swojego GitHuba programiście jednego z języków raczej ciężko będzie mu się zorientować czym w ogóle się zajmujesz. Nie sądzę też, abyś zaimponował programiście np. PHP tym, że masz jakieś małe projekty np. w C++.

Nie chciałbym Cię zniechęcić, ale prosiłeś o wszelkie uwagi, a więc proszę :)

1	komentarz 16 grudnia 2017 przez efiku Szeryf (75,160 p.) TL;DR Weź się do roboty bo jest średnio, a może być lepiej. :D Jeśli to są projekty tupu by do CV było, to mogą bardziej odstraszać. Głównie patrzy się na historię commitów i ogólny standard kodu

1	komentarz 16 grudnia 2017 przez Ehlert Ekspert (212,670 p.) Zacząłem pisać odpowiedź dotyczącą kodu PHP i po chwili... *To nie ma sensu*.

komentarz 16 grudnia 2017 przez Arkadiusz Waluk Ekspert (287,950 p.)

@efiku no jeśli to miałoby być do CV to nawet na pewno tylko odstraszą

@Ehlert heh, może znajdziesz coś jeszcze czego nie wyłapałem, ale niewiele ogólnie kodu jest :p