w związku z tym: http://forum.php.pl/index.php?showtopic=38025 tematem zgłosiło się kilka osób chętnych do współpracy przy tworzeniu nowej wersji mojego serwisu. Każdego z nich poprosiłem o napisanie części jednego z modułów, aby ocenić ich umiejętności i później wybrać tego, którego kod będzie najpoprawniejszy, etc.

Ponieważ sam nie znam php postanowiłem poprosić o ocenę Was, jak mniemam, specjalistów w tej dziedzinie. Pierwszy kod do oceny już mam. Jest to niewielka część modułu newsów, zaledwie odłamek serwisu, który ma na celu sprawdzenie, czy autor poradzi sobie z napisaniem szybkiego, zaawansowanego i bezpiecznego CMS'a.

Link do modułu: http://www.kadysz.e9.pl/tomek/
Link do źródła: http://www.kadysz.e9.pl/tomek/source.zip

Proszę o ocenę, krytykę i propozycje ewentualnych poprawek.
BTW. jeśli ktoś chciałby dołączyć do grona programistów generacjax.pl
proszę o kontakt. wszelkie dane podałem w temacie:
http://forum.php.pl/index.php?showtopic=38025

Tak na szybko:
Jak ustawi się najwyższy poziom błędu, to powinno polecieć pełno notticów(bledow).

Aby w pełni ocenić ten kod, dobrze byłoby wiedzieć czy ma działać na PHP4 czy PHP5.

Dwie ciekawych rzeczy:

• Brak jakiegokolwiek (widocznego) zabezpieczenia zmiennych użytych w zapytaniu do bazy danych (linie 56, 66, 83) - wystarczy wpisać coś w adres i można żegnać się z bazą.
• Użycie stałych (większość zapytań do bazy) - niekoniecznie błąd, lecz przy większych serwisach to idiotyzm.

Jeśli te zmienne są gdzieś tam zabezpieczone przed wysłaniem ich do bazy, i jakby usunąć te stałe, to kod byłby IMO ok.

Aczkolwiek nie jest to najwydajniejszy kod do wykonania tak prostej funkcjonalności (200+ linii kodu - przy użyciu odpowiednich nażędzi spoko możnaby to zrobić w 100 lub mniej (IMO)).

Trudno powiedzieć więcej nie widząc całości kodu.

Jak dla mnie OOD w całym CMS-ie jest nie poprawne, no bo czemu Newsy dziedziczą po Configu.

Poza tym jeśli to php 4, to powinny być znaczki & w argumentach metod.

ma działać na PHP4

@k_c2or - temu kodowi sporo do doskonalosci - dosc szybko mozesz sie dorobic attackow typu sql_injection.

A co myślicie o takim kodzie?

Jest to wycinek modulu panelu adminitracyjnego, ktory zwraca kod HTML do szablonu. Layer do bazy obrabia odpowiednio dane w zaleznosci od typu.
Zależy mi na porównaniu dwóch tych wycinków (news powyżej i tego poniżej)
pod względem jakości kodu.

[PHP] pobierz, plaintext 
<?php
 
class system_settings
{
	private $lastSub='';
 
	public function run($action)
	{
		$html='';
 
		switch ($action)
		{
			default:
			case 'groups':
				$html=$this->groups();
			break;
 
			case 'newgroup':
				$html=$this->newGroup();
			break;
 
			case 'delete':
				$html=$this->delete();
			break;
		}
 
		return $html;
	}
 
	public function getName()
	{
		return "System settings";
	}
 
	public function getSubName()
	{
		return $this->lastSub;		
	}
 
	private function newGroup()
	{
		$request=httpContext::instance()->request;
 
		if($request->contains('group_name','group_key','group_description'))
		{
			$group_name=$request->map(POST,'group_name','',new stringCheck(3));
 
			$group_key=$request->map(POST,'group_key','alphanumeric',new stringCheck(3));
 
			$group_description=$request->map(POST,'group_description','string');
 
			if($request->noErrors())
			{
				$db=db::instance();
 
				$db->insert('settings_groups',array('gkey'=>$group_key,'name'=>$group_name,'description'=>$group_description));
 
				if($db->affectedRows())
				{
					acpMessages::add(M_INFO,'New group has been successfully added.');
					return $this->groups();
				}
			}
		}
 
		$form=new acpForm('Add new settings group');
 
		$form->add(new textField('Group name*','group_name',''));
 
		$form->add(new textarea('Group description','group_description',''));
 
		$form->add(new textField('Group key*','group_key','','Needed when you want to access variable from specified group'));
 
		$this->lastSub="New group";
 
		return $form->fetch();
	}
 
