Új hozzászólás Aktív témák

• #10475 Sk8erPeter nagyúr Soak #10474

  Új Válasz 2012-07-18 15:11:57 #10475

  Új hozzászólás

  Összes hozzászólása itt Válaszok az összes hozzászólására itt Válaszok erre a hozzászólásra

  Privát üzenet küldése

  

  Sk8erPeter

  nagyúr

  válasz Soak #10474 üzenetére

  Használd ezt a formázót, ha nem sikerül másként...:
  http://beta.phpformatter.com/

  Szétb@szhatja azért is a formázást, mert public functionnel kezded kapásból, és nem adsz nevet az osztálynak. Nyilván most csak kivágtad a megfelelő részt, de akkor ne az ideone.com-ra rakd, hanem a pastebinre, ha előbbire, akkor legalább adj egy nevet az osztálynak, és tedd oda kommentbe a három pontot, ami jelöli, hogy ott amúgy van más is. (Hogy ne legyen itt ezen az oldalon syntax error.)

  if(isset($_GET['i']))
  Már eleve rossz kezdés szerintem. Itt már szerintem nem kellene $_GET, $_POST és hasonlókhoz nyúlni, hanem a megfelelő helyen be kellene ezeket állítani, legalábbis ez ebben a formában csúnya osztályon belül.
  Ezenkívül mi az az "i"? Miért szopatod magad ilyen kulcsokkal? Miért nem használsz beszédesebb neveket, teljes szavakat?
  Én kimegyek a fazonomból az ilyenektől, elég utálatos feladat, amikor más kódját kell kotorásznod, és ilyeneket látsz, egyből letépnéd a fejét annak, aki írta ezt a kódot, és még egy nyomorék kommentet sem rakott oda. Ha már agilis szoftverfejlesztés, akkor már a kód legyen beszédes.

  $messages = self::find_messages_by_sender_id($_GET['i']);
  Miért static függvényhívás? Miért kevered a szezont a fazonnal? Egyáltalán minek ide static függvény?
  Erről már korábban is volt szó, cucka is említette neked.

  foreach($messages as $message)
  Összesen háromszor?? Mi a francnak mész végig rajta külön-külön?!
  Ugye tudod, hogy ez így háromszor annyi időbe is kerül (és egyébként totál értelmetlen)?

  if(strlen($message->body) > 140){$dots = "...";} else {$dots = "";}
  Ez is több helyen szerepel, az már eleve gáz, de amúgy is: mi értelme van? Ennyiből kb. semmi.
  Ha le kell vágni a törzsből valamennyit, mert adott karaktermennyiséget túllép, akkor azt egy tök különálló truncate() metódusban végezd el, ne ugyanebben a függvényben. A default karakterlimit pedig a függvény egyik paramétere lehet, default értékkel. Vagy tárold pl. osztályváltozóként a limitet, de inkább előbbi.

  Aztán egyszer a $message_username változót használod, egyszer a $message->sender_username-et. Abszolút semmi értelme. Egyébként sincs értelme itt átadni másik változónak.

  $senders = array();
  foreach($messages as $message){
  if(array_key_exists($message->sender_username,$senders)){
  $senders[$message->sender_username]++;
  }
  else{
  $senders[$message->sender_username] = 1;
  }
  }

  Ennek magyarázd már el légyszi, mi értelme, mert szerintem kb. semmi.
  Miért kotorászol a $senders-ben, amikor egy üres tömbbel tetted előtte egyenlővé?

  Tovább nem volt kedvem keresgélni a hibákat, ezek csak első blikkre tűntek fel, némi fájdalmat okozva. Bocs, de ez így iszonyatosan gány. A kritikákon ne sértődj meg, mert Te kérted.

Új hozzászólás Aktív témák