Gå til innhold

PLudrer med OOP for første gang; feil metodikk?

Mads-b

Av Mads-b
i Programmering og webutvikling

Anbefalte innlegg

Mads-b

Mads-b

Skrevet
Skrevet (endret)

Jeg hadde litt mye tid i dag, så jeg slengte sammen en liten mikroklasse av noe som skal bli en kalender når jeg er ferdig. Det er mitt første steg ut i OOP-verdenen, og jeg skjønner enda ikke hvorfor folk velger denne veien. Nåværende kode er skrevet på 2 timer (!), og spyter for øyeblikket ut en array som bare må ha en linebreak hvert 7'ende element for å gi en kalender. Videre funksjoner jeg planla å lage inkluderer et slags templatesystem, så jeg bare spesifiserer HTML i en egen fil, så kan jeg endre output'en til det maksimale.

 

Hva gjør jeg feil? Selv føler jeg koden ble som prosedyrisk kode stappet inn i en klasse..

 

<?php
class calendar {
var $calendarArray; //Nuff said.
function calendar() { //Constructor
	$this->calendarArray = array();
}
function buildCalendarMonth($currentMonth, $currentYear, $weekFirst) { //build calendar array
	$curTimeStamp = mktime(0,0,0,$currentMonth,1,$currentYear); //Build timestamp for first day in month
	$firstDay = date('w', $curTimeStamp); //Get day number of the first day in order to calculate spaces before the 1'st
	$monthSpaces = 0; //Initialise counter for blank spaces before numbered days
	if($weekFirst == 'Sunday') $monthSpaces = $firstDay-1; //If weeks first day is Sunday (SMTOTFS), then calculate number of blank spaces before first day in month.
	if($weekFirst == 'Monday') $monthSpaces = $firstDay-2; //Same as above, except we're using the (MTOTFSS) system.
	if($monthSpaces == -1) $monthspaces = 6; //Day number -1 equals day number 6 (I think)
	$daysOfMonth = date('t', $curTimeStamp); //Return number of days in current month
	$minSquares = $daysOfMonth+$monthSpaces; //Food for the loop below.
	for($curSquare=0;$curSquare<=$minSquares;$curSquare++) { //Loop until all days have been processed.
		if($curSquare <= $monthSpaces) $this->calendarArray[] = ''; //Blank blocks in beginning of month
		else $this->calendarArray[] = $curSquare-$monthSpaces; //Numbered blocks
	}
	for(;$curSquare%7;$curSquare++) { //Math magic.
		$this->calendarArray[] = ''; //Add blank blocks at the end of month
	}

}
function replaceCalendarValues($newValues) { //Just a small testing function which makes it possible to rewrite the day numbers to anything (nice for chinese etc..)
	foreach($newValues as $original => $replacement) {
		$key = '';
		$key = array_search($original,$this->calendarArray);
		$this->calendarArray[$key] = $replacement;
	}
}
function get() { //Test function to see if array is processed correctly
	return $this->calendarArray;
}



}
//Alt under denne linjen er for å teste tullet over.. Ja, det funker som ønsket..

$cal = &new calendar();
$cal->buildCalendarMonth(8,2008,'Monday');
$cal->replaceCalendarValues(array(22=>'idag'));
$cal->get();
print_r($cal);

?>

 

EDIT: Diskusjon.no knuser formateringen. View in notepad2/dreamweaver/geany etc..

Endret av Mads-b
Lenke til kommentar

Videoannonse

Videoannonse
Annonse
TorS

TorS

Skrevet
Skrevet

Objektorientert programmering ser mer på objekter som en samling med data, og noen funksjoner som kan utføres på dem. I et kalenderprosjekt kunne en kanskje tenke seg en måned som en samling med dager, eller eventuelt uker, som i sin tur inneholdt dagene igjen. For å skrive ut måneden, kunne du da f.eks. iterere over alle ukene som kalendermåneden din inneholdt, og skrive ut hver av dem.

 

Nå er kanskje ikke det nødvendig bare for å skrive ut en enkel kalender, men hvis vi antar at du vil ha mer informasjon, f.eks. avtaler eller andre ting som du vil ha tilgang til fra kalenderen, så kan du lagre det i de enkle dagene. Poenget mitt er vel at du, som du selv hevder, mer har laget kode som du vanligvis gjør, og slengt det inn i en klasse. Og det i seg selv gjør ikke koden spesielt objektorientert. Jeg vet ikke om innlegget mitt bidrar til å skape en forståelse av objektorientert programmering, men det er iallfall slik jeg tenker på det, og det er en ny måte å tenke på, ingen tvil om det.

Lenke til kommentar
Lokaltog

Lokaltog

Skrevet
Skrevet

