Forum PHP.pl > [MySQL][PHP]Czystość kodu

Pomoc - Szukaj - Użytkownicy - Kalendarz

Wertas

15.07.2012, 18:54:07

Witam. Z góry chcę podkreślić, że jestem słaby w pisaniu dobrego i czystego kodu ponieważ, powtarzam go na okrągło. Temu chciałbym aby ktoś pokazał mi jak można to zrobić, dużo przy tym się nauczę wejdzie mi to w nawyk.

Teraz przejdźmy do rzeczy.

Oto SQL bazy danych

[SQL] pobierz, plaintext 
CREATE TABLE `items` (
  `id` int(10) NOT NULL AUTO_INCREMENT,
  `Strength` int(11) NOT NULL,
  `Dexterity` int(11) NOT NULL,
  `Intelligence` int(11) NOT NULL,
  `Vitality` int(11) NOT NULL,
  `Lok` int(11) NOT NULL,
  `Lph` int(11) NOT NULL,
  `Lphg` int(11) NOT NULL,
  `Lifesteal` int(11) NOT NULL,
  `Additional Life` int(11) NOT NULL,
  `Lps` int(11) NOT NULL,
  `IArcane` int(11) NOT NULL,
  `ICold` int(11) NOT NULL,
  `IFire` int(11) NOT NULL,
  `IHoly` int(11) NOT NULL,
  `ILightning` int(11) NOT NULL,
  `IPoison` int(11) NOT NULL,
  `IWeapon` int(11) NOT NULL,
  `AttackSpeed` int(11) NOT NULL,
  `Increased Minimum Damage` int(11) NOT NULL,
  `Increased Maximum Damage` int(11) NOT NULL,
  `CHD` int(11) NOT NULL,
  `Wd` int(11) NOT NULL,
  `Chc` int(11) NOT NULL,
  `Bdmgvsel` int(11) NOT NULL,
  `CBlind` int(11) NOT NULL,
  `CChill` int(11) NOT NULL,
  `CFear` int(11) NOT NULL,
 ...... itd
  UNIQUE KEY `id` (`id`),
  KEY `id_2` (`id`)
) ENGINE=InnoDB  DEFAULT CHARSET=latin1;
[SQL] pobierz, plaintext

Ogólnie skrypt jest przeznaczony do wpisywania wartości w te pola by ludzie mogli zobaczyć co możemy zaoferować.

Tutaj jest przykład mojej bezmyślności i niedołęstwa

1)Fragment kodu

