Najoptmalniejsze wyjście?

[Largo]

Witam,

Czy zaprezentowany poniżej kod jest optymalny? Oto opis jego działania - przed wezwanie mediatora skrypt ma sprawdzać czy są aktywni jacyś mediatorzy. Jeżeli nie, ma podawać formularz z zapytaniem czy ma dodać mimo to, ale tego nie dodałem tutaj :-)

		if ( $this->ipsclass->vars['ram_no_online_pass'] )
	   {
		   if ( $this->ipsclass->vars['ram_active_medi'] )
		   {
			   /* Select all mediator's :-) */
			   $this->ipsclass->DB->build_query( array( 'select'	=> '*',
														'from'		=> 'members',
														'where'	=> 'trader IN ( '.$this->ipsclass->vars['ram_active_medi'].' )') );

			   $this->ipsclass->DB->exec_query();

			   while ( $row = $this->ipsclass->DB->fetch_row() )
			   {
				   $row = $this->ipsclass->member_set_information( $row );
				   $mediators[] = $row;
			   }

			   $mediators_count = count( $mediators );
			   $offline		  = 0;

			   foreach ( $mediators AS $mediator )
			   {
				   if ( $mediator['_online'] == 0 )
				   {
					   $offline++;
				   }
			   }

			   /* Is all mediators offline? */
			   if ( $offline == $mediators_count )
			 {
				 $this->output .= $this->ipsclass->compiled_templatesp['skin_mediator']->no_medi_online();
				 return;
			 }
		   }
	   }

Pozdrawiam,

Largo

Edytowane 5 Maja 2009 przez Largo

[Batman]

potrzebujesz wszystkich ( czy też większości) kolumn z tabeli members?

[Largo]

Witam,

Hmm, tyle ile wymaga member_set_information()? Dałem *, bo on tam wymaga wielu pól, mam je listować? Nie widzę sensu. Do Online potrzebuję: last_visit, login_anonymous i last_activity, mam tylko je pobierać? Jak widzisz lukę w bezpieczeństwo to zmienię.

Pozdrawiam,

Largo

Edytowane 5 Maja 2009 przez Largo

[DawPi]

Zamień:

					   $row = $this->ipsclass->member_set_information( $row );
				   $mediators[] = $row;

Na:

					   $mediators[] = $this->ipsclass->member_set_information( $row );

Kod wydaje się OK. Przy microsekundach wykonywania raczej nie odczujesz optymalizacji takiego drobnego kodu.

Trochu tylko dziwne te jakieś warunki, że coś tam liczysz, sumujesz liczbę online. Jak bym to to trochę inaczej mimo wszystko napisał.

[Largo]

Witaj,

Fakt, dzięki :-)

A jak Ty nad myślałeś? Ja właśnie jestem ciekawy czy wybrałem dobry sposób.

[Batman]

skoro potrzebujesz większości to dobrze

co do reszty kodu wygląda ok

[Largo]

Witam,

Ciutkę pokminiłem i wydajniejszą opcją wydaje się zatrzymanie pętli podczas wystąpienia pierwszego mediatora Online. Wymodziłem taki kodzik:

				//-----------------------------------------
			 // Check all mediator's online indicator
			 //-----------------------------------------
			 foreach ( $mediators AS $mediator )
			 {
				 /* If at least one mediator is online, break that loop! */
				 if ( $mediator['_online'] == 1 )
				 {
					 break;
				 }
				 else
				 {
					 $this->output .= $this->ipsclass->compiled_templates['skin_mediator']->no_medi_online();
					 return;
				 }
			 }

Zastanawiam się jedynie co się stanie jeśli nie będzie żadnego mediatora?

PS. Znalazłem chyba optymalne wyjście z sytuacji. Oto kod:

		//-----------------------------------------
	// Any mediator is Online?
	//-----------------------------------------
	if ( $this->ipsclass->vars['ram_no_online_pass'] )
	{
		if ( $this->ipsclass->vars['ram_active_medi'] )
		{
			/* Select all mediator's :-) */
			$this->ipsclass->DB->build_query( array( 'select'	=> '*',
													 'from'		=> 'members',
													 'where'	=> 'trader IN ( '.$this->ipsclass->vars['ram_active_medi'].' )') );

			$this->ipsclass->DB->exec_query();

			/* What if tag exists but no members have it? Continue with code :-) */
			if ( $this->ipsclass->DB->get_num_rows() )
			{
				while ( $row = $this->ipsclass->DB->fetch_row() )
				{
					$row['members_display_name'] = $this->ipsclass->make_name_formatted( $row['members_display_name'], $row['mgroup'] );
					$mediators[] = $this->ipsclass->member_set_information( $row );
				}

				$all = count( $mediators );

				//-----------------------------------------
				// Check all mediator's online indicator
				//-----------------------------------------
				foreach ( $mediators AS $mediator )
				{
					/* If at least one mediator is online, break that loop! Thx to oLaudix for great idea! */
					if ( $mediator['_online'] == 1 )
					{
						break;
					}
					else
					{
						$this->output .= $this->ipsclass->compiled_templates['skin_mediator']->no_medi_online();
						return;
					}
				}
			}
		}
	}

Z testów co zrobiłem jeżeli istnieją aktywne grupy, ale nie ma usera, omijaj to zabezpieczenie, w innym przypadku pokazuje formularz. Zrobię test na 3 osobach i dam znać.

PS2. Cholera, nie do końca. W pętli foreach trzeba dodać chyba if z weryfikacją ilu Medi jest, bo np. mnie ( Largo ) przepuszcza, a inny nick nie ( Doomsday ). Obstawiam foreach.