Jeg tror kanskje jeg rett og slett ville laget en enkel funksjon for å gjøre det du gjør i den klassen. Det er ikke noe galt å lage funksjoner i et språk som PHP, men med en gang man trenger mer funksjonalitet rundt f.eks. kalenderen, så er det helt klart praktisk å kode mer objektorientert. I tillegg anbefaler jeg at du like greit lærer deg litt av PHP5s OO-funksjonalitet, for den er ufattelig mye bedre enn i PHP4.

 

Et eksempel på en mer "objektorientert" kalenderklasse (kun eksempel for å illustrere måten man kan gjøre ting på):

<?php

class Calendar
{
private $month;
private $year;

private $calendar_array	= array();
private $event_array 	= array();

public function __construct($month, $year)
{
	$this->month	= (int)$month;
	$this->year		= (int)$year;
}

public function addEvent(CalendarEvent $event)
{
	$event_date = $event->getDate();

	$this->event_array[$event_date['year']][$event_date['month']][$event_date['day']] = $event;
}

public function getCalendar()
{
	// Legg inn all funksjonalitet knyttet til selve produksjonen av kalenderen her
	// ...
	// I tillegg kan man loope over $this->event_array for å hente ut events til kalenderen, som dette:
	foreach ($this->calendar_array as $date)
	{
		$event = & $this->event_array[$date['year']][$date['month']][$date['day']];

		if (isset($event) && $event instanceof CalendarEvent)
		{
			$ev = $event->getEvent();

			// Lagre dataene fra $ev på en bestemt dato i kalenderen
		}
	}

	// ...

	return $this->calendar_array;
}

public function getHTML(array $calendar_array)
{
	// Parse arrayet med kalenderen her og returner pen HTML-kode

	return $html;
}
}

class CalendarEvent
{
private $day;
private $month;
private $year;

private $event_data = array();

public function __construct($day, $month, $year)
{
	$this->day		= (int)$day;
	$this->month	= (int)$month;
	$this->year		= (int)$year;
}

// Funksjoner for å lagre events i kalenderen her

public function getDate()
{
	return array(
		'day'	=> $this->day,
		'month'	=> $this->month,
		'year'	=> $this->year,
	);
}

public function getEvent()
{
	return $this->event_data;
}
}

?>

 

Så kan man gjøre noe slikt:

 

<?php

$calendar = new Calendar(8, 2008);

$event = new Event(10, 8, 2008);
$event->addData(...);
$event->foo(...);

$other_event = new Event(11, 9, 2008);
$other_event->bar(...);

$calendar->addEvent($event);
$calendar->addEvent($other_event);

echo $calendar->getHTML();

?>

Lenke til kommentar
Lokaltog

Lokaltog

Skrevet
Skrevet

For øvrig returneres objekter alltid som referanser i PHP5. Nøkkelordet var er deprecated, og hvis jeg ikke tar feil bør man også spesifisere funksjoner som public/private/protected.

 

I koden din antar jeg at ingenting av verdi skrives ut når du kjører scriptet. Dette tror jeg skyldes denne kodebiten:

$cal = &new calendar();
$cal->buildCalendarMonth(8,2008,'Monday');
$cal->replaceCalendarValues(array(22=>'idag'));
$cal->get();
print_r($cal);

... som bør erstattes med:

$cal = &new calendar();
$cal->buildCalendarMonth(8,2008,'Monday');
$cal->replaceCalendarValues(array(22=>'idag'));
$calendar = $cal->get(); // returnerer array
print_r($calendar); // $cal er en instance av calendar-objektet, og vil derfor printes helt forskjellig når man kjører det gjennom print_r

Lenke til kommentar
Mads-b

Mads-b

Skrevet
  • Forfatter
Skrevet

Lokaltog: Joda, print_r() printet ut hele objektet til meg, uten at jeg fant ut hvorfor ^^. Takk for rettleding. Jeg har brukt en halv time på å granske koden du slengte opp nå. Mye syntaks jeg ikke var klar over som heller ikke er så tydelig i manualen.

 

Uansett, jeg skal slenge opp det jeg lurer på nå, så slipper jeg å plages med det senere:

 

Hvorfor __construct fremfor "calendar"? Constructfunksjonen tillater vel begge?

 

Er det vanlig å spesifisere variabeltype når man lager plass i minnet? ser du gjør noe slikt som

$this->variable = (int)$var;

Skal man alltid gjøre dette? Ser du bare gjør det i __construct.. er det en form for å bekrefte User-input?

 

Og så ser jeg du bruker en egen event-klasse for hver event som slenges opp i kalenderen. Shite.. Dette vraker hodet mitt..

 

Jaja, nå, 44 minutter etter at jeg begynte å studere koden skjønner jeg endelig opplegget :p Spørsmålene ovenfor står ennå though.

Lenke til kommentar
luxus

luxus

Skrevet
Skrevet
Hvorfor __construct fremfor "calendar"? Constructfunksjonen tillater vel begge?

 