[PHP] pobierz, plaintext 
if($_REQUEST['do'] == 'list')
	{ 
                 echo '<table border="1"><thead>
                        <th>id</th>
                        <th>Strength</th>
                        <th>Dexterity</th>
                        <th>Intelligence</th>
                        <th>Vitality</th>
                        <th>Lok</th>
                        <th>Lph</th>
                        <th>Lphg</th>
                        <th>Lifesteal</th>
                        <th>Additional Life</th>
                        <th>Lps</th>
                        <th>IArcane</th>
                        <th>ICold</th>
                        <th>IFire</th>
                        <th>IHoly</th>
                        <th>ILightning</th>
                        <th>IPoison</th>
                        <th>IWeapon</th>
                        <th>AttackSpeed</th>
                        <th>Increased Minimum Damage</th>
                        <th>Increased Maximum Damage</th>
                        <th>CHD</th>
                        <th>Wd</th>
                        <th>Chc</th>
                        <th>Bdmgvsel</th>
                        <th>CBlind</th>
                        <th>CChill</th>
                        <th>CFear</th>
                        <th>CFreeze</th>
                        <th>CImmobilize</th>
                        <th>CKnockback</th>
                        <th>CSlow</th>
                        <th>CStun</th>
                        <th>Ctbleed</th>
                        <th>Bleeddmg</th>
                        <th>Gf</th>
                        <th>Mf</th>
                        <th>Exp</th>
                        <th>Indestructible</th>
                        <th>Movement Speed</th>
                        <th>Yardspick</th>
                        <th>Reduced Level Requirement</th>
                        <th>Sock1</th>
                        <th>Sock2</th>
                        <th>Sock3</th>
                        <th>Resistance All</th>
                        <th>RArcane</th>
                        <th>RCold</th>
                        <th>RFire</th>
                        <th>RHoly</th>
                        <th>RLightning</th>
                        <th>RPhysical</th>
                        <th>RPoison</th>
                        <th>Increased Armor</th>
                        <th>CCred</th>
                        <th>Meleeatd</th>
                        <th>Reduced Damagefeli</th>
                        <th>Chance to blockomshield</th>
                        <th>Chance to blockomattr</th>
                        <th>Block Amount</th>
                        <th>Type</th>
                        <th>Price</th>
                        <th>Seller</th>
                    </thead>';
            WHILE ($row = mysql_fetch_array($result)){
               echo '
                <tr><td>'.$row['id'].'</td> 
                <td>'.$row['Strength'].'</td> 
                <td>'.$row['Dexterity'].'</td> 
                <td>'.$row['Intelligence'].'</td>
                <td>'.$row['Vitality'].'</td>
                <td>'.$row['Lok'].'</td> 
                <td>'.$row['Lph'].'</td> 
                <td>'.$row['Lphg'].'</td> 
                <td>'.$row['Lifesteal'].'</td> 
                <td>'.$row['Additional Life'].'</td> 
                <td>'.$row['Lps'].'</td> 
                <td>'.$row['IArcane'].'</td> 
                <td>'.$row['ICold'].'</td> 
                <td>'.$row['IFire'].'</td> 
                <td>'.$row['IHoly'].'</td> 
                <td>'.$row['ILightning'].'</td> 
                <td>'.$row['IPoison'].'</td> 
                <td>'.$row['IWeapon'].'</td> 
                <td>'.$row['AttackSpeed'].'</td> 
                <td>'.$row['Increased Minimum Damage'].'</td> 
                <td>'.$row['Increased Maximum Damage'].'</td> 
                <td>'.$row['CHD'].'</td> 
                <td>'.$row['Wd'].'</td> 
                <td>'.$row['Chc'].'</td> 
                <td>'.$row['Bdmgvsel'].'</td> 
                <td>'.$row['CBlind'].'</td> 
                <td>'.$row['CChill'].'</td> 
                <td>'.$row['CFear'].'</td> 
.... itd do każdego ze schematu SQL
                </tr>';
            }
            echo '</table>';
        }
[PHP] pobierz, plaintext

Jak to w ogóle wygląda? Czy macie jakiś pomysł jak bardzo to skrócić?

Nie wiem czy jest szansa stworzenia jakiegoś PDO czy czegoś tam w celu wywoływania tego samego "małego" kodu na innych podstrona gdyż chcę zrobić do tego opcję edytowania,usuwania więc kod zajął by kilka linijek.....

Kod jest taki obszerny i beznadziejny, że nie ma miejsca na wklejenie go.

2)Fragment kodu

[PHP] pobierz, plaintext 
echo '<table cellpadding="0" cellspacing="0" border="0" class="display" id="example">
	<thead>
	<tr align="center">
		<th>Item Number</th>
		<th>Item Name</th>
		<th>Stats</th>
		<th>Socketed</th>
		<th>Price</th>
		<th>Seller</th>
		<th>Status</th>
	</tr>
	</thead>
	<tbody>';
