Witam wszystkich. Dopiero raczkuję w PHP ale ostro wziąłem się do pracy i się uczę. Możecie mi powiedzieć czy ten kod jest poprawny pod względem składniowym i estetycznym?

[PHP] pobierz, plaintext 
<?php
 
		//Zmienne, które będą przechowywać dane z formularza
		$opony = $_POST['iloscopon'];
		$swiece = $_POST['iloscswiec'];
		$olej = $_POST['iloscoleju'];
		$ulica = $_POST['ulica_klienta'];
		$mieszkanie_numer = $_POST['numer_mieszkania_klienta'];
		$miasto = $_POST['miasto_klienta'];
		$plik_zamowienia = $_SERVER['DOCUMENT_ROOT'];
 
		//Utworzenie dodatkowych zmiennych i stałych
		$ilosc = 0;
		$cena_netto = 0.00;
		$centa_brutto = 0.00;
		$podatek_vat = 0.22;
		$data = date('H:ia, dS, F.');
		$adres = 'Ulica: '.$ulica."\t".'Numer Mieszkania: '.$mieszkanie_numer."\t".'Miasto: '.$miasto;
 
		define('CENAOPON', 200);
		define('CENASWIEC', 75.98);
		define('CENAOLEJU', 123.54);
 
		//Obliczenie wartości zamówienia i ilości zakupionych przedmiotów
		$ilosc = $opony + $swiece + $olej;
 
		$cena_netto = $opony * CENAOPON   +
					  $swiece * CENASWIEC +
					  $olej * CENAOLEJU;
 
		$cena_brutto = $cena_netto * (1 + $podatek_vat);
 
		//Odczytywanie i zapisywanie danych do pliku
		@ $wskaznik_pliku = fopen("$plik_zamowienia/zamowienia.txt", 'ab');
		if($ilosc > 0)
			{
				$ciag_wyjsciowy = $data."\t".$opony." opon\t".$swiece." świec zapłonowych\t".$olej." butelek oleju\t".$cena_netto."PLN\t".$cena_brutto."PLN\t".$adres."\r\n";
 
				@ $zapis_do_pliku = fwrite($wskaznik_pliku, $ciag_wyjsciowy, strlen($ciag_wyjsciowy));
 
				@ $zamkniecie_pliku = fclose($wskaznik_pliku);
			}
		//Wyświetlenie wyników zamówienia
		echo '<h1>Części Samochodowe Janka</h1>'."\n".'<br />'."\n".'<hr />'."\n";	
 
 
		if($ilosc > 0 && $wskaznik_pliku == TRUE)
			{
				echo '<h2>Wyniki Zamówienia</h2>';
				echo '<hr />';
			}
 
 
		if($opony > 0 && $wskaznik_pliku == TRUE)
			{
				echo '<p>Ilość Opon: '.$opony.'</p>';
			}
 
 
		if($swiece > 0 && $wskaznik_pliku == TRUE)
			{
				echo '<p>Ilość Świec: '.$swiece.'</p>';
			}
 
 
		if($olej > 0 && $wskaznik_pliku == TRUE)
			{
				echo '<p>Ilość Oleju: '.$olej.'</p>';
			}
 
 
		if($ilosc > 0 && $wskaznik_pliku == TRUE)
			{
				echo '<p>Ilość Produktów: '.$ilosc.'</p>';
				echo '<hr />';
				echo '<p>Cena Netto: '.number_format($cena_netto, 2).'</p>';
				echo '<p>Cena Brutto: '.number_format($cena_brutto, 2).'</p>';
				echo '<hr />';
				echo '<p>Twoje zamówienie zostało przyjęte i zapisane w anszej bazie o godzinie: '.date('H:ia, dS, F.').' W ciągu 7 dni roboczych powinieneś otrzymać zamówiony/e towar/y. Dziękujemy za skorzystanie z usług z naszego sklepu</p>';
			}
 
 
		if($ilosc == 0 && $wskaznik_pliku == TRUE)
			{
				echo '<h1>Na poprzedniej stronie nie zostało złożone żadne zamówienie</h1>';
				exit;
			}
 
		if(!$wskaznik_pliku || !$zapis_do_pliku || !$zamkniecie_pliku)
			{
				echo '<p><b>Przepraszamy lecz państwa zamówienie nie może zostać przyjęte w danej chwili</b></p>'."\n".'</body>'."\n".'</html>';
				exit;
			}
	?>
[PHP] pobierz, plaintext 

1) Zamiast @ przed otwieraniem i zapisem do pliku lepiej zrób jakąś obsługę błędów.
2) Zamiast == TRUE stosuj lepiej === TRUE
3) Zamiast exit w przypadku niepowodzenia zastosowałbym cofnięcie do poprzedniej strony.

A tak to wszystko raczej ok.

PS. W tym zdaniu Przepraszamy lecz państwa zamówienie nie może(...) powinien być przecinek: Przepraszamy, lecz(...).

Dzięki za odpowiedź i wytknięcie mi błędów