Å kalle konstructoren det samme som klassen henger igjen fra PHP4; i PHP 5 har man gått over til __construct (sammen med flere andre "magic"-methods som __destruct, _sleep, __get/set osv )

 

Er det vanlig å spesifisere variabeltype når man lager plass i minnet? ser du gjør noe slikt som

$this->variable = (int)$var;

Skal man alltid gjøre dette? Ser du bare gjør det i __construct.. er det en form for å bekrefte User-input?

Det som gjøres her er "casting", og må ikke gjøres, men det kan være kjekt når du vet du vil ha typen int lagret i objektet. I tillegg kan man f.eks. sjekke først om verdien is_numeric(), også kaste en feil hvis dette ikke er tilfellet.

Lenke til kommentar
Lokaltog

Lokaltog

Skrevet
Skrevet
Hvorfor __construct fremfor "calendar"? Constructfunksjonen tillater vel begge?

Det er litt mer praktisk å bruke __construct fremfor klassenavnet som constructor, selv om begge deler er tillatt. Man slipper jo å endre på navnet på constructoren hvis man senere endrer navnet på klassen f.eks. :p Og som Jonas sier, så henger dette igjen fra PHP 4, og det er like greit å bruke __construct nå som PHP5 likevel er så utbredt.

Er det vanlig å spesifisere variabeltype når man lager plass i minnet? ser du gjør noe slikt som

$this->variable = (int)$var;

Skal man alltid gjøre dette? Ser du bare gjør det i __construct.. er det en form for å bekrefte User-input?

Man burde kanskje hivd det inn i en try/catch-blokk som luxus skriver. Men jeg caster variablene i eksemplet mitt fordi datofunksjoner (mktime, etc.) tar numeriske verdier, og da slipper scriptet å dø senere hvis du sender ikke-numeriske variabler til klassen din. Men med en try/catch og sjekke variablene med is_numeric ville så klart vært bedre, særlig mtp. debugging senere.

Og så ser jeg du bruker en egen event-klasse for hver event som slenges opp i kalenderen. Shite.. Dette vraker hodet mitt..

Det er det som er ulempen med OOP - det kan være litt klønete å sette seg inn i. Jeg anbefaler boken "PHP 5 Objects, Patterns, and Practice" (Apress), den er svært god og gir deg en bra introduksjon til OOP i PHP 5.

Lenke til kommentar
Mads-b

Mads-b

Skrevet
  • Forfatter
Skrevet

Tja, jeg fikk et ganske bra script ut av arbeidet og jeg tror jeg forstår en smule bedre nå.

 

Siste spørsmål:

Jeg lagde en event-klasse, og i kalenderklassen er det en funksjon som leser inn en fil med "events" og lager en array med eventobjekter. Finnes det en effektiv måte å hente ut events med felles dato, eller må jeg loope gjennom hver eneste event hver eneste dagsoppføring? Høres knøvlete ut. Vil så gjerne unngå sql når scriptet er såppass enkelt..

Lenke til kommentar
Jonas

Jonas

Skrevet
Skrevet (endret)

Bruk timestamp for dager som array-indekser. Ellers trenger du vil strengt tatt ikke sql for å loope gjennom et array, dersom du har lagret det i en fil.

 

function dayFromTimestamp ( $timestamp ) {
return $timestamp - ( $timestamp % ( 60 * 60 * 24 ) );
}

Endret av Jonas
Lenke til kommentar
Mads-b

Mads-b

Skrevet
  • Forfatter
Skrevet (endret)

Hmm.. Testet konseptet. Ser ut til at det ikke vil gå. I søken på fleksibilitet har jeg en liten fiffig sak i datafilen min. Jeg legger med de viktigste hendelsene som eksempel:

 

22|11|x|Mads' Bursdag
24|12|x|Julaften
20|x|x|Skifte linser

 

22|11|x betyr at eventen skal føres opp på kalenderen hvert år.

