Tilbakemelding på objektorientert program

10. oktober 2016

Hallo!

Da kjerringa mi begynte på universitetet og så smått var innom Phyton programmering blusset min interesse for programmering opp igjen. Prøvde derfor å lage et program skrevet i det jeg vil kalle objektorientert PHP. Ikke at jeg er så veldig dreven per dags dato, men mener å huske jeg kunne det noe godt før.

Programmet skal være å beregne hele meter om til andre enheter som yards, inches og hele den smala der med et simpelt grensesnitt. Ønsker derfor egt litt tilbakemelding på koden min.

Kan første starte med klassen jeg har laget for dette programmet:

Kode

<?php

class Enhetsomregning {
	public $meter;
	public $centimeter;
	public $inch;
	public $foot;
	public $yard;
	public $mile;
	
	public function konstruktør() {
		switch($this->meter) {
			case isset($_POST['regn_cm']):
				$this->meter = (int)$_POST['meter'];
				break;
			case isset($_POST['regn_inch']):
				$this->meter = (int)$_POST['meter'];
				break;
			case isset($_POST['regn_foot']):
				$this->meter = (int)$_POST['meter'];
				break;
			case isset($_POST['regn_yard']):
				$this->meter = (int)$_POST['meter'];
				break;
			case isset($_POST['regn_mile']):
				$this->meter = (int)$_POST['meter'];
				break;
		}
		
		return $this->meter;
	}
	
	public function validerMeter() {
		
			print gettype($this->meter);
			print($this->meter);
		
	}
	
	public function printMeter() {
		if($this->meter != 0) {
			print "Du har valgt og konvertere " . $this->meter . " meter";
		} else {
			die();
		}
	}
	
	public function regnCentimeter() {
		$this->centimeter = ($this->meter*100);
		
		print ("<b>".$this->centimeter." centimeter</b><br>");
	}
	
	public function regnInches() {
		$this->inches = ($this->meter*39.3700787);
		
		print ("<b>".$this->inches." inches</b><br>");
	}
	
	public function regnFoot() {
		$this->foot = ($this->meter*3.2808399);
		
		print("<b>".$this->foot." foot</b><br>");
	}
	
	public function regnYard() {
		$this->yard = ($this->meter*1.0936133);
		
		print("<b>".$this->yard." yards<br>");
	}
	
	public function regnMiles() {
		$this->mile = ($this->meter*0.000621371192);
		
		print("<b>".$this->mile." miles</b><br>");
	}
}

?>

Så har jeg laget den fila med koden som genererer selve nettsiden:

Kode

<?php
//error_reporting(0);
require_once "class/enhetsomregner_class.php";

$c = new Enhetsomregning();
//var_dump($c);
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Enhetsomregner</title>
</head>

<body>
<h1><center>Enhetsomregner</center></h1><br />
<form action="<?php $_SERVER['PHP_SELF']; ?>" method="post">
<center><p>Oppgi antall meter:</p>
  <p>
    <input type="text" name="meter" />
  </p>
  <p>&nbsp; </p>
</center>
<center><input type="submit" value="Konverter til Centimeter" name="regn_cm" /></center><br />
<center><input type="submit" value="Konverter til Iches" name="regn_inch" /></center><br />
<center><input type="submit" value="Konverter til Foots" name="regn_foot" /></center><br />
<center><input type="submit" value="Konverter til Yards" name="regn_yard" /></center><br />
<center><input type="submit" value="Konverter til Miles" name="regn_mile" /></center><br /><center>
</form>

<?php
$c->konstruktør();
$c->printMeter();
?>
<br />
<p>Om til:</p>
<?php
if(isset($_POST['regn_cm'])) {
	$c->regnCentimeter();
	//$c->validerMeter();
} 

if(isset($_POST['regn_inch'])) {
	$c->regnInches();
	//$c->validerMeter();
} 