To nie tyle co błędy a raczej drobne niedociągnięcia.
W linijce 79 masz literówkę. Popraw anszej na naszej. A jak już mamy być super dokładni to po dacie, czyli przed W ciągu 7 dni roboczych(...) powinna być kropka

Idąc za przykłądem sadistic sona walnij 100 znaków równości żeby parser wiedział o co ci chodzi. Trzy znaki równości używa się jeżeli są zmienne tego samego typu; czyli np liczba-liczba, tekst-tekst, ciąg znakowy-ciąg znakowy trzy znaki równości interpretuje tylko do wersji pHP 4. Do widzenia...

Ps.trzy znaki = to jeden bit więcej, a to szkodzi stronie skoro goooooooogle patrzy na szybkość wczytywania się strony. Jeżeli stosowałby to w całym kodzie to sobie policz ile więcej zajmują pliki wyjściowe.

Na zakończenie mój cytat:"Jeżeli myślisz, że to zbyt proste zrób to, inni będą się z ciebie śmiać dopóki nie uświadomią sobie swojego błędu"

worek widzę masz ostatnio fazę gadania od rzeczy....

ałć x 1
ałć x 2
ałć x 3

@crazy_programmer dla własnego dobra przyjmij, że w tym temacie nie pisał worek.


cytat roku. Ja mam kolejny, mój:
jeśli myślisz, że 2+2 = 4, napisz to. Inni będą się z Ciebie śmiać dopóki nie uświadomią sobie swojego błędu


@sadistic_son ale patrz co on przyrównuje. On przyrównuje wynik fopen do true. a fopen zwraca resource, wiec resource === true da fałsz

w tym przypadku, jeśli chcesz używać 3 znaków porównania to powinno być !== false

W ogóle cały ten kod:

[PHP] pobierz, plaintext 
if($ilosc > 0 && $wskaznik_pliku == TRUE)
if($opony > 0 && $wskaznik_pliku == TRUE)
if($swiece > 0 && $wskaznik_pliku == TRUE)
if($olej > 0 && $wskaznik_pliku == TRUE)
if($ilosc > 0 && $wskaznik_pliku == TRUE)
[PHP] pobierz, plaintext 

Lepiej zapisać poprostu tak:

[PHP] pobierz, plaintext 
if ($wskaznik_pliku){
if($ilosc > 0) 
if($opony > 0)
if($swiece > 0)
if($olej > 0)
if($ilosc > 0)
}
[PHP] pobierz, plaintext 

krócej i czytelniej i mniej sprawdzania

Hmmm... Moim zdaniem mozna by mocniej rozdzielić struktury danych skryptu oraz logikę od widoku. Czemu?

[PHP] pobierz, plaintext 
if($olej > 0 && $wskaznik_pliku == TRUE) {
    echo '<p>Ilość Oleju: '.$olej.'</p>';
}
[PHP] pobierz, plaintext 

W chwili gdy nie ma wysłanych danych postem to do $olej niejawnie zostanie przypisane zero, a dostaniesz zapewne przy okazji warning, że iloscoleju w zmiennej POST nie istnieje (klasyczny index undefined)

Do tego pomyślałbym nad VAT, bonieważ obecny sposób narzuca jeden wszystkim produktom, co nie musi być prawdą. Dla każdego powinieneś mieć możliwość nadania osobno.

Zrób sobie strukturę (obiekt lub tablicę), gdzie masz domyślne wartości pól i w razie istnienia zmiennych w POST, zmień je. Dzięki temu będziesz mógł połaczyć strone formularza oraz informacyjną zamówienia w jedną. Co sprawi, że błędy bedziesz mógł pod danym polem zamieścić do poprawki zamiast kazać mu się cofać do formularza by poprawić byki, a dodatkowo złożenie jednego zamówienia pozwoli bez przechodzenia znowu do strony formularza złożyc kolejne. Im mniej klikania tym dla usera lepiej i wygodniej. Worka nie słuchaj, bo jego rady faktycznie są takie, jakby urwał się z choinki Skomentuję tylko jedno dodatkowo, to co nospor opatrzył jako "ałć x 2": jak myślisz... porównanie zgodności typów będzie szybsze czy wolniejsze od niejawnej konwersji z jednego do innego i ponowne porównanie?

Jeśli połączysz obie strony (tj. formularz i informacyjną), to generowanie strony na samym końcu, a całe działania wrzuć do "jednego worka", przed tworzeniem strony jako html.

Poza tym uwzględniając poprawki nospora do tego co napisał sadistic_son i moje sugestie, myślę że powinna Ci się w miarę wszystko już rozjaśnić i ponownie cytując nospora... Nie słuchaj worka, a co więcej to z jego rad możesz sie pośmiać, co zresztą ciutkę wyżej chyba Ci wykazałem na drugim przykładzie wytknietym przez nospora.

Tak jak już ktoś napisał dając "Zamiast == TRUE stosuj lepiej === TRUE", skrypt nie będzie działał poprawnie Dzięki wszystkim za podpowiedzi. Dopeiro się uczę więc jeszcze wszystkiego nie ogarniam i trochę czasu minie zanim napiszę czysty kod Co do worka to faktycznie jego rady są śmieszne jak i cały on