20|x|x betyr at eventen skal vises den 20. hver måned. Det gjør timestamp eller annen identifikasjon veldig vanskelig.. :( forslag?

 

edit: funksjonen min som gjør denne jobben med å finne ut om dato=dato er som følger:

	private function compareDates($d1,$m1,$y1,$d2,$m2,$y2) { //small function to check if two dates are matching
	if($d1 == $d2 || $d2 == 'x') {
		if($m1 == $m2 || $m2 == 'x') {
			if($y1 == $y2 || $y2 == 'x') {
				return TRUE;
			} else return FALSE;
		} else return FALSE;
	} else return FALSE;
}

Endret av Mads-b
Lenke til kommentar
Jonas

Jonas

Skrevet
Skrevet (endret)

Jeg ser ikke helt problemstillingen. Kan du ikke hente ut slik du vanligvis gjør når du viser kalenderen? Eller har du laget en database du ikke skjønner hvordan du enkelt skal hente ut ifra?

 

Edit: Hva med å bare bruke funksjonen du allerede har? Loop igjennom alle events og se etter like datoer.

Endret av Jonas
Lenke til kommentar
Mads-b

Mads-b

Skrevet
  • Forfatter
Skrevet

Altså: Magien skjer når jeg skal printe ut kalenderen og må kombinere eventArray og calendarArray.

CalendarArray blir slik: (en celle):

[6] => Array

(

[day:private] => 1

[month:private] => 11

[year:private] => 2008

)

 

EventArray blir slik (en celle):

[1] => calendarEvent Object

(

[day:private] => 24

[month:private] => 12

[year:private] => x

[info:private] => Julaften

 

)

 

Så har du maskinen som skal kombinere kaoset. Jeg henter ut en og en celle fra calendarArray.

 

Jeg sitter da med full dato. Det optimale slik jeg ser etter det, er om jeg kunne fått ut alle calendarEvent objektene i eventArray som matcher datoen jeg sitter med, men som må ta hensyn til x-ene..

 

Slik jeg gjør det nå:

foreach($this->calendarArray as $day) {

//kode

foreach($this->eventArray as $event) {

kjør comparedates for alle events her..

}

}

Problemet er at hvis jeg har ganske mange events vil dette bli ganske heftig etter hvert, siden jeg må loope gjennom hele lista opp til 31 ganger hver gang jeg tråkker gjennom klassen, og samme antall ganger hvis jeg vil vise detaljer for èn dag i en popup.. Skjønner du problemstillingen?

Lenke til kommentar
Jonas

Jonas

Skrevet
Skrevet (endret)

Problemet med å ikke lagre timestamps er nettopp at alle datoer må parses hver gang, og ja - du ender opp med å loope gjennom hele lista. Men dette kan jo naturligvis optimaliseres. F.eks. kan du sørge for å lagre datoer i database-filen din i en kronologisk rekkefølge, slik at du kan avbryte en løkke i det du treffer første dato som ikke passer. Ellers kan du prøve å implementere caching i systemet ditt, som hentes hver gang du viser detaljer for èn dag.

 

Edit: Jeg tror uansett du kommer til å klare deg ganske bra med å bare loope gjennom alle events. Du skal ha særdeles mange events før du i det hele tatt begynner å merke noe på ytelsen.

Endret av Jonas
Lenke til kommentar
Lokaltog

Lokaltog

Skrevet
Skrevet

Angående kodevaner, ta en titt på funksjonen du skrev ovenfor:

	private function compareDates($d1,$m1,$y1,$d2,$m2,$y2) { //small function to check if two dates are matching
	if($d1 == $d2 || $d2 == 'x') {
		if($m1 == $m2 || $m2 == 'x') {
			if($y1 == $y2 || $y2 == 'x') {
				return TRUE;
			} else return FALSE;
		} else return FALSE;
	} else return FALSE;
}

For oversiktlighetens skyld så trenger du ikke å ha en "else return false" eller så mange nestede if-blokker. Jeg ville kanskje heller skrevet det slik, som fungerer helt likt:

private function compareDates($d1, $m1, $y1, $d2, $m2, $y2)
{
return (($d1 == $d2 || $d2 == 'x') && ($m1 == $m2 || $m2 == 'x') && ($y1 == $y2 || $y2 == 'x'));
}

Lenke til kommentar
Mads-b

Mads-b

Skrevet
  • Forfatter
Skrevet

Jonas: OK, vi satser på at databehandling er kjappere enn jeg tror ^^

 

Lokaltog: Hvor lærer du slikt? Ingen tuts eller manualer jeg har funnet snakker om slike snarveier?

 

lagde uansett en kjip og kjapp funksjon for å lage en array av events som gjelder en gitt dag.. Reusability factor==0

	public function fetchEventsForDate($day, $month, $year) { 
//outputs array with events for the given day only
	foreach($this->eventArray as $event) {
		$evData = $event->fetchData();
		if($this->compareDates($day,$month,$year,$evData['day'],$evData['month'],$evData['year'])) 
//Check if looped event is associated with current day
			$foundEvents[] = $evData;
	}
	if(isset($foundEvents)) return $foundEvents;
}

 

Garantert dårlig kode dette også, men jeg kan ikke bedre.

Lenke til kommentar

Opprett en konto eller logg inn for å kommentere

Du må være et medlem for å kunne skrive en kommentar

Opprett konto

Det er enkelt å melde seg inn for å starte en ny konto!

Start en konto

Logg inn

Har du allerede en konto? Logg inn her.

Logg inn nå
Gå til emneliste

  • Hvem er aktive   0 medlemmer

    • Ingen innloggede medlemmer aktive
×
×
  • Opprett ny...