Edytowane 5 Maja 2009 przez Largo

[Batman]

jak rozumiem pobieramy zawartość tabeli w/g tego czy dany user jest mediatorem czy tak?

[Largo]

Witaj,

Tak, to jest oczywiste. Nie ma żadnego order i limit :-) Zwykły prosty select z MySQL. Co Ty na to? On prostu musi weryfikować jeszcze na koniec sytuację kiedy nie ma żadnego mediatora.

[Batman]

skoro nie mam pojęcia co robisz to muszę zgadywać

pokaż zawartość tabeli $mediators przed foreach

[Largo]

Witaj,

Hmm - wyobraż sobie zrzucaną tablicę wszystkich danych z members :-) Sparsowaną poprzez member_set_information(). Nie ma sensu. A może wyjaśnić słownie co i jak, a ja sobie to zakoduje. Ale pomyślę nad tym jeszcze.

Pozdrawiam,

Largo

[Batman]

$mediators to tablica wielowymiarowa?

[DawPi]

No tak, nawet z kodu to widać.

[Batman]

wolałem się upewnić

i zapewne pierwsza tablica reprezentuje Ciebie ?

[DawPi]

A to już nie wynika. Ale do czego zmierzasz?

//OT sorki nie mogłem.

BATMAN- Bokserki Satynowe z Kamasutrą

:D

//OT

[Largo]

Ostatecznie chyba zostaje wersja ze zliczaniem wszystkich mediatorów z zapytania i w foreach dodawać offline i po pętli sprawdzać czy wszyscy są Offline na zasadzie warunku sprawdzającego czy wszyscy z zapytania są równi z wszystkimi z pętli.

Edytowane 5 Maja 2009 przez Largo

[Batman]

tymi gaciami to mnie tak zabiłeś że teraz to już sam nie wiem o co mi chodziło ale wydaje się ze foreach nie sprawdza wszystkich tablic

[DawPi]

tymi gaciami to mnie tak zabiłeś że teraz to już sam nie wiem o co mi chodziło

ale wydaje się ze foreach nie sprawdza wszystkich tablic

Ideą foreach jest, że 'skacze' po wszystkich elementach tablicy. Oczywiście jeśli nie zastosuje się w dowolnym momencie break;.

[Batman]

ale chyba żeby przeleciało zawartość wszystkich tablic w tablicy potrzeba zagnieździć jeszcze jedną pętelkę ?

[DawPi]

ale chyba żeby przeleciało zawartość wszystkich tablic w tablicy potrzeba zagnieździć jeszcze jedną pętelkę ?

W tym kodzie leci po tabeli, której elementy są tabelami, potem sprawdza już warunek wg nowej zmiennej, zawierającej już tę 'wewnętrzną' tabelę. Do czego zmierzamy?

[Batman]

Largo pisze ze przepuszcza tylko jego mimo ze inni spełniają warunek czyli są online o ile dobrze zrozumiałem.

[DawPi]

To ja już nie wiem, co on kombinuje.

Largo - jeszcze w kodzie zmień to return na break; w tej pętli foreach.

[Largo]

		//-----------------------------------------
	// Any mediator is Online?
	//-----------------------------------------
			if ( $this->ipsclass->vars['ram_no_online_pass'] )
			{
					if ( $this->ipsclass->vars['ram_active_medi'] )
					{
							/* Select all mediator's :-) */
							$this->ipsclass->DB->build_query( array( 'select'	   => '*',
																											 'from'		 => 'members',
																											 'where'		=> 'trader IN ( '.$this->ipsclass->vars['ram_active_medi'].' )') );

							$this->ipsclass->DB->exec_query();

							/* What if tag exists but no members have it? Continue with code :-) */
							if ( $this->ipsclass->DB->get_num_rows() )
							{
									while ( $row = $this->ipsclass->DB->fetch_row() )
									{
											$row['members_display_name'] = $this->ipsclass->make_name_formatted( $row['members_display_name'], $row['mgroup'] );
											$mediators[] = $this->ipsclass->member_set_information( $row );
									}

									$all = count( $mediators );

									//-----------------------------------------
									// Check all mediator's online indicator
									//-----------------------------------------
									foreach ( $mediators AS $mediator )
									{
											/* If at least one mediator is online, break that loop! Thx to oLaudix for great idea! */
											if ( $mediator['_online'] == 1 )
											{
													break;
											}
											else
											{
													$offline++;
											}
									}

									/* No online in loop? So, if no Online, check if all mediators is the same number as offline mediators */
									if ( $all == $offline )
									{
											$this->output .= $this->ipsclass->compiled_templates['skin_mediator']->no_medi_online();
											return;
									}
							}
					}
			}

Masz lepszy sposób?

[DawPi]

Możesz napisać jeszcze raz, co ten kod ma robić? I czy to cała funkcja ( masz return więc mniemam, że to funkcja )?

[Largo]

Możesz napisać jeszcze raz, co ten kod ma robić? I czy to cała funkcja ( masz return więc mniemam, że to funkcja )?

Witaj,

Przed wejściem do możliwości wysłania raportu, sprawdza czy jakikolwiek mediator z ich aktywnych grup istnieje i czy są online. Jeżeli trafi na pierwszy możliwy online kończy wykonywanie kodu. A jeżeli nie trafi na żadnego z wszystkich możliwych, ma pokazać formularz z pytaniem czy mimo to chcę kontynuować.