Hei
tekemäni verkkokauppa alkaa olla jo aika hyvässä vaiheessa
latasin funktiotiedoston näkyville:
http://koti.mbnet.fi/ollins/muuta/functions.txt
Voisitteko antaa palautetta ko tiedostosta, esim. mitä pitäisi tehdä erilailla yms. ?
Olli
Tuo sinun kommentointitapasi on aika työläs, kun joudut määrittelemään funktion nimen ja parametrit kahdessa eri paikassa. Se johtaa väistämättä tilanteisiin kuten heti ensimmäisellä ruudullisella olevan getDetails:n kanssa, kun olet unohtanut päivittää toisen niistä, jolloin dokumentointi on heti väärässä.
Funktio getName() vaikuttaa huonosti suunnitellulta, koska sitä voi kutsua ilman argumentteja, mikä ei taida olla sallittu use-case.
Itse käyttäisin globaalin $yhteys-muuttujan sijaan funktiota, joka pitää sisällään staattisen instanssin yhteydestä.
The Alchemist kirjoitti:
Tuo sinun kommentointitapasi on aika työläs, kun joudut määrittelemään funktion nimen ja parametrit kahdessa eri paikassa. Se johtaa väistämättä tilanteisiin kuten heti ensimmäisellä ruudullisella olevan getDetails:n kanssa, kun olet unohtanut päivittää toisen niistä, jolloin dokumentointi on heti väärässä.
Joo tämän ongelman tiedostin ja arvelin että tuo kommentointi on aika turha
The Alchemist kirjoitti:
Funktio getName() vaikuttaa huonosti suunnitellulta, koska sitä voi kutsua ilman argumentteja, mikä ei taida olla sallittu use-case.
Taitaa käydä niin että jos argumentteja ei ole niin se palauttaa vaan tyhjää? Ja tuskinpa tulee tilannetta jossa argumentteja ei olisi.
The Alchemist kirjoitti:
Itse käyttäisin globaalin $yhteys-muuttujan sijaan funktiota, joka pitää sisällään staattisen instanssin yhteydestä.
Eli miten tuollaisen saisi sitten tehtyä?
Jotain tällaista olen itse käyttänyt:
Kiitos tuosta. Olisi vielä mukava kuulla lisää palautetta tuosta tiedostosta.
Yleisesti ottaen näyttää melkoiselta purkalta, jonka ylläpito tulee olemaan tarpeettoman hankalaa. Tuskin asia parilla vinkillä korjaantuu, mutta jos katsot koodia uudestaan esim. viiden vuoden kuluttua, ehkä ymmärrät, mitä tarkoitan.
Suomen ja englannin sekoittaminen esim. muuttujien nimeämisessä ei ole kovin tyylikästä. Koodin tasosta antaa ikävän ensivaikutelman jo se, että sisennykset ovat aivan sekaisin. Surkuhupaisa esimerkki huonosta koodista on funktio modifyCart.
Globaalit muuttujat $ostatus ja $pstatus ovat ongelma – suurempi ongelma kuin se $yhteys, koska nämä voi helposti tietämättään ylikirjoittaa *Status-funktioiden paluuarvolla.
Funktiot kannattaisi nimetä ja luokitella järkevästi. Esimerkiksi nyt koodissasi on globaali funktio isEmpty, jonka toimintaa ei voi mitenkään nimestä arvata. Cart-sana olisi tarpeen. Lisäksi funktioiden löytämistä auttaisi, jos nimet alkaisivat arvattavasti (cartIsEmpty, cartAdd) tai olisivat luokan tai nimiavaruuden sisällä (Cart::isEmpty, Cart::add).
Epämääräisten tekstivakioiden käyttö ("prod", "cat", "decr" jne.) on pahasta, varsinkin, jos ei viitsi edes kirjoittaa kokonaisia sanoja. Korvaa nämä vakioilla (define-funktion tai luokkien avulla, esim. URL_TYPE_SHOW_PRODUCT tai UrlType::SHOW_PRODUCT). Toisaalta monissa tilanteissa olisi vielä parempi tehdä eri toiminnoista erillisiä funktioita.
Lyhentely ei kannata, se vain sotkee koodia, etenkin, kun keksit itse lyhenteitä ("rmv" vs. yleisti käytetyt "rm" ja "del"). "Helpompi kirjoittaa" on huono perustelu: tuskinpa koodausnopeutesi on oikeasti tästä kiinni, ja jos tuota joku muu joutuu joskus korjailemaan, menee kohtalaisesti aikaa hukkaan, kun selvittää, mitä lyhenteesi tarkoittavat.
Epämääräisten lukujen käyttö on myös paha. Määrittele statuksetkin vakioiksi (OrderStatus::WAITING jne.). Laita status myös tietokantaan ENUM-sarakkeena tai erilliseen tauluun id–nimi-pareina; näin tietokannasta näkee todellisen tilanteen eikä kantaan voi tallentaa kuin määrättyjä arvoja.
Funktio seoText on hieman hutera. Jälkimmäisen str_replacen tilalle järkevämpi olisi preg_replace, joka muuttaa kaikki ei-sallitut merkit.
Ynnä muuta.
Kiitoksia palautteesta. Tarkoitatko että sisennykset pitäisi tehdä tabilla välilyönnin sijaan?
kirjoittaja kirjoitti:
Lisäksi funktioiden löytämistä auttaisi, jos nimet alkaisivat arvattavasti (cartIsEmpty, cartAdd) tai olisivat luokan tai nimiavaruuden sisällä (Cart::isEmpty, Cart::add).
Miten tuollaisen saa tehtyä ? Olisi aika hyödyllinen.
Ja mikä vika modifyCart -funktiossa on?
Ja kannattaisiko muuten eri sivujen funktiot jakaa omiin tiedostoihinsa?
Varmaan periaatteessa melko sama käytätkö sisennyksissä tabulaattoria vai välilyöntiä, kunhan et käytä kumpiakin sekaisin. En kuitenkaan usko että siitä oli kyse, vaan enemmänkin siitä että sisennykset menee miten sattuu..
Tyyliin:
try {
$yhteys = new PDO("mysql:host=********;dbname=*********", "**********", "********");
} catch (PDOException $e) {
die("VIRHE: " . $e->getMessage());
}vs
if($type == "prod"){
$table = "tuotteet";
} else {
$table = "tuoteryhmat";
}Ahaa tuo on ihan totta, voisin korjata tuon. Voisin ehkä laittaa näytille muitakin tiedostoja tuosta verkkokaupasta.
Ja olisi muuten kiva saada vielä tietää miten nuo nimiavaruudet oikein luodaan ja käytetään
Olli kirjoitti:
Ja mikä vika modifyCart -funktiossa on?
Tekstiparametrit, monta toimintoa samassa funktiossa, turha silmukka ja toistuvia koodirivejä if- ja else-lohkoissa. Muissakin funktioissa kierrät koko korin läpi etsiessäsi tiettyä tuotetta, vaikka asia selviäisi suoraan yhdellä empty-tarkistuksella.
Nimiavaruudet ovat PHP:ssä uusi ominaisuus (eivät toimi kaikkialla) ja minusta myös aika kömpelöitä, joten käyttäisin ennemmin luokkia. Alla on tästä esimerkki sekä samassa malli, miten jakaisin modifyCart-funktion kahteen osaan (add ja remove).
abstract class Cart {
// Julkinen funktio (public)
public static function get() {
return $_SESSION["cart"];
}
// Sisäinen apufunktio (private)
private static function set($cart) {
$_SESSION["cart"] = $cart;
}
// Julkinen funktio (public)
// $id = lisättävä tuote.
// $count = lisättävä määrä, oletuksena 1.
public static function add($id, $count = 1) {
$cart = self::get();
if (empty($cart[$id])) {
$cart[$id] = 0;
}
$cart[$id] += $count; // TODO: Tuotteiden määrän rajoitus.
self::set($cart);
}
// Julkinen funktio (public)
// $id = poistettava tuote.
// $count = poistettava määrä, null = kaikki.
public static function remove($id, $count = null) {
$cart = self::get();
if (!empty($cart[$id]) && $count) {
$cart[$id] -= $count;
}
if ($count === null || empty($cart[$id])) {
unset($cart[$id]);
}
self::set($cart);
}
}Vielä tyylikkäämpää olisi toki käyttää ihan oikeaa luokkaa (ei tällaista abstraktia nimiavaruuden korviketta), jolloin voisi koodiin kirjoittaa Cart::get()->add(plaa);
Jäi muuten hyvin epäselväksi, mitä varten cart-funktiosi on niin mutkikas (ja miksi pitää olla kaksi eri korimuuttujaa tilanteen mukaan). Ilmeisesti olet taas tunkenut liikaa asioita samaan funktioon. Tee yksi funktio, joka hakee korin, ja toinen funktio, joka laskee siitä hinnat ym. lisätiedot.
En kyllä ymmärrä miksi tuo on tehty noin kauhean vaikeaksi eli pitää krjoittaa "public static function" tai "private static function". Paljon yksinkertaisempaa on kirjoittaa vain "function"
Eli miksi tuo on tehty noin vaikeaksi?
Ja miksei "self" voi olla "this" ?
Ja mikä ero oikealla luokalla ja abstraktilla on?
Olli kirjoitti:
En kyllä ymmärrä miksi tuo on tehty noin kauhean vaikeaksi eli pitää krjoittaa "public static function" tai "private static function". Paljon yksinkertaisempaa on kirjoittaa vain "function"
https://www.ohjelmointiputka.net/oppaat/opas.
Ilman public-määrettä ne olis pelkkiä metodeja luokan sisällä. Käytän mieluummin sitä nimitystä kuin funktio, silloin kun on kyse luokista.
Pistemies on oikeilla jäljillä. Ilman public-näkyvyysaluemäärettä luokkafunktioille, ts. metodeille, luokille ja sen muuttujille oletetaan näkyvyysalue public. On hyvien koodauskonventioiden mukaista ilmaista tämä aina erikseen.
Yleisesti ilmaisten avainsanojen avulla määritellään ominaisuuksia luokille ja niiden jäsenille. Kts. yllä linkitetty opas sekä itse oppaassa linkitetty PHP:n manuaalisivu olio-ohjelmoinnista.
tsuriga kirjoitti:
On hyvien koodauskonventioiden mukaista ilmaista tämä aina erikseen.
Varsinkin kun tuo käytäntö vaihtelee kielittäin, eli monissa muissa kielissä oletus on private (joka on mielestäni loogisempi)
Olli kirjoitti:
En kyllä ymmärrä miksi tuo on tehty noin kauhean vaikeaksi eli pitää krjoittaa "public static function" tai "private static function". Paljon yksinkertaisempaa on kirjoittaa vain "function"
Eli miksi tuo on tehty noin vaikeaksi?
Tarkennetaan vielä vastausta tähän. Siksi koska ne on eri asia. Public-määreen voi periaatteessa php:ssa jättää pois koska se on oletusnäkyvyysalue, mutta sekin kannattaa pitää siellä mukana mainituista syistä. Static-avainsana sen sijaan ei ole oletusarvo joten se on pakko mainita jos haluaa metodistaan staattisen. Staattisilla metodeilla ei ole oliokontekstia vaan ne suoritetaan kyseisen luokan kontekstissa. Tuo esimerkkiluokka taas on määritelty abstraktiksi, joten sen metodien tuleekin olla staattisia, mikäli niitä haluaa mitenkään hyödyntää ilman perintää.
Olli kirjoitti:
Ja miksei "self" voi olla "this" ?
Koska nekin on eri asia. self viittaa kyseiseen luokkaan kun taas this-avainsanalla viitataan olioon. Staattisissa metodeissa ei ole oliokontekstia, joten niissä ei voi viitata mihinkään this:iä käyttäen. Saman luokan muihin staattisiin metodeihin tai luokan staattisiin muuttujiin voi sen sijaan viitata self:illä.
Metabolix kirjoitti:
Nimiavaruudet ovat PHP:ssä uusi ominaisuus (eivät toimi kaikkialla) ja minusta myös aika kömpelöitä, joten käyttäisin ennemmin luokkia.
Ei ne nyt ihan tuore juttu ole kun yli kaksi vuotta sitten on niitä tukeva stable release (5.3) julkaistu ja ei kai niiden suhteen muita isompia ongelmia tomivuudessa ole kuin se että monet web hotellit eivät tykkää päivittää php-versioitaan kovinkaan tiheään. Pointtina siis että jos käytössä on php 5.3 tai uudempi ja koodin ei tarvitse toimia vanhemmilla versioilla, niin toimimattomuus tuskin on syy olla käyttämättä nimiavaruuksia.
Entä mitä tarkoitat kömpelyydellä? Eihän nimiavaruudet ole luokkien käytön kanssa mitenkään toisiaan poissulkevia tai vaihtoehtoisia menetelmiä, vaan täydentävät toisiaan. Minusta ainakin on kätevämpää hoitaa asia nimiavaruuksilla kuin kirjoittaa luokkien nimiä tyyliin Zend_Db_Adapter_Pdo_Mysql tai Swift_Mime_ContentEncoder_Base64ContentEncoder varsinkin kun ide lisää tarvittavat use-statementit automaattisesti.
Jännä mahdollisuus on myös kirjoittaa tavallisia funktioita omiin nimiavaruuksiisa globaalin nimiavaruuden sijaan esim. käyttötarkoituksen mukaan, mutta se menee kyllä jo luokkien käytön kanssa jonkin verran päällekkäiseksi käyttötavaksi. Ehkä se voisi olla jopa siistimpi tapa kuin tehdä tuollaisia pelkästään staattisia metodeja sisältäviä luokkia.
Tällä hetkellä 5.3 13,8% ja 5.2 79,3% sivustoista.
Eli toki jos tosiaan käytössä on 5.3 ja ei tarvitse tukea muita ympäristöjä niin go for it. Silti mielestäni ihan validi pointti vielä että "ei toimi kaikkialla".
Tukki kirjoitti:
Entä mitä tarkoitat [nimiavaruuksien] kömpelyydellä?
Minusta PHP:n merkintätapa nimiavaruuksille on ruma ja epäkäytännöllinen. Taas keksittiin aivan turhaan uusi syntaksi sen sijaan, että olisi kopioitu C++:n tapa (kuten luokkien merkinnöissä :: ja ->) tai edes matkittu Javaa (kuten luokkien jäsenten määreissä).
Teknisiä pulmia PHP:n nimiavaruuksissa taas ovat muuttujien puute (ovat hämäävästi aina globaalissa nimiavaruudessa) ja se järjenvastainen viritelmä, että samassa tiedostossa ei voi olla nimiavaruuksia ja niiden ulkopuolista koodia mutta voi silti olla monta eri nimiavaruutta. En ole PHP:n tekniikkaan tutustunut, mutta tuntuisi, että heillekin on ollut tästä kikkailusta ylimääräistä vaivaa.
Kunnolla toteutetut nimiavaruudet (Java, C++) ovat hyvä ja tarpeellinen työkalu, mutta PHP:ssä minusta homma meni vähän pieleen.
Grez kirjoitti:
Tällä hetkellä 5.3 13,8% ja 5.2 79,3% sivustoista.
Laitappa vastaavat luvut niiden sivustojen osalta joiden ylläpitäjällä on käytännössä mahdollisuus vaikuttaa käytettävään php versioon ja jotka tietävät mitä nimiavaruudet ovat ja joiden koodin ei tarvitse toimia vanhemmilla versioilla :)
Grez kirjoitti:
Eli toki jos tosiaan käytössä on 5.3 ja ei tarvitse tukea muita ympäristöjä niin go for it. Silti mielestäni ihan validi pointti vielä että "ei toimi kaikkialla".
"Ei toimi" on minusta vähän hämäävä ilmaisu. Ikäänkuin toteutus olisi jotenkin rikki tai buginen riippumatta siitä onko ominaisuus ylipäänsä tuettu. Validi pointti on se että monissa web hotelleissa on käytössä vanha nimiavaruuksia tukematon versio php:stä.
Metabolix kirjoitti:
Minusta PHP:n merkintätapa nimiavaruuksille on ruma ja epäkäytännöllinen. Taas keksittiin aivan turhaan uusi syntaksi sen sijaan, että olisi kopioitu C++:n tapa (kuten luokkien merkinnöissä :: ja ->) tai edes matkittu Javaa (kuten luokkien jäsenten määreissä).
Syntaksista väitteleminen menee vähän epäoleelliseksi varsinkin kun koko erotinmerkkiä ei juuri tarvitse koodatessa edes kirjoittaa jos ide hoitaa use-lauseet.
Metabolix kirjoitti:
Teknisiä pulmia PHP:n nimiavaruuksissa taas ovat muuttujien puute (ovat hämäävästi aina globaalissa nimiavaruudessa) ja se järjenvastainen viritelmä, että samassa tiedostossa ei voi olla nimiavaruuksia ja niiden ulkopuolista koodia mutta voi silti olla monta eri nimiavaruutta. En ole PHP:n tekniikkaan tutustunut, mutta tuntuisi, että heillekin on ollut tästä kikkailusta ylimääräistä vaivaa.
Kunnolla toteutetut nimiavaruudet (Java, C++) ovat hyvä ja tarpeellinen työkalu, mutta PHP:ssä minusta homma meni vähän pieleen.
Jos nimiavaruuksien ulkopuolisella koodilla tarkoitetaan globaalissa nimiavaruudessa olevaa koodia niin kyllähän se php:ssa onnistuu samassa tiedostossa muiden nimiavaruuksien kanssa. Kovin järkevältä se ei silti kuulosta. Php:n manuaalissakin sanotaan että useampien nimiavaruuksien sisällyttäminen samaan tiedostoon on erittäin epäsuositeltavaa tiettyjä erityistilanteita lukuunottamatta.
Itse olen nyt puolisen vuotta koodannut php:ta nimiavaruuksia käyttäen enkä ole pahempia puutteita tai ongelmia havainnut. Oma koodini on tosin 99,99% oliopohjaista, joten esim muuttujien (globaalien?) nimiavaruudettomuus ei ole tullut vastaan. Myös esim. symfonyn, zend frameworkin, doctrinen, monologin ja imaginen tuoreet versiot käyttävät nimiavaruuksia joten ei kai toteutus ihan susi voi olla varsinkin hyödyt huomioiden.
Tukki kirjoitti:
Koko erotinmerkkiä ei juuri tarvitse koodatessa edes kirjoittaa jos ide hoitaa use-lauseet.
Innokkaalla use-lauseiden käytöllä taas menetetään se hyöty, josta tämä keskustelu lähti: että koodari näkisi suoraan, että isEmpty on nimenomaan Cart\isEmpty. Eli jos lähdetään tuolle linjalle nimiavaruuksien suhteen (kuten usein kannattaakin), pitää joka tapauksessa tehdä myös ostoskorista luokka, jolloin toki Cart-niminen nimiavaruus olisi aika höpsö.
Tukki kirjoitti:
Jos nimiavaruuksien ulkopuolisella koodilla tarkoitetaan globaalissa nimiavaruudessa olevaa koodia niin kyllähän se php:ssa onnistuu samassa tiedostossa muiden nimiavaruuksien kanssa.
Kyllä ja ei.
<?php
namespace X {}
namespace {
// Näin kyllä.
echo "ok\n";
}
// Näin ei.
echo "fail\n";
// PHP Fatal error: No code may exist outside of namespace {} in - on line 8Minusta tämä nimetön namespace {} on peruskoodarin kannalta aivan tarpeeton mutka matkassa.
Tukki kirjoitti:
Kovin järkevältä se ei silti kuulosta.
Jos syntaksin perusteella jotain pitäisi pystyä tekemään eikä asiassa ole mitään epäselvyyttä, ei kuulosta kovin järkevältä erikseen estääkään.
Tukki kirjoitti:
Itse olen nyt puolisen vuotta koodannut php:ta nimiavaruuksia käyttäen enkä ole pahempia puutteita tai ongelmia havainnut.
En väitäkään, että nimiavaruudet olisivat täysin käyttökelvottomia tai aiheuttaisivat virheitä oikein kirjoitettuun koodiin. Vertaan vain toimintaa muihin kieliin, joissa asiat ovat minusta paremmin. Ehkä ymmärrät näkökantani paremmin, jos joskus koodaat enemmän niillä muilla kielillä.
Tukki kirjoitti:
Oma koodini on tosin 99,99% oliopohjaista, joten esim muuttujien (globaalien?) nimiavaruudettomuus ei ole tullut vastaan.
Valitettavasti suuri osa maailman PHP-koodista ei ole yhtä hyvää, vaan PHP:llä koodaavat paljon juuri aloittelijat, jotka käyttävät globaaleja muuttujia aivan huomaamattaan ja tiedostamatta niiden riskejä (ks. Ollin $ostatus). Juuri heidän kannaltaan tämä nimiavaruusratkaisu on minusta erityisen huono; taitava koodari tietysti sopeutuu mihin tahansa.
Tukki kirjoitti:
Ei kai toteutus ihan susi voi olla
Ei, eikä PHP:kään voi olla aivan huono kieli, kun sitä niin moni käyttää, vai mitä? Kuitenkin saisit varmasti tälläkin sivustolla kymmeniltä kokeneilta koodareilta lausunnon, että PHP on melkoinen purkkapallo moniin muihin nähden. :)
Sanotaan sitten vaikka, että PHP:n namespacet kaikkine vikoineen sopivat ihan kivasti kieleen, joka on muutenkin täynnä outoja kikkoja ja rajoittuneita ratkaisuja. Ei ole ensimmäinen eikä varmasti viimeinenkään asia listalla PHP:n kummallisuuksista.
Metabolix kirjoitti:
Tukki kirjoitti:
Myös esim. symfonyn, zend frameworkin, doctrinen, monologin ja imaginen tuoreet versiot käyttävät nimiavaruuksia joten ei kai toteutus ihan susi voi olla
Ei, eikä PHP:kään voi olla aivan huono kieli, kun sitä niin moni käyttää, vai mitä? Kuitenkin saisit varmasti tälläkin sivustolla kymmeniltä kokeneilta koodareilta lausunnon, että PHP on melkoinen purkkapallo moniin muihin nähden. :)
Ja vastaavasti ne (10 - koodarin) mielipiteet täytyvät pitää paikkansa. Kaikki on suhteellista..
Tuskin löydät yhtään koodaria, joka on aktiivisesti koodaillut useilla kielillä ja pitäisi PHP:tä tyylikkäänä ja selkeänä kielenä.
Grez kirjoitti:
Tuskin löydät yhtään koodaria, joka on aktiivisesti koodaillut useilla kielillä ja pitäisi PHP:tä tyylikkäänä ja selkeänä kielenä.
Njoo.. Mut onhan se tyylikkyys kiinni (myös) paljolti siittä mitä näytön ja penkin välissä on. Omasta kokemuksestani ne jotka kritisoivat php:tä, ovat yleensä juuri niitä "kokeneita koodareita" jotka ovat muodostaneet näkemyksensä php:stä sen (php:n) ensi taipaleella, eivätkä omaisi (välttämättä) samaa mielipidettä jos näkisivät nykytilanteen ilman ennakkoluuloja. Toki! Jotta ei tämä muuttuisi väittelyksi, myönnän olevani väärässä ja sinä oikeassa.
Metabolix kirjoitti:
... ja se järjenvastainen viritelmä, että samassa tiedostossa ei voi olla nimiavaruuksia ja niiden ulkopuolista koodia mutta voi silti olla monta eri nimiavaruutta.
Mielestäni se vasta ON suoraviivaista ja loogista että samassa tiedostossa ei voi olla ulkopuolista koodia, jos se ei ole toisessa avaruudessa. Itse kun käytän n-avaruuksia (harvemmin) pidättäydyn yhden avaruuden yhden tiedoston tyylissä. Se olisi mielestäni hämmentävää jos tiedosto sisältäisi avaruuden lisäksi ulkopuolista koodia. Ja mielestäni nimiavaruuksien käyttö tuossa tapauksessa olisi vähintään yhtä hyvä vaihtoehto kuin esim staattinen lähestymistapa. Nimet selkeytyisivät, ja pelko päällekkäisyyksistä häipyisi.
<?php
namespace tuote;
function tiedot($tuoteId) {
//...
}
function varastossa() {
//...
}
function hinta() {
//...
}
function lisaa() {
//...
}
/**
$tuote = tuote\tiedot(132);
**/
?>Itse väitän ainakin 5.3:n pohjalta että omaisivat saman mielipiteen (että useat muut kielet on paremmin suunniteltuja, loogisemmin toimivia, tyylikkäämpiä ja selkeämpiä) kuin PHP. Toisaalta ei PHP näissä asioissa varmasti kaikkein huonoinkaan ole. En ole hirveästi tutustunut 5.5 tai 6.0 versioihin, jos niitä tarkoitit nykytilanteella.
PHP on ihan hyvä moneen asiaan, mutta noi nyt vaan ei ole sen vahvuuksia. Sillä on myös historian painolastinsa, kun ovat kuitenkin pyrkineet säilyttämään yhteensopivuutta taaksepäin. Todennäköisesti jos PHP:n tekijät lähtisi suunnittelemaan sitä nyt puhtaalta pöydältä, niin tekisivät monia asioita eri tavalla. Ja onhan monia asioita toki muutettukin.
Heippa taas!
Itse olen aina kokenut PHP-koodin vääntämisen jotenkin oksettavana, mutta on vain niin, että jossain tilaneteissa sen käyttö on kyllä nopein tie 'onnelaan'...
No sie nyt kyllä väännät välillä sellasta purkkaa muutenkin :D. Ei niinko pahalla, mutta melkoisia wall of code -pasteemuksia nähneenä, varsinkin ne JavaScript-pätkäs joskus tuntuu olevan ihan suoraan tehty quick & dirty -ratkaisu mielessä. Toki samaa mieltä yleisesti ottaen – PHP:ssä on omat heikkoutensa ja erikoisuutensa.
Qeijon lähestymistapa nimiavaruuksiin on... mielenkiintoinen, taas sitä oikomista. Ei niitä ole tehty luokkia korvaamaan. Jos nyt kummiski pysyttäs aiheessa ja mode poistas turhat löpinät, mukaanlukien tämän viestin.
Ei tämä löpinä haittaa,mutta ihmettelen edelleenkin sitä miksi nuo luokat on tehty niin vaikeaksi. Olisi hyvä olla jokin selkeä opas josta voisin opiskella ne.
tsuriga kirjoitti:
Qeijon lähestymistapa nimiavaruuksiin on... mielenkiintoinen
Otin asian esille (näin "mielenkiintoisella" tavalla) koska täällä ihmeteltiin nimiavaruuksien "epäkäytännöllisyyttä", tarkoituksen oli osoittaa että se on vähintään yhtä suoraviivainen kuin Metabolixin abstrakti - luokka. Itse en lähtökohtaisesti käyttäisi nimiavaruuksia tuolla tavalla, mutta voisin harkita.
tsuriga kirjoitti:
Ei niitä ole tehty luokkia korvaamaan
Noei.
Olli kirjoitti:
miksi nuo luokat on tehty niin vaikeaksi
Olio-ohjelmointi pitäisi periaatteiltaan kyllä olla hyvin intuitiivinen paradigma. Toki erilaiset avainsanat hämmentävät soppaa, mutta kannattaa opetellessaan jaotella opeteltavat asiat pienempiin kokonaisuuksiin sen sijaan, että lähtee hakkaamaan päätä seinään mahdollisimman pitkän metodimäärittelyrivin kanssa. Jos vaan englanti taittuu niin eikun PHP:n manuaalia lukemaan, tuolla näin on tehty. Myös jostain Ohjelmointiputkan oppaasta saattaa löytyä ihan olio-ohjelmoinnin perusteita ilman kielisidonnaisuutta kun vaan jaksaa kaivella. Mahdollisesti :). Samat perusperiaatteet pätevät kaikkiin (citation needed) "olio-ohjelmointikieliin" vaikka avainsanat muuttuisivatkin. PHP:ssä avainsanojen joukko on vielä pieni verratuna esimerkiksi C#:iin.
Niin no C#:ssa nyt tulee parikymmentä avainsanaa jo natiivitietotyypeistä ja LINQsta, joita ei ole olemassakaan PHP:ssä.
77 avainsanaa 1.1 frameworkissa, 78 2.0:ssa, 87 3.0:ssa (suurin osa 3.0:n uusista tulee LINQ:sta)
PHP:ssä 49. Ero ei mielestäni ole mitenkään suuren suuri.
Toisaalta PHP:ssä on ihan turhaan roskaan hukattu avainsanoja, kuten nyt vaikka kaksi erilaista tapaa tehdä if -lauseita, joista toista varten on endif avainsana.
Tarkennuksena: tarkoitin tässä lähinnä luokkien ja ominaisuuksien etumääreitä. On sealed, dynamic, delegate, implicit, volatile, virtual, unsafe, async & await (C# 5)... joita ei PHP:stä löydy. Näinpä opettelun kannalta PHP:n määrekenttä on vielä yksinkertainen.
No toisaalta vaikka niitä on enemmän, niin mielestäni ne on ehkä jopa selkeämmät. Ja niitähän tarvitaan ominaisuuksiin, joita PHP:ssä ei ole.
Suattaphan ne joissain tilanteissa selkeyttää jos verrataan PHP:n dynaamisuudesta (ja muista erikoisuuksista, mm. override vs. overload) aiheutuvaan ailahtelevaisuuteen, mutta enemmän niissä on silti opeteltavaa jos ja tässä kun lähtökohtana on se, että etumääreet ja niiden määrä vaikuttavat hankalilta. Kuten itsekin totesit, niitä tarvitaan suurimmissa määrin PHP:ssä esiintymättömiin ominaisuuksiin.
Niin no mun pointti oli lähinnä siinä se, että jos ei tarvitse ominaisuuksia joita ei PHP:ssäkään ole, niin ei niitä kaikkia avainsanojakaan tarvitse heti opetella. Jos haluat tehdä esimerkiksi vastaavaa asynkronista settiä PHP:llä, mitä asyncillä ja awaitilla voi tehdä, niin pääset opettelemaan vielä vähän enemmän kuin nuo pari avainsanaa.
Nojoo, voi vaan tulla yllättäen ja täysin pyytämättä (TM) eteen toisten koodeja lukiessa.
Disclaimer: Ei siis ollut tarkoitus antaa sellaista kuvaa, että pitäisin suurta määreiden määrää huonona asiana. Samat olio-ohjelmoinnin perusperiaatteet voi opetella vaikka C#:lla ja ne pätevät myös PHP-maailmassa, molemmat kun ovat taineet ottaa vaikutteita Javasta?
Miten esimerkiksi tuon funktion getDetails saisi luokkaan?
Pitäisikö sitä käyttää sitten tyyliin
prod::details("Nimi/ID/Seonimi", "seo/normal/id");
En oikein kyllä käsitä nuita luokkia php:n oppaan katsomisen jälkeenkään.
Olisiko kellään vastausta kysymykseen?
Olli kirjoitti:
Miten esimerkiksi tuon funktion getDetails saisi luokkaan?
Lue nyt edes joku perus OOP - opas.. kysele sitten..
Ok. Olisiko tämä parempi koodi tuolle getDetails-funktiolle?
abstract class details {
global $yhteys;
$table = getTable($type);
public static function id($id, $type = "prod"){
$kysely = $yhteys->prepare("SELECT * FROM ".$table." WHERE id = ?");
$kysely->execute(array($id));
}
public static function seo($id, $type = "prod"){
$kysely = $yhteys->prepare("SELECT * FROM ".$table." WHERE seonimi = ?");
$kysely->execute(array($id));
}
public static function normal($id, $type = "prod"){
$kysely = $yhteys->prepare("SELECT * FROM ".$table." WHERE nimi = ?");
$kysely->execute(array($id));
}
$tulos = $kysely->fetch();
$maara = $kysely->rowCount();
if($maara == "maara"){return array($maara, $tulos); }else {return $tulos;}
}Olen vähän epävarma, että miten tuo execute tulisi sijoittaa, eli voisiko sen sijoittaa yhteen paikkaan. Onko tuo ylläoleva yleensä edes järkevä tapa toteuttaa tuota?
Vie mielummin $yhteys __constructorille instanssina eikä globaalina.. Muutenkin voisi olla parempi tehdä functioista "oikea" luokka.
class details {
private $yhteys;
public function __construct(PDO $yhteys) {
$this->yhteys = $yhteys;
}
//..
}
$tuotteet = new details(PDOConnection::connect(DBUSER,DBPASS,DB));
abstract class PDOConnection
{
public static function connect($user = "", $pass = "", $db = "", $host = "localhost")
{
try {
$connection = new PDO("mysql:host=" . $host . ";dbname=" . $db, $user, $pass);
$connection->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
return $connection;
}
catch (PDOException $e) {
throw $e;
}
}
}Mutta ei tuota pdo-yhteyttä kannata useaa kertaa tehdä.
Eihän tuossa luodakkaan yhteyttä moneen kertaan?
No siinä luodaan uusi yhteys joka kerta kun tehdään uusi $tuotteet.
No eihän sun tarvitse luoda kuin yksi. Laitat details-luokkaan tarvittavat tiedot tuotteista (tai teet tuotteille vielä oman luokan).
Olli kirjoitti:
No siinä luodaan uusi yhteys joka kerta kun tehdään uusi $tuotteet.
Aivan oikein. Mutta yksi instanssi riittää.
<?php
$tuotteet = new details(PDOConnection::connect(DBUSER,DBPASS,DB));
$tuotteet->joku($jokuId);
$tuotteet->jokuToinen("kaija");
$tuotteet->jokuKolmas("jukka");
?>Jos halutaan tehdä luokka, jonka metodeita käytetään vain staattisesti, olisi semanttisempaa määritellä sen konstruktori privaatiksi. Abstract-määre viittaisi siihen, että luokka tarjoaa pohjarakenteen siitä periytettäville luokille. Parasta toki olisi, jos PHP:ssä voisi määritellä luokan staattiseksi. Catchattua virhettä ei myöskään ole järkeä heittää uudelleen (paitsi joissain erikoistapauksissa, jotka kannattaisi mainita tarvittaessa).
tsuriga kirjoitti:
Abstract-määre viittaisi siihen, että luokka tarjoaa pohjarakenteen siitä periytettäville luokille.
Abstract - määre myös estää luokan suoran käytön, joka on mielestäni tärkeämpi asia.
tsuriga kirjoitti:
Catchattua virhettä ei myöskään ole järkeä heittää uudelleen
Miksi ei?! Tottakai se kannattaa heittää uudelleen jos et halua käsitellä ko. virhettä siinä ja heti? Ei mun mielestä se mikään erikoinen asia ole..
Minusta tässä ei kyllä ole hirveästi järkeä paitsi ehkä työkaluista riippuen debugatessa jos laittaa siihen breaklinen
try {
//Jotain
}
catch (PDOException $e) {
throw $e;
}Eikö täsmälleen saman saisi aikaan olemalla laittamatta try-catch-lohkoa?
Jos tuolla catchin sisällä olisi edes "//TODO: lisää virhekäsittely" tai se tekisi jotain ennen kuin antaisi sen jatkokäsittelyyn, niin sitten siinä olisi jotain ideaa.
Mutta haluaisin että joudun tekemään yhden yhteyden muillekin tyypeille, kuten esim. ostoskori, osoitteiden luonti ym ym
Grez kirjoitti:
Minusta tässä ei kyllä ole hirveästi järkeä paitsi ehkä työkaluista riippuen debugatessa jos laittaa siihen breaklinen
Noh. Pakko myöntää että juuri tuossa esimerkissä juuri tolla tavalla käytettynä toinen blokki on turha. Mutta näin yleisesti ottaen en pidä tota käytäntöä (Uudelleen throw) mitenkään erikoisena asiana..
qeijo kirjoitti:
Abstract - määrä myös estää luokan suoran käytön, joka on mielestäni tärkeämpi asia.
Eikä estä. Se estää ainoastaan sen ettei luokasta voi luoda olioita, mutta silti sen staattisia metodeja voi kutsua ja sen voi periä. Ylipäänsä abstrakti luokka tuntuu minusta väärältä tavalta toteuttaa tuollainen staattisten funktioiden kirjasto.
Mielummin vaikka tavallinen luokka staatisila metodeilla jolla ei ole julkista konstruktoria ja tarvittaessa vielä final ettei perimälläkää saa luotua olioita siitä. Vielä siistimpää minusta olisi oikeastan välttää tuollaisia funktiokokoelmia kokonaan ja toteuttaa toiminnot tavallisisla luokilla.
qeijo kirjoitti:
tsuriga kirjoitti:
Catchattua virhettä ei myöskään ole järkeä heittää uudelleen
Miski ei?! Tottakai se kannattaa heittää uudelleen jos et halua käsitellä ko. virhettä siinä ja heti? Ei mun mielestä se mikään erikoinen asia ole..
Yksi hyvä syy grezin mainitseman lisäksi on se että udelleenheittäminen hävittää poikkeuksen alkuperäisen esiintymiskohdan (tiedoston ja rivinueron).
Tukki kirjoitti:
Eikä estä. Se estää ainoastaan sen ettei luokasta voi luoda olioita
Sama asia..
Abstract - määre myös estää luokan suoran käytön == Se estää ainoastaan sen ettei luokasta voi luoda olioita.
Mitä sä luulit et mä tarkoitin "suoralla käytöllä".
Tukki kirjoitti:
mutta silti sen staattisia metodeja voi kutsua
Joka on sen käyttötarkoitus..
Tukki kirjoitti:
Yksi hyvä syy grezin mainitseman lisäksi on se että udelleenheittäminen hävittää poikkeuksen alkuperäisen esiintymiskohdan (tiedoston ja rivinueron).
Eikä menetä.. Se ei ole uusi poikkeus.. Jos se olisi:
catch (PDOException $e) {
throw new Exception ($e);
}Niin se heittäis kokonaan uuden poikkeuksen.. jne.
Olli kirjoitti:
Mutta haluaisin että joudun tekemään yhden yhteyden muillekin tyypeille, kuten esim. ostoskori, osoitteiden luonti ym ym
En ihan täysin tajua mitä tässä haetaan, mutta olisiko singleton-pattern ratkaisu ongelmaan?
qeijo kirjoitti:
Mutta näin yleisesti ottaen en pidä tota käytäntöä (Uudelleen throw) mitenkään erikoisena asiana..
Joo, ei uudelleenviskominen ollutkaan se kritiikin kohde vaan se, että ylipäätään kopataan poikkeus jos ei ole aikomustakaan koskea siihen edes pitkällä tikulla.
qeijo kirjoitti:
Eikä menetä.. Se ei ole uusi poikkeus.. Jos se olisi:
catch (PDOException $e) { throw new Exception ($e); }Niin se heittäis kokonaan uuden poikkeuksen.. jne.
Totta, sekoitin tämän siihen että wrapataan poikkeusen viesti toisen poikkeuksen viestiin näin.
} catch(InvalidArgumentException $iae) {
throw new Exception("foo".$iae->getMessage());
}Omilla php-asetuksillani tuo poikkeuksen antaminen kokonaan uuden poikkeuksen konstruktorille säilyttää myös alkuperäisen esiintymiskohdan.
Tukki kirjoitti:
Olli kirjoitti:
Mutta haluaisin että joudun tekemään yhden yhteyden muillekin tyypeille, kuten esim. ostoskori, osoitteiden luonti ym ym
En ihan täysin tajua mitä tässä haetaan, mutta olisiko singleton-pattern ratkaisu ongelmaan?
Ehkä on simppelimpää vain käyttää tuota yhteys-muuttujaa ja asettaa globaaliksi. Mikä vika siinä on?
qeijo kirjoitti:
Abstract - määre myös estää luokan suoran käytön == Se estää ainoastaan sen ettei luokasta voi luoda olioita.
Huomannethan, että puhuin semanttisuudesta, ja mainitsin, mitä abstract-määre ilmaisee. Se, että jollakin määreellä voidaan saavuttaa jokin ominaisuus ei tarkoita sitä, että niin kannattaisi tehdä. Tässä tapauksessa abstract antaa luokan toiminnasta väärän kuvan. Komppaan tässä Tukkia täysin.
Virheen uudelleenheittämisessä esitin asian huonosti, oli tosiaan tarkoitus kritisoida vain uudelleenheittoa ilman muuta käsittelyä sen ollessa redundanttia. Muistaakseni TDWTF:stä löytyy tästä juttu ja hyvä keskustelu kommenteissa.
Olli kirjoitti:
Ehkä on simppelimpää vain käyttää tuota yhteys-muuttujaa ja asettaa globaaliksi. Mikä vika siinä on?
Vika boldattu. Kts. The Clean Code Talks - "Global State and Singletons".
tsuriga kirjoitti:
qeijo kirjoitti:
Abstract - määre myös estää luokan suoran käytön == Se estää ainoastaan sen ettei luokasta voi luoda olioita.
Huomannethan, että puhuin semanttisuudesta, ja mainitsin, mitä
abstract-määre ilmaisee. Se, että jollakin määreellä voidaan saavuttaa jokin ominaisuus ei tarkoita sitä, että niin kannattaisi tehdä. Tässä tapauksessaabstractantaa luokan toiminnasta väärän kuvan. Komppaan tässä Tukkia täysin.Virheen uudelleenheittämisessä esitin asian huonosti, oli tosiaan tarkoitus kritisoida vain uudelleenheittoa ilman muuta käsittelyä sen ollessa redundanttia. Muistaakseni TDWTF:stä löytyy tästä juttu ja hyvä keskustelu kommenteissa.
Olli kirjoitti:
Ehkä on simppelimpää vain käyttää tuota yhteys-muuttujaa ja asettaa globaaliksi. Mikä vika siinä on?
Vika boldattu. Kts. The Clean Code Talks - "Global State and Singletons".
Mikä vika globaaleiden käytössä sitten on ? Metabolix sanoi:
https://www.ohjelmointiputka.net/keskustelu/
...suurempi ongelma kuin se $yhteys, koska nämä voi helposti tietämättään ylikirjoittaa *Status-funktioiden paluuarvolla.
Eli miksen voi käyttää globaaleita ?
Rupesin nyt kokeilemaan tuota funktion tekoa mutta ongelmia tuli heti.
class db {
public function connect() {
$yhteys = new PDO("mysql:host=*****;dbname=*****", "*******", "******");
}
public function get($query, $parameters){
$prepare = $yhteys->prepare($query);
$prepare->execute($parameters);
$result = $prepare->fetch();
$count = $prepare->rowCount();
return array($result, $count);
}
}$db = new db();
$content = $db->get("SELECT * FROM tuotteet WHERE id = ?", array("1"));
print $content;En ymmärrä virheilmoitusta: Parse error: syntax error, unexpected T_OBJECT_OPERATOR
Mikä tuossa oikein on vikana?
Olli kirjoitti:
En ymmärrä virheilmoitusta:
Parse error: syntax error, unexpected T_OBJECT_OPERATOR
Mikä tuossa oikein on vikana?
Käytät get-metodissasi alustamatonta muuttujaa $yhteys oliona.
Nyt alustan sen ja mitään virheilmoitust ei tule:
class db {
private $yhteys;
public function connect() {
$yhteys = new PDO(".....");
}
public function get($query, $parameters){
$prepare = $yhteys->prepare($query);
$prepare->execute($parameters);
$result = $prepare->fetch();
$count = $prepare->rowCount();
return array($result, $count);
}
}$db = new db();
$content = $db->get("SELECT * FROM tuotteet WHERE id = ?", array("1"));Heippa taas!
Lähes valmiilta vaikuttaa tämän verkkokaupan toteutus, että tsemppiä vaan...
hei,
ei tuo vielä läheskään valmis ole, Osaisitko sinäkään auttaa tuossa ongelmassa?
No nyt alustat luokan jäsenmuuttujan $yhteys null:iksi mutta et käytä sitä missään. Jäsenmuuttujiin viitataan luokan sisällä $this-> -etuliitteellä. Lisäksi et kutsu funktiota connect() missään vaiheessa vaikka näyttäisi siltä että tuo $yhteys olisi tarkoitus alustaa siellä.
Alustukset kannattaa yleensä hoitaa konstruktorissa, jos luokan muut metodit vaativat ja olettavat että tietyt muuttujat on alustettu tietyllä tavalla niinkuin tässä tapauksessa tuo $yhteys.
Sain nyt nuo ongelmat ratkaistua.
Kysyisin nyt erityisesti Metabolixilta että onko tässä parempi tapa tehdä getDetails-funktio:
class db {
private $connection;
function __construct() {
try {
$this->connection = new PDO("******");
} catch (PDOException $e) {
echo 'Yhteysvirhe: ' . $e->getMessage();
}
}
public function get($query, $parameters, $method = 'all'){
$prepare = $this->connection->prepare($query);
$prepare->execute($parameters);
if($method == "all"){
$result = $prepare->fetchAll();
} else {
$result = $prepare->fetch();
}
$count = $prepare->rowCount();
return array($result, $count);
}
}
class product {
function __construct($db) {
$this->db = $db;
}
public function getId($from, $id){
switch($from){
case "seo":
$query = "SELECT * FROM tuotteet WHERE id = ?";
break;
case "normal":
$query = "SELECT * FROM tuotteet WHERE nimi = ?";
break;
}
$result = $this->$db->get($query, array($id));
return $result[0];
}
public function details($id){
$query = "SELECT * FROM tuotteet WHERE id = ?";
$result = $this->$db->get($query, array($id));
return $result;
}
}Koodiasi on aika hankala arvioida, kun se on niin täynnä virheitä, ettei sitä saa läheskään tuollaisessa muodossa edes toimimaan. Mutta niiltä osin, joilta tuota nyt voi arvioida, en näe mitään parannusta aiempaan nähden, päin vastoin: käytät luokkia todella kummallisesti (olio-ohjelmoinnin näkökulmasta "väärin") etkä ole korjannut mitään varsinaisista ongelmista vaan lisännyt vain turhaa sotkua ympärille. Funktion nimi, josta tämä koko luokka-asia lähti liikkeelle, on tuossa kokonaisuudessa jo aivan sivuseikka.
Olio-ohjelmointi ei ole mitenkään välttämätöntä, enkä ole tässä keskustelussa missään vaiheessa edes suositellut sitä, vaan suosittelin vain loogisempaa nimeämistä ja itse funktioiden toiminnan korjaamista. Voi olla vaikea pysyä juonessa mukana, kun eri henkilöiltä tulee erilaisia neuvoja, joten ehkä kannattaa lukea tämä keskustelu uudestaan läpi niin, että yrität lukea kerralla vain yhden henkilön neuvot kokonaisuutena välittämättä siitä, mitä muut ovat välissä sanoneet.
Jos luokkien tai nimiavaruuksien käyttö tuntuu aivan kohtuuttoman vaikealta (kuten nyt näyttää), voit nimetä funktiosi helpommin vain lisäämällä alkuun aihepiirin: productGetDetails, cartIsEmpty jne. Jos haluat käyttää luokkia vain nimen selventämiseen (kuten alussa ehdotin), kannattaa lukea ihan ajatuksella läpi se ensimmäinen koodini, jossa asiaa esittelin. Siinä on malliksi jo monta cart-aiheista funktiota.
Tiivistelmä alkuperäisestä ehdotuksestani (luokan käytöstä nimeämiseen):
Vanha koodi:
function isEmpty() {
// ...
}Lisättävät osat:
class Cart {
public static // ... Vanha funktio tähän ...
}Muokattu koodi:
class Cart {
public static function isEmpty() {
// ...
}
}Hei,
korjasin nyt virheitä koodista. Päivitin koodin suoraan tuohon edelliseen viestiin.
Ja haluan muutenkin opetella luokkien käytön joten tässä se on hyvä opetella.
Mutta onko tuossa koodissa siis edelleenkin virheitä. En kyllä oikeasti ymmärrä mikä tuossa vielä voisi olla väärin. Minun mielestä tuo on vain huomattavasti selvempi kuin edelliset.
Metabolix, tuliko tuo uudemepi viestini perille. Tai saattaa olla että nuitten luokkien laittaminen on turhan vaikeaa. Taidan mennä eteenpäin vanhalla siis , koska se toimii ihan riittävän hyvin.
Minusta on ihan turha kommentoida koodiasi yhtään enempää niin kauan, kuin siinä on yksinkertaisia virheitä, jotka pitäisi helposti pystyä korjaamaan minkä tahansa PHP:n perusoppaan avulla. Jos edes kokeilisit koodiasi, huomaisit, että se ei toimi, ja jos tajuaisit kunnolla edes ensimmäisen olio-ohjelmointiesimerkin Antin PHP-oppaasta, näkisit tuosta myös sen rakenteellisen virheen luokkien käytössä. Ja kuten sanoin, et ole korjannut niitä lukuisia muita asioita, jotka luettelin jo ensimmäisessä viestissäni: sisennystä, tekstivakioita, useaa toimintoa samassa funktiossa jne.
Ok, miten ehdottaisit sitten vaikka buildLink funktion korjausta, siinä nimittäin on aika monta muuttujaa. Ja on aika hankala jakaa toimintoja omiin funktioihinsa tuossa.
Aihe on jo aika vanha, joten et voi enää vastata siihen.