Jak w temacie co byście w tym kodzie zmienili tylko nie oceniajcie kodu java script bo wiem, że ten kod java script jest marny.
Teraz wrzuciłem na Githuba.
tu jest link
Tym razem uprzedze Cie @Shalom :D
Myślę, że kod daje radę. Szczególnie ten fragment muszę sobie gdzieś zapisać:
if ($czas_dodania < 60) $this->data = "Mniej niż minutę temu";
else if ($czas_dodania < 120) $this->data = "Mniej niż 2 minuty temu";
else if ($czas_dodania < 180) $this->data = "Mniej niż 3 minuty temu";
else if ($czas_dodania < 240) $this->data = "Mniej niż 4 minuty temu";
else if ($czas_dodania < 300) $this->data = "Mniej niż 5 minut temu";
else if ($czas_dodania < 360) $this->data = "Mniej niż 6 minut temu";
else if ($czas_dodania < 480) $this->data = "Mniej niż 8 minut temu";
else if ($czas_dodania < 600) $this->data = "Mniej niż 10 minut temu";
else if ($czas_dodania < 900) $this->data = "Mniej niż 15 minut temu";
else if ($czas_dodania < 1200) $this->data = "Mniej niż 20 minut temu";
else if ($czas_dodania < 1500) $this->data = "Mniej niż 25 minut temu";
else if ($czas_dodania < 1900) $this->data = "Około pół godziny temu";
else if ($czas_dodania < 2400) $this->data = "Około 40 minut temu";
else if ($czas_dodania < 2700) $this->data = "Około 45 minut temu";
else if ($czas_dodania < 3000) $this->data = "Około 50 minut temu";
else if ($czas_dodania < 3300) $this->data = "Około 55 minut temu";
else if ($czas_dodania <= 3600) $this->data = "Około Godzinę temu";
To ja spróbuję dać jakiś konstruktywny komentarz.
- index.php
tutaj najbardziej rzuca się w oczy ten kod https://github.com/tomi0001/strona/blob/master/index.php#L116 Serio nie można zrobić tego choćby w pętli? Zrób sobie klasę, w której będziesz trzymać informacje o Menu. (jaki url -> jaki plik ładujemy).
https://github.com/tomi0001/strona/blob/master/index.php#L31 Nie prościej tak?
if (!$nowy_user->sprawdz_czy_zalogowany())
https://github.com/tomi0001/strona/blob/master/index.php#L28 nie używaj mysql_escape_string tylko http://php.net/manual/pl/function.mysql-real-escape-string.php a najlepiej przerzuć się na PDO -> https://pl.wikibooks.org/wiki/PHP/Biblioteka_PDO
- panel.php
https://github.com/tomi0001/strona/blob/master/strony/panel.php#L9 czytelniej jest zamknąć PHP ?> i kodu HTML nie wyświetlać w ten sposób.
https://github.com/tomi0001/strona/blob/master/strony/zaloguj.php#L2 "czy"? Co to znaczy?
Unikaj takich ifów -> https://github.com/tomi0001/strona/blob/master/clasy/data.php#L12 to TRAGICZNIE wygląda. Spokojnie możesz to zastąpić jakąś pętlą.
To tak z grubsza. Więcej mi się nie chce :)
PS. Folder o naszwie clasy? Serio? Po jakiemu to? Używaj tylko angielskiego.
od razu, rzucił mi się w oczy ponglish: "clasy" oraz "dodaj_kome".
Krótka odpowiedź: Zmieniłbym ** dużo ** :D
Długa odpowiedź:
-
print
to funkcja, więc powinno się ją pisać z nawiasami. - zamień
$nowy_user = new user;
$data = new data;
$mysql = new baza;
na
$nowy_user = new user();
$data = new data();
$mysql = new baza();
bo nikt nie będzie wiedział o co chodzi.
- Biedny @pol90 wszyscy się na niego rzucają a on nie wie o co chodzi.
Takie rzeczy jak na przykłąd od tąd do linii 185 mają MASĘ powtórzeń. W programowaniu nie lubimy powtórzeń. Trzeba je eliminować, np
można by z łatwością zrobić funkcję
function checkPozycja($name, $displayName) {
if ($_GET["pozycja"] == $name) {
include ("./strony/$name.php");
print ("<title>$displayName</title>");
}
}
i teraz całe te Twoje powtózenia można zapisać tak
checkPozycja('aktu', 'Aktualności');
checkPozycja('logowanie', 'Logowanie');
checkPozycja('zaloguj', 'Logowanie');
checkPozycja('wyloguj', 'Wylogowywanie');
checkPozycja('user', 'Użytkownicy');
checkPozycja('panel', 'Panel Administracyjny');
checkPozycja('dodaj_zdje', 'Zapisz zdjęcie');
checkPozycja('gale', 'Galeria');
checkPozycja('gale_kome', 'Komentarze');
checkPozycja('statyu', 'Aktualności');
checkPozycja('dodaj_aktu1', 'Aktualności');
checkPozycja('dodaj_aktu2', 'Aktualności');
checkPozycja('dodaj_aktu_kome', 'Aktualności');
checkPozycja('czat', 'Czat');
- Po prostu ** nie wyświetlanie ** linka do panelu admina raczej nie jest zbyt dobrym zabezpieczeniem, nie sądzisz? :) (na razie może zostać, tylko żebyś wiedział, nie? :D )
if ($nowy_user->typ == "root") {
print ("<a href=index.php?pozycja=panel>");
- Java to nie JavaScript ;)
<SCRIPT TYPE="text/javascript" SRC="java.js"></SCRIPT>
-
Jeżeli chciałbyś popracować nad czytelnością plików które mają dużo html a mało php, może mógłbyś się zainteresować inną formą osadzania kodu (php ma tak jakby dwie) Jedna ta w której piszesz a druga np taka
http://pastebin.com/1ZRFMrBs
Gdzie używa się dwukropków zamiast klamerek (można tego użyć do if,elseif / for / foreach), może Ci się spodoba gdybyś chciał coś więcej. -
Jak chcesz żeby Twoja strona przechodziła przez walidatory html to niestety samo to nie wystarczy
<link href="./style.css" rel="stylesheet" type="text/css" />
<meta content="text/html; charset=utf-8" http-equiv="content-type">
Spróbuj osadzić kod w takich znacznikach html
<!doctype html>
<html>
<head>
<title>Moja strona</title>
<link href="./style.css" rel="stylesheet" type="text/css" />
<meta content="text/html; charset=utf-8" http-equiv="content-type">
</head>
<body>
<!-- Tutaj Twój html (czyli w sumie wszystko co masz w index.php) -->
</body>
</html>
- Ten kod:
private function oblicz_dzien_tygodnia($dzien_tygodnia) {
if ($dzien_tygodnia == "Monday") return "Poniedziałek";
else if ($dzien_tygodnia == "Tuesday") return "Wtorek";
else if ($dzien_tygodnia == "Wednesday") return "Środę";
else if ($dzien_tygodnia == "Thursday") return "Czwartek";
else if ($dzien_tygodnia == "Friday") return "Piątek";
else if ($dzien_tygodnia == "Saturday") return "Sobotę";
else return "Niedziele";
}
Można by fajnie zamienić na taki
private function oblicz_dzien_tygodnia($dzien) {
return strstr($dzien, [
"Monday" => "Poniedziałek",
"Tuesday" => "Wtorek",
"Wednesday" => "Środę",
"Thursday" => "Czwartek",
"Friday" => "Piątek",
"Saturday" => "Sobotę",
"Sunday" => "Niedziele"
]);
}
- Jezus maria co to? :O
$baza->query("insert into galeria(sciezka,data) values ('$nazwa_pliku','$data') ");
Wiesz co to sql injection? Pomyśl co by było gdyby ktoś do Twojej zmiennej $nazwa_pliku wpisał takie coś:
dupa'); DROP TABLE galeria; --
.
Twoje query wyglądałoby tak:
$baza->query("
insert into galeria(sciezka,data) values ('dupa');
DROP TABLE galeria;
--','$data')
");
Nie muszę chyba mówić jakie są konsekwencje? Dla przypomnienia --
w SQL to komentarz.
- A ten kod
if($rozszerzenie !== "jpg" &&
$rozszerzenie !== "gif" &&
$rozszerzenie !== "png") {
return false;
}
// ...
if($imageFileType != "jpg" && $imageFileType != "png" && $imageFileType != "jpeg"
&& $imageFileType != "gif" ) {
return 5;
}
Można by zamienić na
if (!in_array($rozszerzenie, ['jpg', 'gif','png'])) {
return false;
}
// ...
if(!in_array($imageFileType, ["jpg", "png", "jpeg", "gif"]) {
return 5;
}
- Zamienić
if(imagejpeg($canvas, $plik2, 70)) {
return true;
} else {
return false;
}
na to
return imagejpeg($canvas, $plik2, 70);
- Noi jeszcze, taki refactor szybki, pokazuje jak można by lepiej zrobić Twoją klasę
os.php
.
<?php
class os
{
function os($text)
{
$text = strtolower($text);
return $this->findInArray($text, $this->operatingSystemNames, "Nieznany");
}
function browser($text) {
$text = strtolower($text);
return $this->findInArray($text, $this->browserNames, "Nieznana");
}
private $operatingSystemNames = [
"android" => "Linux Android",
"linux" => "Linux",
"windows nt 10" => "Windows 10",
"windows nt 6.3" => "Windows 8.1",
"windows nt 6.2" => "Windows 8",
"windows nt 6.1" => "Windows 7",
"windows nt 6.0" => "Windows Vista",
"windows nt 5.2" => "Windows 2003 Server",
"windows nt 5.1" => "Windows XP",
"windows nt 5.0" => "Windows 2000",
"windows nt 4" => "Windows NT 4",
"win98 " => "Windows 98",
"windows 98" => "Windows 98",
"win95" => "Windows 95",
"windows 95" => "Windows 95",
"win9x4.9" => "Windows ME",
"freebsd" => "FreeBSD",
"desktopbsd" => "DesktopBSD",
"solaris" => "Solaris",
"netbsd" => "NetBSD",
"qnx" => "QNX";
"mac os" => "Mac OS",
"macos" => "Mac OS",
];
private $browserNames = [
'opera' => "Opera",
'firefox' => 'Mozilla Firefox',
'msie' => 'Internet Exploler',
'applewebkit' => 'AppleWebKit'
'links' => 'Links',
'chrome' => 'Chrome',
'konqueror' => 'Konqueror'
];
private function findInArray($thisName, $array, $notFound)
{
foreach ($array as $key => $value) {
if (strsrt($thisName, $key)) return $value;
}
return $notFound;
}
}
?>
-
Klasy
baza
wmysql.php
nawet nie chce mi się czytać bo jest tak brzydka że jap**** :C -
Dzieki za komentarz @mad_penguin Jasne że tak, chociaż nie wiem czy to faktycznie poprawi czytelność.
checkMany([
'aktu' => 'Aktualności',
'logowanie' => 'Logowanie',
'zaloguj' => 'Logowanie',
'wyloguj' => 'Wylogowywanie',
'user' => 'Użytkownicy',
'panel' => 'Panel Administracyjny',
'dodaj_zdje' => 'Zapisz zdjęcie',
'gale' => 'Galeria',
'gale_kome' => 'Komentarze',
'statyu' => 'Aktualności',
'dodaj_aktu1' => 'Aktualności',
'dodaj_aktu2' => 'Aktualności',
'dodaj_aktu_kome' => 'Aktualności',
'czat' => 'Czat'
]);
function checkMany($all)
{
foreach ($all as $name => $displayName) {
if ($_GET["pozycja"] == $name) {
include ("./strony/$name.php");
print ("<title>$displayName</title>");
}
}
}