while($row = mysql_fetch_array($result))
 {
if ($row['Strength'] == '0') { echo '';} else {  $dataStrength ='<li class=d3-color-blue><p style="color:#6969FF;"><span class=value>+ '.$row['Strength'].'</span> Strength</p></li>'; }
if ($row['Dexterity'] == '0') { echo '';} else { $dataDexterity = '<li class=d3-color-blue><p style="color:#6969FF;"><span class=value>+ '.$row['Dexterity'].'</span> Dexterity</p></li>'; }
..............
[PHP] pobierz, plaintext

I tak dalej do każdego pola ze schematu SQL

a potem wywołane to jest

[PHP] pobierz, plaintext 
$attributedump = '
'.@$dataStrength.' '.@$dataDexterity.' '.@$dataIntelligence.' '.@$dataVitality.' '.@$dataRArcane.'
'.@$dataLok.'  '.@$dataLph.'  '.@$dataLphg.'  '.@$dataLifesteal.'  '.@$dataAdditional.'   
'.@$dataLps.'  '.@$dataICold.' '.@$dataIFire.'  '.@$dataIHoly.'  '.@$dataIPoison.'  '.@$dataIWeapon.'  
'.@$dataAttackSpeed.'  '.@$dataIncreasedMinimumDamage.'  '.@$dataIncreasedMaximumDamage.'  
'.@$dataCHD.'  '.@$dataWd.'  '.@$dataChc.'  '.@$dataBdmgvsel.'  '.@$dataCBlind.' '.@$dataCChill.'  
'.@$dataCFear.'  '.@$dataCFreeze.'  '.@$dataCImmobilize.'  '.@$dataCKnockback.'  '.@$dataCSlow.'  
'.@$dataCStun.'  '.@$dataCtbleed.'  '.@$dataBleeddmg.'  '.@$dataGf.'  '.@$dataMf.'  '.@$dataExp.'  
'.@$dataIndestructible.' '.@$dataMovementSpeed.'  '.@$dataYardspick.'  '.@$dataReducedLevelRequirement.' 
'.@$dataSock1.'  '.@$dataSock2.'  '.@$dataSock3.'  '.@$dataResistanceAll.'  '.@$dataRCold.'  '.@$dataRFire.'
'.@$dataRHoly.'  '.@$dataRLightning.'  '.@$dataRPhysical.'  '.@$dataRPoison.'  '.@$dataIncreasedArmor.'  
'.@$dataCCred.'  '.@$dataMeleeatd.'  '.@$dataReducedDamagefeli.'  '.@$dataChancetoblockomshield.'  
'.@$dataChancetoblockomattr.'  '.@$dataBlockAmount.'
';
[PHP] pobierz, plaintext

Jestem pewien że nie jeden spadł z krzesła

no ale działa.

Ma to na celu sprawdzenie, że jeżeli jest podana wartość w SQL np. dla pola Strength = 0 to nie pokaże tego pola w $attributedump

Jest to na prawdę bardzo patologicznie pokazane wszystko lecz jestem pewien, że jest to żenada, proszę o pomoc.

Chętnie nauczę się operacji na takich dużych zmiennych itd. Proszę o podpowiedzi.

redeemer

15.07.2012, 19:01:56

Na początek to zrób sobie jakiś konfig, coś w stylu:

[PHP] pobierz, plaintext 
$fieldsConfig = array(
...
'Vitality' => 'Vitality',
'RArcane' => 'Arcane Resistance',
...
);
[PHP] pobierz, plaintext

Do tego pętla foreach i sprawa załatwiona.

peter13135

15.07.2012, 19:09:12

1. Aby oddzielić php od htmla, użyj systemu szablonów - smarty albo twig (te polecam bo z tych miałem okazję korzystać).
2. Nie mam pojęcia w jakim celu używasz te małpy, w każdym razie ich używanie jest niezalecane. Błędy należy poprawiać, a nie ukrywać i udawać, że ich nie ma.

[PHP] pobierz, plaintext 
<tr><td>'.$row['id'].'</td> 
 
                <td>'.$row['Strength'].'</td> 
 
                <td>'.$row['Dexterity'].'</td> 
 
                <td>'.$row['Intelligence'].'</td>
 
                <td>'.$row['Vitality'].'</td>
 
                <td>'.$row['Lok'].'</td> (...)
[PHP] pobierz, plaintext

To możesz zastąpić pętlą, ale chyba zwykle się tego tak nie robi.

[PHP] pobierz, plaintext 
foreach($row as $field)
{
echo '<td>' .$field . '</td>';
}
[PHP] pobierz, plaintext

Widzę, że w wielu miejscach masz taki sam znacznik style="color:#6969FF;" i podobne. Użyj do tego klasy.

Poza tym, jednoznacznie trudno wskazać, jak ten kod trzeba skrócić. Jeśli masz wiele pól i każde pole ma jakieś różne "reguły" to trzeb je wzystkie uwzględnić.

Na pewno dużo czytelniej będą one wyglądać w systemie szablonów.

Wertas

15.07.2012, 19:12:36

@Peter zobacz kod cały.Jeden i drugi.

http://pastebin.com/tsT95LCU
http://pastebin.com/PUdEzyH9

Tu nic nie ma skomplikowanego i nie będzie. Tylko jest tego za dużo

niby działa wszystko ale mi się nie podoba tak pisać. ;( Wpaja mi to złe nawyki

Większość może być znacznie skrócona bo np taki kod

[PHP] pobierz, plaintext 
if ($row['RCold'] == '0') { echo '';} else { $dataRCold ='<li class=d3-color-blue><p style="color:#6969FF;"><span class=value>+ '.$row['RCold'].'</span> Cold Resist</p></li>'; }
[PHP] pobierz, plaintext

sprawdza czy coś jest w bazie jak jest 0 to nic nie da a jak będzie to znaczki @$dataRCold pokaże jego wartość a to @ dla zabezpieczenia jak by pole 0 przeszło sam nie wiem..

Ale jestem pewien że to można skrócić do 3-10 linijek nie 50

CuteOne

15.07.2012, 19:19:43

Mogę cię uspokoić kod nie jest tragiczny a ilość kolumn wynika z ilości danych.. nie raz pisałem większe potworki

ps. http://phpdao.com/ poczytaj, zobacz na necie przykłady i oceń czy się nada

EDIT do tego co napisałeś o if(){}else{}

[PHP] pobierz, plaintext 
$str .= (!empty($row['xxx'])) ? $row['xxx'];
[PHP] pobierz, plaintext

EDIT2:
używaj funkcji w miejscach powtórzeń kodu !

[PHP] pobierz, plaintext 
 
function showCell($row, $description) {
 
   return '<li class="d3-color-blue"><p style="color:#6969FF;"><span class="value">+ '.$row.'</span>'.$description.'</p></li>';
}
 
 
while() {
 
 ...
 $str .= showCell($row['RCold'], 'Cold');
 ...
}
[PHP] pobierz, plaintext

peter13135

15.07.2012, 19:25:16

@ przed niczym nie zabezpiecza i nie mam nadal pojęcia po co on tam jest.

Wertas

15.07.2012, 21:42:55

A co z tym

[PHP] pobierz, plaintext 
$attributedump = '
'.@$dataStrength.' '.@$dataDexterity.' '.@$dataIntelligence.' '.@$dataVitality.' '.@$dataRArcane.'
'.@$dataLok.'  '.@$dataLph.'  '.@$dataLphg.'  '.@$dataLifesteal.'  '.@$dataAdditional.'   
'.@$dataLps.'  '.@$dataICold.' '.@$dataIFire.'  '.@$dataIHoly.'  '.@$dataIPoison.'  '.@$dataIWeapon.'  
'.@$dataAttackSpeed.'  '.@$dataIncreasedMinimumDamage.'  '.@$dataIncreasedMaximumDamage.'  
'.@$dataCHD.'  '.@$dataWd.'  '.@$dataChc.'  '.@$dataBdmgvsel.'  '.@$dataCBlind.' '.@$dataCChill.'  
'.@$dataCFear.'  '.@$dataCFreeze.'  '.@$dataCImmobilize.'  '.@$dataCKnockback.'  '.@$dataCSlow.'  
'.@$dataCStun.'  '.@$dataCtbleed.'  '.@$dataBleeddmg.'  '.@$dataGf.'  '.@$dataMf.'  '.@$dataExp.'  
'.@$dataIndestructible.' '.@$dataMovementSpeed.'  '.@$dataYardspick.'  '.@$dataReducedLevelRequirement.' 
'.@$dataSock1.'  '.@$dataSock2.'  '.@$dataSock3.'  '.@$dataResistanceAll.'  '.@$dataRCold.'  '.@$dataRFire.'
'.@$dataRHoly.'  '.@$dataRLightning.'  '.@$dataRPhysical.'  '.@$dataRPoison.'  '.@$dataIncreasedArmor.'  
'.@$dataCCred.'  '.@$dataMeleeatd.'  '.@$dataReducedDamagefeli.'  '.@$dataChancetoblockomshield.'  
'.@$dataChancetoblockomattr.'  '.@$dataBlockAmount.'
';
[PHP] pobierz, plaintext

Jest coś innego możliwego do zrobienia z tym?

sobol6803

15.07.2012, 21:48:39

Cytat(Wertas @ 15.07.2012, 22:42:55 )

A co z tym

[PHP] pobierz, plaintext 
$attributedump = '
'.@$dataStrength.' '.@$dataDexterity.' '.@$dataIntelligence.' '.@$dataVitality.' '.@$dataRArcane.'
'.@$dataLok.'  '.@$dataLph.'  '.@$dataLphg.'  '.@$dataLifesteal.'  '.@$dataAdditional.'   
'.@$dataLps.'  '.@$dataICold.' '.@$dataIFire.'  '.@$dataIHoly.'  '.@$dataIPoison.'  '.@$dataIWeapon.'  
'.@$dataAttackSpeed.'  '.@$dataIncreasedMinimumDamage.'  '.@$dataIncreasedMaximumDamage.'  
'.@$dataCHD.'  '.@$dataWd.'  '.@$dataChc.'  '.@$dataBdmgvsel.'  '.@$dataCBlind.' '.@$dataCChill.'  
'.@$dataCFear.'  '.@$dataCFreeze.'  '.@$dataCImmobilize.'  '.@$dataCKnockback.'  '.@$dataCSlow.'  
'.@$dataCStun.'  '.@$dataCtbleed.'  '.@$dataBleeddmg.'  '.@$dataGf.'  '.@$dataMf.'  '.@$dataExp.'  
'.@$dataIndestructible.' '.@$dataMovementSpeed.'  '.@$dataYardspick.'  '.@$dataReducedLevelRequirement.' 
'.@$dataSock1.'  '.@$dataSock2.'  '.@$dataSock3.'  '.@$dataResistanceAll.'  '.@$dataRCold.'  '.@$dataRFire.'
'.@$dataRHoly.'  '.@$dataRLightning.'  '.@$dataRPhysical.'  '.@$dataRPoison.'  '.@$dataIncreasedArmor.'  
'.@$dataCCred.'  '.@$dataMeleeatd.'  '.@$dataReducedDamagefeli.'  '.@$dataChancetoblockomshield.'  
'.@$dataChancetoblockomattr.'  '.@$dataBlockAmount.'
';
[PHP] pobierz, plaintext

Jest coś innego możliwego do zrobienia z tym?

Chyba najlepiej zmiana na:

[PHP] pobierz, plaintext 
$attributedump = "$dataStrength $dataDexterity $dataIntelligence $dataVitality"; //itp.
[PHP] pobierz, plaintext

Mephistofeles

16.07.2012, 09:30:37

Nie lepiej. Lepiej takie dane trzymać w tablicy, wtedy łatwiej je ze sobą złączyć, ale najlepiej byłoby je oddzielić od PHP, bo z tego co widzę zmienne te zawierają kod HTML.

Wertas

16.07.2012, 21:33:33

Mogę poprosić o przykład?

CuteOne

17.07.2012, 01:28:14

W tym wypadku na nic mu z tablicy skoro ma wyświetlać mix HTML + dane z bazy. Wertas podałem ci odpowiedź - utwórz funkcję, która zwróci kod HTML i dopisze go do zmiennej.

[PHP] pobierz, plaintext 
$name = array(
 'crest' => 'Cold Resist',
 'rrest' => 'Red Resist',
 ...
);
 
function createCell($description, $value) {
 
   return '<li class="d3-color-blue"><p style="color:#6969FF;"><span class="value">+ '.$value.'</span>'.$description.'</p></li>';
}
 
function createRow($aRow, $aName) {
 
    $html = '';
 
    foreach($aRow as $key => $value) {
 
         $html .= createCell($aName[$key], $value);
    }
 
return $html;
}
 
 
$str = '';
while() {
 
   $str .= crateRow($row);
}
 
echo $str; // zamiast $attributedump = '';
[PHP] pobierz, plaintext

Następnego gotowca już nie będzie.. więc pora zacząć myśleć i szukać pomocy w manualu.

ps. żeby nie było za łatwo zostawiłem parę błędów, które sam musisz odnaleźć i poprawić

Wertas

17.07.2012, 08:13:23

Poradziłem sobie bez funkcji

To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.