	private function groups()
	{
		$db=db::instance();
 
		$groups=$db->getIterator("SELECT * FROM settings_groups");
 
		$table=new acpTable('Groups',$groups);
 
		$table->setActions('system/settings','sg_id',array('edit.gif'=>'edit/%ID%','delete.gif'=>'delete/%ID%'));
 
		$table->setFields('name','description');
 
		$table->setHeader('Name','Description','&nbsp;');
 
		$this->lastSub="Groups";
 
		return $table->fetch();
	}
 
	private function delete()
	{
		$id=httpContext::instance()->request->map(GET,'3','int');
 
		if($id>0)
		{
			$db=db::instance();
 
			$count=$db->count('settings',array('sg'=>$id));
 
			if($count>0)
			{
				acpMessages::add(M_WARNING,'Requested group must be empty before removing');
			}else
			{
				if($db->delete('settings_groups','sg_id='.$id))
				{
					acpMessages::add(M_INFO,'Group has been successfully removed.');
				}else
				{
					acpMessages::add(M_ERROR,'Failed to remove specified group.');
				}
			}
		}
		return $this->groups();
	}
}
 
?>
[PHP] pobierz, plaintext 

A gdzie komentarze ;?

[OT]@sf - czego dot ten post ?[/OT]

@NuLL: chyba chodzilo o komentarze w kodzie.
Przydaly by sie bo mamy klasy system_settings, funkcje jak: groups() delete() run() i nie wiadomo (po ich nazwie i nazwie klasy) co maja robic.

troche poza tematem ale ciekaw jestem jaka sawke za godzine lub za wykonana prace oferuje zleceniodawca, skoro chce wylonic najlepszego programiste jaki sie do niego zglosil, oczekuje pelnego profesionalizmu to mam nadzieje ze wynagrdzenie dla tego wybranca jest adekwatne do wymagan ... ale to juz sprawa stron jak sie dogadaja

teraz do nikogo nic nie przyklejam ale nizadko mozna sie spotkac z podjesciem typu "maximum za minimum" :/ do przyszlych "pracodawcow" mam taki maly apel jezeli oczekujecie profesionalizmu to przygotujcie sie na profesionalna cene za usluge

jesli nad projektem ma pracowac kilku programistow, a tak mnie wyika z postu to nawet nie patrze na poprawnosc kodu jesli nie widze komentarzy.

Bez komentarzy skad jak mam wiedziec co toto ma robic, jak dzialac, co zwracac, jak podlaczyc do innego kawalka kodu. Na GG mam pytac kolegów miale wysyłać, paranoja!

kolega CHAOS zawita w progi takiego projektu w ciagu tygodnia. I z czegos co mialo byc szybkie, fajne itp, zrobi sie papka.

Mimio tego wszystkiego to drugi kod wyglada na lepszy gatunkowo (nie znaczy ze jest, nie studiowalem go), widac stosowanie wzorców

Wg. mnie jest tworzonych za dużo obiektów. Jeden formularz = 4 obiekty?

Po pierwsze w ogłoszeniu podałem stawkę, więc Ci którzy się zgłosili jednoczesnie zgodzili się na taką a nie inną stawkę, to chyba proste i logiczne. Po drugie nie szukam pełnego profesjonalizmu a próbuję wyłonić najlepszego z tych, którzy się zgłosili. Bo przecież mogę zapłacić xxx komuś kto owszem pisze w php ale pisze na poziomie początkującego, a równie dobrze mogę zatrudnić kogoś kto jest profesjonalistą, pracuje gdzieś tam jako programista a tu chce sobie po prostu dorobić.

To się nazywa "podanie stawki" ? rotfl.
Czyli płacisz max: 9.99 ;]

nie, tzn. np. 999, ale nie więcej niż 1000 bo mnie po prostu na to nie stać...
durny komentarz z Twojej strony. chyba wiadomo o co chodzi...
jeśli ktoś chce to zrobić za 100, to zrobi to za 100, jak chce za to 999 to za tyle zrobi.
dobra.. niech będzie, że STAWKA: DO 1000PLN (czyt. tysiąca złotych)
lepiej?

Starczy tego OT na troche