jak można to zoptymalizować, próbka kodu na rozmowę o pracę Opcje

[damianooo]

Witam,

Szukam obecnie pracy w webmasterce ... mam już pewne doświadczenie ale w tworzeniu kodu samodzielnie, ponieważ prowadziłem własną działalność gospodarczą. Teraz aplikując do pracy w dowolnej firmie okazuje się że występują pewne standardy w tworzeniu kodu - zaznaczam że nie dziwi mnie ten fakt wogóle ( jest to oczywiste dla mnie ).
Przejdę do rzeczy. Poniżej umieszczę jeden ze skryptów i prosiłbym o surową krytykę oraz o podpowiedzenie mi co można zmienić, poprawić, zoptymalizować w tym kodzie, czego się robić nie powinno itd. Będę serdecznie wdzięczny. Od razu zaznaczę że wiem iż kod css powinien być zawsze w innym pliku więc tej uwagi proszę nie zamieszczać (IMG:style_emoticons/default/smile.gif) ....

Kto najwięcej mi pomoże i najwięcej wymieni błędów dostanie status "Pomógł" (IMG:style_emoticons/default/smile.gif)

Poniżej podam też odpowiedź mailową od jednej z firm po obejrzeniu m.in. poniższego kodu:
"Przesłane próbki kodu nie spełniają jednak naszych oczekiwań i są raczej dalekie od naszych standardów. Nie stosuje Pan funkcji, wszystko jest napisane jednym ciągiem. Zamiast wykonać pętlę kopiuje Pan kod."