if(isset($_POST['regn_foot'])) {
	$c->regnFoot();
	//$c->validerMeter();
} 

if(isset($_POST['regn_yard'])) {
	$c->regnYard();
	//$c->validerMeter();
} 

if(isset($_POST['regn_mile'])) {
	$c->regnMiles();
	//$c->validerMeter();
} 
?>

<br /><br /><br /><br /><p align="center">Copyright: Sivert Opsahl Gjermshus</p>
</body>
</html>

Denne inneholder riktig nok noen kommentarer over kodesetninger jeg brukte under testing/utvikling. Men hva syns dere som sikkert har mer peiling på dette enn meg?

robhol

NOOOOOOOOOOOOOOOOOO-

6.423 5.657

10. oktober 2016

Tja. I ingen spesiell rekkefølge: (en del av punktene er ganske nært knyttet opp mot hverandre, så unnskyld meg hvis jeg gjentar meg - det er stort sett fordi det er relevant)

a) en klasse bør være selvstendig og isolert i den grad det er mulig - din klasse tukler med $_POST, for eksempel. La den få informasjonen den trenger via metodekall (helst) og konstruktor, og være isolert ellers.

b) en klasse bør ha kun ett formål, innenfor rimelighetens grenser. Din klasse gjør ikke bare utregninger (formålet i henhold til navnet), men også printing. Dette er, relativt sett, ikke særlig alvorlig, men en grei ting å tenke på.

c) konstruktoren din er ikke en konstruktor, den er en metode med et misvisende navn. Jeg husker ikke syntaksen PHP bruker, men det er forskjellig fra en vanlig metode. Den gir også absolutt ingen mening. Hva var idéen med den?

d) En funksjon bør ha kun ett formål, som (i likhet med alt annet i koden) bør uttrykkes klart ved navnet, og som helst ikke gjør noe annet. Det bør være et veldig klart skille, språklig via navn osv., mellom metoder som leser tilstand, og metoder som endrer tilstand. Hvordan begrepene brukes er ofte en vanesak og kan variere fra språk til språk - det som er helt sikkert er at en metode som i utgangspunktet skal regne ut et enkelt uttrykk ikke har noen business med å endre tilstanden i objektet.

e) printMeter: en metode som i utgangspunktet skal printe noe bør ikke under noen omstendigheter ha den mulige bivirkningen at den dreper programmet ditt. Det er også lite som taler for at den skal være en del av klassen i det hele tatt.

f) metoder som "kommuniserer" (input/output) via variabler er fy. Utrolig fy. Send inn det du vil konvertere til metoden som parametere, gjør det du skal der, og send resultatet ut igjen med return

g) koden din er et godt eksempel på hvorfor man ikke bruker "global state" mer enn nødvendig: kvitt deg med members i klassen din (ikke at det er noe feil med å bruke members, det må bare være et poeng med det), så slipper du også å "kontrollere" tilstanden de har, og mulige problemer med at den forandres uten at du har oversikt over den. For eksempel, hvis kontrollen i programmet ditt var litt mer komplisert, kunne du ha kommet i skade for å kalle en eller annen regnWhatever() og en annen før du fikk lest ut resultatet av den første.

h) copyright? ... nei. Bare... bare nei.

i) repetisjon av kode er i hovedsak noe man ønsker å unngå. For eksempel: alle cases i switchen din har nøyaktig samme utfall, du gjentar en del HTML i alle regn*-metodene, osv

Sist endret av robhol; 10. oktober 2016 kl. 23:34.

alpha01

10 0

11. oktober 2016

Kode

<?php

class Base {
	private $meters = 0.0;
	
	public function __construct($meters) {
		if(is_numeric($meters)) {
			$this->setMeters($meters);
		} else {
			throw new Exception("Input should be numeric.");
		}
	}
	
	protected function getMeters() {
		return $this->meters;
	}
	
	private function setMeters($meters = 0.0) {
		$this->meters = $meters;
	}
}