[PHP] pobierz, plaintext 
	<div style="float:left;padding:0;margin:0;height:20em;width:60em;">
 
		<div style="float:left;padding:0;margin:0;height:20em;width:2em;">
		</div>
 
		<div style="float:left;padding:0;margin:0;height:20em;width:56em;">
 
		<?php
 
			if(isset($_SESSION['nazwa']))
			{
				try
				{
					$sql_czyjuzjest = "SELECT count(*) as ile FROM test_specjalny.typ WHERE (id_gracz = ".$_SESSION['id'].") AND (id_kolejka = ".$numer_kolejka.")";
					$result_czyjuzjest = @mysql_query($sql_czyjuzjest);
					$row_czyjuzjest = @mysql_fetch_array($result_czyjuzjest);
					$czyjuzjest = $row_czyjuzjest['ile'];
 
					if( $result_czyjuzjest === false )
					throw new Exception("błąd zapytania");
				}
				catch( Exception $e ) 
				{
					echo "&nbsp;chwilowo nie można wyświetlić danych ... ";
				}
 
 
				if($czyjuzjest != 0)
				{
					$infotyp = "<span style='color:black;font-size:x-small;'>STATUS - </span><span style='color:red;font-size:x-small;'>WYTYPOWAŁ</span>";
				}
				else
				{
					$infotyp = "<span style='color:black;font-size:x-small;'>STATUS - </span><span style='color:red;font-size:x-small;'>NIE TYPOWAŁ</span>";
				}
			}
			else
			{
				echo "";
			}
 
 
 
 
			if (!isset($_POST['submit']))
			{
				echo "<form action='ligatyperow.php?a=o' method='post'>";
 
					$vcv = 1;
 
					try
					{
						$sql_mecz = "SELECT id,gospodarz,gosc,opis,termin FROM test_specjalny.mecz WHERE (id_kolejka = 1) AND (status_m = 1) ORDER BY pozycja ASC";
						$result_mecz = @mysql_query($sql_mecz);
						while ($row_mecz = @mysql_fetch_array($result_mecz))
						{
							$id = $row_mecz['id'];
							$gospodarz_mecz = $row_mecz['gospodarz'];
							$gosc_mecz = $row_mecz['gosc'];
							$opis_mecz = $row_mecz['opis'];
							$termin_mecz = $row_mecz['termin'];
 
							echo "<input type='hidden' name='idmecz[]' value='".$id."'>";
 
 
							echo "<span class='dd' >";
 
 
								if( $vcv % 2 )
								{
									echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:56em;text-align:right;background-color:#a6a6a6;'>";
 
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:24.5em;text-align:right;'>";
											echo "<span style='font-size:x-small;'>(".$opis_mecz.")</span>&nbsp;&nbsp;".$gospodarz_mecz."";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:1em;'>";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:2em;'>";
											echo "<input type='text' name='gsp[]'  maxlength='1' value='' dir='rtl' style='background-color:transparent;color:black;width:2em;'>";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:1em;'>";
										echo "<img src='img/pileczka.png' alt='pileczka' />";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:2em;'>";
											echo "<input type='text' name='gos[]'  maxlength='1' value='' dir='ltr' style='background-color:transparent;color:black;width:2em;'>";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:1em;'>";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:24.5em;text-align:left;'>";
											echo "".$gosc_mecz."&nbsp;&nbsp;<span style='font-size:x-small;'>(".$termin_mecz.")</span>";
										echo "</div>";
 
									echo "</div>";
								}
								else
								{
									echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:56em;text-align:right;background-color:#e5e2df'>";
 
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:24.5em;text-align:right;'>";
											echo "<span style='font-size:x-small;'>(".$opis_mecz.")</span>&nbsp;&nbsp;".$gospodarz_mecz."";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:1em;'>";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:2em;'>";
											echo "<input type='text' name='gsp[]'  maxlength='1' value='' dir='rtl' style='background-color:transparent;color:black;width:2em;'>";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:1em;'>";
										echo "<img src='img/pileczka.png' alt='pileczka' />";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:2em;'>";
											echo "<input type='text' name='gos[]'  maxlength='1' value='' dir='ltr' style='background-color:transparent;color:black;width:2em;'>";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:1em;'>";
										echo "</div>";
										echo "<div style='float:left;padding:0;margin:0;height:1.5em;width:24.5em;text-align:left;'>";
											echo "".$gosc_mecz."&nbsp;&nbsp;<span style='font-size:x-small;'>(".$termin_mecz.")</span>";
										echo "</div>";
 
									echo "</div>";
								}
 
							echo "</span>";
 
							$vcv++;
						}
 
						if( $result_mecz === false )
						throw new Exception("błąd zapytania");
					}
					catch( Exception $e ) 
					{
						echo "&nbsp;chwilowo nie można wyświetlić danych ... ";
					}
 
					echo "<div style='float:left;padding:0;margin:0;width:56em;height:1.5em;text-align:right;'>";
						echo $infotyp;
					echo "</div>";
 
					echo "<div style='float:left;padding:0;margin:0;width:56em;height:1.5em;text-align:right;'>";
 
						if(isset($_SESSION['nazwa']))
						{
							echo "<input type='submit' name='submit' value='Typuję'  style='width:6em;font-family:arial'>";
						}
						else
						{
							echo "<img src='img/typ_button.png' />";
						}
 
					echo "</div>";
 
 
 
 
				echo "</form>";
 
			}
			else
			{
				$liczbaRekordow = count($_POST['gsp']);
 
 
				if ($czyjuzjest != 0)
				{
					echo "<center>Uwaga! Już typowałeś w tej kolejce. <a href='ligatyperow.php?a=o' style='font-size:small;'>Powrót</a></center>";
				}
				else
				{
					for($i = 0; $i < $liczbaRekordow; $i++) 
					{
						$mecz_01_01 = $_POST['idmecz'][$i];
						$mecz_02_01 = strip_tags($mecz_01_01);
						$mecz_03_01= trim($mecz_02_01);
						$mecz_04_01 = stripslashes($mecz_03_01);
						$mecz = htmlspecialchars($mecz_04_01, ENT_QUOTES);
 
						$gsp_01_01 = $_POST['gsp'][$i];
						$gsp_02_01 = strip_tags($gsp_01_01);
						$gsp_03_01= trim($gsp_02_01);
						$gsp_04_01 = stripslashes($gsp_03_01);
						$gsp = htmlspecialchars($gsp_04_01, ENT_QUOTES);
 
						$gos_01_01 = $_POST['gos'][$i];
						$gos_02_01 = strip_tags($gos_01_01);
						$gos_03_01= trim($gos_02_01);
						$gos_04_01 = stripslashes($gos_03_01);
						$gos = htmlspecialchars($gos_04_01, ENT_QUOTES);
 
 
						if (($gsp == '' ) OR ($gos == '' ))
						{
							echo "<center>Uwaga! Musisz wypełnić wszystkie pola. <a href='ligatyperow.php?a=o' style='font-size:small;'>Powrót</a></center>";
						}
						else
						{
							if (	((preg_match('/^[0-9]$/D', $gsp)) != true) || ((preg_match('/^[0-9]$/D', $gos)) != true)  )
							{
								echo "<center>Uwaga! W polach mogą pojawiać się tylko liczby. <a href='ligatyperow.php?a=o' style='font-size:small;'>Powrót</a></center>";
							}
							else
							{
								try 
								{
									$sql = "INSERT INTO test_specjalny.typ (typ_gospodarz,typ_gosc,data_typ,id_mecz,id_gracz,id_kolejka) VALUE (".$gsp.",".$gos.",'".$data_typu."',".$mecz.",".$_SESSION['id'].",".$numer_kolejka.")";
									$wynik = @mysql_query($sql);
 
									if( $wynik === false )
									throw new Exception("błąd zapytania");
								}
								catch( Exception $e ) 
								{
									echo "&nbsp;chwilowo nie można dodać danych ... ";
								}
 
								header("Location:ligatyperow.php?a=o");
							}
						}
					}
				}
			}
 
			?>
 
 
		</div>
 
		<div style="float:left;padding:0;margin:0;height:20em;width:2em;background-color:white;">
		</div>
 
	</div>