class MetersToCentimeters extends Base {
	public function __construct($meters) {
		parent::__construct($meters);
	}
	
	public function getCentimeters() {
		return $this->getMeters() * 100;
	}
}

class MetersToDecimeter extends Base {
	public function __construct($meters) {
		parent::__construct($meters);
	}
	
	public function getDecimeters() {
		return $this->getMeters() * 10;
	}
}

class MetersToFeet extends Base {
	public function __construct($meters) {
		parent::__construct($meters);
	}
	
	public function getFeet() {
		return $this->getMeters() * (1 / 0.3048);
	}
}

?>
<!DOCTYPE html>
<head><title>MeterConverter</title></head>
<body>
	
<form method="post" name="convertionForm" action="">
	<input type="text" name="input" <?php if(isset($_POST['input'])) echo ' value="'.$_POST['input'].'"'; ?>>
	<select name="convertionType">
		<option value="centimeter">Convert to centimeter</option>
		<option value="decimeter">Convert to decimeter</option>
		<option value="feet">Convert to feet</option>
	</select>
	<input type="submit" value="Convert">
</form>
	
<?php

if(isset($_POST['convertionType']) && isset($_POST['input'])) {
	$input = $_POST['input'];
	
	switch($_POST['convertionType']) {
		case 'centimeter':
			$m2cm = new MetersToCentimeters($input);
			echo $input . ' meters equals ' . $m2cm->getCentimeters() . ' centimeters.<br>';
			break;
		case 'decimeter':
			$m2dm = new MetersToDecimeter($input);
			echo $input . ' meters equals ' . $m2dm->getDecimeters() . ' decimeters.<br>';
			break;
		case 'feet':
			$m2f = new MetersToFeet($input);
			echo $input . ' meters equals ' . $m2f->getFeet() . ' feet.<br>';
			break;
		default:
	}
}
	
?>	

</body>
</html>

kil0ver

22 2

16. oktober 2016

^Skjønner ikke helt poenget med å lage alle de klassene?

Kode

    class Meter {
    	private $meters = 0.0;
    	
    	public function __construct($meters) {
    		if(is_numeric($meters)) {
    			$this->setMeters($meters);
    		} else {
    			throw new Exception("Input should be numeric.");
    		}
    	}
    	
    	protected function getMeters() {
    		return $this->meters;
    	}
    	
    	private function setMeters($meters = 0.0) {
    		$this->meters = $meters;
    	}
		
	        public function getAsDecimeters() {
    		return $this->getMeters() * 10;
    	}
		
    	public function getAsFeet() {
    		return $this->getMeters() * (1 / 0.3048);
    	}
		public function getAsCentimeters() {
    		return $this->getMeters() * 100;
    	}
    }

alpha01

10 0

18. oktober 2016

Sitat av kil0ver

^Skjønner ikke helt poenget med å lage alle de klassene?[/code]

Vis hele sitatet...

De var vel egentlig ikke påkrevd her i denne sammenhengen, men kan gi en pekepinn på hva OO er.. Ting er ikke OO fordi om alt finnes i en(1) klasse..

PS!
Base bør jo også hete: Meters
og getMeters() bør være public

Sist endret av alpha01; 18. oktober 2016 kl. 10:16.

robhol

NOOOOOOOOOOOOOOOOOO-

6.423 5.657

18. oktober 2016

En oppgave du kan tenke på (som også kan gi et litt mer reellt "use case" for klasser) er å prøve å lage en klasse som kan konvertere mellom to forskjellige enheter - og der denne klassen ikke har spesifikk kunnskap om noen spesifikk enhet.

Edit: Jeg vil påstå at alpha11s eksempel er et steg i riktig retning, men at designet med én klasse per kombinasjon bør revurderes. Når du begynner å få noen enheter angrer du brått på det valget.

Sist endret av robhol; 18. oktober 2016 kl. 21:59.