[PHP] pobierz, plaintext 

dziękuję, pozdrawiam

Ten post edytował damianooo 19.06.2012, 11:14:29

[tehaha]

Tu nie ma co optymalizować....Żadna normalna firma nie przyjmie takiego kodu, ponieważ jest trudny do zorientowania się i przez to, że wszystko jest razem namieszane, jakiekolwiek zmiany są ciężkie i bardzo czasochłonne, a w przypadku większych przeróbek prościej będzie skasować i napisać to od nowa, a czas to pieniądz.

Oprócz tego co napisał kolega wyżej:

1. Brak tutaj jakiejkolwiek architektury aplikacji, aplikacja powinna być podzielona na warstwy i rodzielać zadania na różne komponenty aplikacji
2. zamiast funkcji mysql_ lepiej jest używać PDO
3. kod html powinien być oddzielony od kodu php
4. to co jeszcze mnie osobiście odraża to polska nomenklatura, która zaburza czytelność kodu, kod php, html, css jest po angielsku więc całość jest czytelniejsza, jeśli stosujesz nomenklaturę angielską
5. Nielogiczne nazywanie zmiennych jak patrzę w kod to nie wiadomo o co chodzi z takimi zmiennymi :$gsp_01_01, $gos_02_01, $sql_czyjuzjest, zmienne powinny mieć proste symboliczne nazwy, które wskazują co to jest
6. Zbyt dużo niezorgranizowanych zmiennych, np. to co jest między linijkami 170-190, można by zorganizować w ładną tablicę wielowymiarową i uporządkować
7. Jeżeli chcesz się tym zająć na poważnie, to musisz nauczyć się programować obiektowo, jeżeli chcesz w miarę szybko podjąć pracę to opanuj jakiś framework, który już na starcie rozwiąże bardzo wiele kwestii, o któych nie masz pojęcia i narzuci Tobie architekturę aplikacji
8. Stosujesz różne standardy nazewnictwa zmiennych, raz camelCase, raz z podkreślnikiem, a czasami to nie wiadomo co, to powinno być ujednolicone