[alg] Slechtste programmeervoorbeelden deel 4

[Victor]

Webgnome schreef op zondag 18 januari 2009 @ 11:09:
mmm, misschien lees ik er overheen maar er wordt toch alleen maar een entity verwijderd waarvan de groupfolder null is en het ID klopt met datgene wat er is opgegeven. Dat de parent wordt verwijdert is de echte fout (maar dat kan ik niet bepalen aan de hand van de opgegeven code ,ken de context niet ).
De eerste declaratie van $q is toch overschreven door de tweede?

ik ga er even van uit dat voor het aanroepen van deze methode al is gecontrolleerd of de ID geldig is etc..

Waar zie je dat de parent wordt verwijderd? Ik zie alleen een ancestor method die wordt aangeroepen om de inheritance chain intact te houden

[rogierslag]

een SQL select in MySQL om het volgende op te halen

1. Huidige weeknummer (ervan uitgaande dat de week begint op zondag
2. Begin van die week
3. Eind van die week
4. Data van die week in YYYY-MM-DD formaat
5. Jaar

En dat met de volgende query

code:

SELECT WEEK(NOW(),6),
DATE_FORMAT(DATE_SUB(NOW(),INTERVAL (WEEKDAY(NOW())+1) DAY),"%d-%m"),
DATE_FORMAT(DATE_ADD(NOW(),INTERVAL (5-WEEKDAY(NOW())) DAY),"%d-%m"),
DATE_FORMAT(DATE_SUB(NOW(),INTERVAL (WEEKDAY(NOW())+1) DAY),"%Y-%m-%d"),
DATE_FORMAT(DATE_SUB(NOW(),INTERVAL (WEEKDAY(NOW())) DAY),"%Y-%m-%d"),
DATE_FORMAT(DATE_ADD(NOW(),INTERVAL (1-WEEKDAY(NOW())) DAY),"%Y-%m-%d"),
DATE_FORMAT(DATE_ADD(NOW(),INTERVAL (2-WEEKDAY(NOW())) DAY),"%Y-%m-%d"),
DATE_FORMAT(DATE_ADD(NOW(),INTERVAL (3-WEEKDAY(NOW())) DAY),"%Y-%m-%d"),
DATE_FORMAT(DATE_ADD(NOW(),INTERVAL (4-WEEKDAY(NOW())) DAY),"%Y-%m-%d"),
DATE_FORMAT(DATE_ADD(NOW(),INTERVAL (5-WEEKDAY(NOW())) DAY),"%Y-%m-%d"),
DATE_FORMAT(DATE_SUB(NOW(),INTERVAL (WEEKDAY(NOW())+1) DAY),"%Y")

En nou week ik niet eens hoe ik dit sneller en vooral duidelijker kan uitvoeren, want MySQL doet er minder dan 0.001s over. Gelukkig stond er commentaar bij.... Enige optimalisatie is het correct maken van date_add en date_sub mbt de negatieve getallen, maar qua snelheid maakt dat niks uit aangezien mysql intern diezelfde conversie maakt.

En het jaar selecteren wordt slechts tweemaal daadwerkelijk gebruikt, en beide keren is het een unset functie

[Config]

Voor de duidelijkheid: Dit is productie code. Het is SugarCRM (open source product).

Ik voer deze diffs handmatig uit met een eigen svn/trac server, Sugar geeft nl. alleen de installeerbare bestanden uit. Ik kom dit soort dingen wel eens tegen . Dit was 5.1.0 patch B naar patch C, en hier kun je zien dat deze meer dan 5 weken de te kiezen release was.

Nu vind ik dit onderdeel (inkomende email) zelf niet interessant in Sugar, maar ik ben even op zoek gegaan naar de (vermoedelijke) bijbehorende bug: http://www.sugarcrm.com/c...ff-7bd7-a298-48ebb8b89d67.

[HawVer]

Config schreef op zondag 18 januari 2009 @ 12:58:
Voor de duidelijkheid: Dit is productie code. Het is SugarCRM (open source product).

Ik voer deze diffs handmatig uit met een eigen svn/trac server, Sugar geeft nl. alleen de installeerbare bestanden uit. Ik kom dit soort dingen wel eens tegen . Dit was 5.1.0 patch B naar patch C, en hier kun je zien dat deze meer dan 5 weken de te kiezen release was.

Nu vind ik dit onderdeel (inkomende email) zelf niet interessant in Sugar, maar ik ben even op zoek gegaan naar de (vermoedelijke) bijbehorende bug: http://www.sugarcrm.com/c...ff-7bd7-a298-48ebb8b89d67.

Woei, leuke code, dat is waarom ik altijd even mijn Sql bekijk buiten de source om. Gewoon voor de zekerheid. Het kan zomaar zijn dat een where mist bij het samenvoegen van twee strings. Gelukkig heeft Sql Server een Sql Profiler zodat ik meteen zie wat er daadwerkelijk uitgevoerd wordt.

[HawVer]

Bij deze nog een slecht voorbeeld: (onderdeel van de msdn documentatie (IEnumerator))

C#:

  public object Current
    {
        get
        {
            try
            {
                return _people[position];
            }
            catch (IndexOutOfRangeException)
            {
                throw new InvalidOperationException();
            }
        }
    }

[Michali]

Is het een slecht voorbeeld of een voorbeeld van slechte code?

In het tweede geval, waarom is dat slechte code? Er wordt een bepaalde exception opgevangen, en een nieuwe exception van een hoger abstractieniveau opgegooid. Je zou kunnen beargumenteren dat je beter kunt checken of de index wel bestaat, maar hier valt ook veel tegenin te brengen. Dit lijkt me toch duidelijk een exceptioneel geval. Het enige wat ik kan bedenken is dat de bestaande exception niet meegegeven wordt aan de nieuwe.

[Sebazzz]

Ik vermoed dat het hem in de Exceptie zit. Sommige mensen houden niet van exceptions in getters.

[Haan]

Sebazzz schreef op zondag 18 januari 2009 @ 13:56:
Ik vermoed dat het hem in de Exceptie zit. Sommige mensen houden niet van exceptions in getters.

Dan moet je geen .NET gebruiken, de standaard .NET klassen staan er bol van

Ik neem aan dat de lege exception in het codevoorbeeld slechts ter illustratie is en dat er in de productiecode wel wat meer info wordt meegegeven.

[HawVer]

Michali schreef op zondag 18 januari 2009 @ 13:54:
Is het een slecht voorbeeld of een voorbeeld van slechte code?

Het is voorbeeldcode uit msdn docu, dus ja een slecht voorbeeld.

In het tweede geval, waarom is dat slechte code? Er wordt een bepaalde exception opgevangen, en een nieuwe exception van een hoger abstractieniveau opgegooid. Je zou kunnen beargumenteren dat je beter kunt checken of de index wel bestaat, maar hier valt ook veel tegenin te brengen.

Vertel

Dit lijkt me toch duidelijk een exceptioneel geval. Het enige wat ik kan bedenken is dat de bestaande exception niet meegegeven wordt aan de nieuwe.

Het zou daadwerkelijk een exceptioneel geval zijn op het moment dat position (private) nooit hoger of gelijk is aan het aantal elementen in de array. In hetzelfde voorbeeld staat:

C#:

  public bool MoveNext()
    {
        position++;
        return (position < _people.Length);
    }

[Michali]

Je hebt 2 manieren om hier tegenaan te kijken (minstens, vast wel meer ).

Je kunt je class helemaal vol met checks zetten, om te zorgen dat het niet mis gaat als een client een method of property gebruikt op moment dat de staat van het object daarvoor invalide is, of niet toestaat dat de staat invalide wordt.

Je kunt ook er vanuit gaan dat de client weet welke eisen een method of property stelt, en dat je bijvoorbeeld Current niet mag aanroepen als de staat van de enumator daarvoor niet meer geldig is. (Dit noemen ze ook wel Design by Contract. (Spec# bijvoorbeeld, voegt dergelijke zaken toe aan C#)) Mocht zo'n method dan toch aangeroepen worden, dan is het beter om een exception te gooien, aangezien de regels dan duidelijk gebroken worden.


Wat betreft die exception zelf. Een andere optie is om null oid. als return waarde te gebruiken. Echter moet de client dan gaan checken of de return waarde null is. En daar wordt de client-code allesbehalve duidelijk door. Vaak zie je ook dat er ontzettend veel van dat soort controles zijn. Beter is om daar gewoon te coden alsof alles goed gaat, en dan eventuele exceptions op te vangen. Dat is beter omdat de logica en essentie van je code dan veel duidelijker naar voren komt.

[Zoijar]

HawVer schreef op zondag 18 januari 2009 @ 14:05:
Het zou daadwerkelijk een exceptioneel geval zijn op het moment dat position (private) nooit hoger of gelijk is aan het aantal elementen in de array. In hetzelfde voorbeeld staat:

C#:

  public bool MoveNext()
    {
        position++;
        return (position < _people.Length);
    }

Als je de class goed gebruikt is het een exceptioneel geval. Het idee is natuurlijk om dit te doen:

while (obj.MoveNext()) {
// doe iets met obj.Current
}

[MBV]

Config schreef op zondag 18 januari 2009 @ 03:16:
*auw auw auw*

PHP:

Index: 5.1/SugarCE-Full-5.1.0/modules/InboundEmail/InboundEmail.php
===================================================================
--- a/5.1/SugarCE-Full-5.1.0/modules/InboundEmail/InboundEmail.php
+++ b/5.1/SugarCE-Full-5.1.0/modules/InboundEmail/InboundEmail.php
@@ -232,5 +232,5 @@
         function mark_deleted($id) {
                 parent::mark_deleted($id);
-                $q = "update inbound_email set groupfolder_id = null";
+                $q = "update inbound_email set groupfolder_id = null WHERE id = $id";
                 $r = $this->db->query($q);
                 $this->deleteCache();

Ha, we hebben een where-clause engineer! Zo noemden we bij mijn vorige baan de persoon die dezelfde fout had gemaakt

[Config]

Nog eentje..

PHP:

        if(!empty($vardef['type'])) {
            $type = $vardef['type'];
            switch($vardef['type']) {
               case 'float':
               case 'int':
                    $type = 'int';
                    break;
               case 'date':
                    $type = 'datetime';
                    break;
               case 'url':
                    $type = 'link';
                    break;   
            }

Ik ben SugarCRM aan het documenteren de laatste dagen, en dit soort dingen houdt het luchtig

[Victor]

Config schreef op zondag 18 januari 2009 @ 20:30:
Nog eentje..

PHP:

        if(!empty($vardef['type'])) {
            $type = $vardef['type'];
            switch($vardef['type']) {
               case 'float':
               case 'int':
                    $type = 'int';
                    break;
               case 'date':
                    $type = 'datetime';
                    break;
               case 'url':
                    $type = 'link';
                    break;   
            }

Ik ben SugarCRM aan het documenteren de laatste dagen, en dit soort dingen houdt het luchtig

Want een float is een integer? Zelfs PHP maakt daar onderscheid tussen

[Config]

Het gaat in dit geval om het feit dat een float en int door dezelfe Smarty widget worden getoond, dus functioneel is het correct. Echter, er zit een andere stommigheid in..

Net als in deze:

PHP:

        if (!empty($fieldDefs)) {
            foreach ($fieldDefs as $key=>$value_array) {
                if ( (isset($value_array['importable']) && $value_array['importable'] == 'false')
                    || (isset($value_array['type']) && $value_array['type'] == 'link')
                    || (isset($value_array['auto_increment'])
                        && ($value_array['type'] == true || $value_array['type'] == 'true')) ) {
                    // do not allow import.
                }
                else {
                    $importableFields[$key]=$value_array;
                }
            }
        }

[Verwijderd]

Ik zou niet eens weten waar ik moet beginnen met die code, echt

[MBV]

Config schreef op zondag 18 januari 2009 @ 22:36:
Het gaat in dit geval om het feit dat een float en int door dezelfe Smarty widget worden getoond, dus functioneel is het correct. Echter, er zit een andere stommigheid in..

Net als in deze:

PHP:

        if (!empty($fieldDefs)) {
            foreach ($fieldDefs as $key=>$value_array) {
                if ( (isset($value_array['importable']) && $value_array['importable'] == 'false')
                    || (isset($value_array['type']) && $value_array['type'] == 'link')
                    || (isset($value_array['auto_increment'])
                        && ($value_array['type'] == true || $value_array['type'] == 'true')) ) {
                    // do not allow import.
                }
                else {
                    $importableFields[$key]=$value_array;
                }
            }
        }

Daar hebben ze dus de syntax @$value_array['importable'] voor uitgevonden. Als het toch niet boeit wanneer hij er niet staat, heeft dat toch hetzelfde effect (wanneer je de conditie goedom zet)? Sowieso heb ik een hekel aan die ongedefinieerde 'structs' in PHP, om hele rijen gegevens door te geven. Er komt altijd een typfout in, en daar kom je bijna nooit achter.

[Michali]

Die @ operator zo gebruiken, dat is pas een vieze hack. Je weet dat je iets fout doet, maar je negeert het en onderdrukt de foutmelding. Bovendien is het erg slecht voor de performance.

[Config]

Ik zou de @ ook niet hiervoor gebruiken.

Waar ik het over had:
- Een IF aanmaken voor iets waarop je niks doet, en bij de else clause daadwerkelijk iets uitvoeren.. Doe dan een if (!(...)) doe();

- Wat ik persoonlijk niet echt goed trek is dat je

PHP:

$value_array['importable'] == 'false'

gebruikt, booleans en strings door elkaar haalt. Plus, als je FALSE (boolean) gebruikt, ziet het script het alsnog als TRUE.

Edit: ik heb persoonlijk geen problemen met het feit dat er zoveel condities staan. Ze kunnen wellicht iets netter worden opgeschreven, maar echt veel leesbaarder worden ze daar ook niet van hoor.

[ Voor 18% gewijzigd door Config op 18-01-2009 23:58 ]

[Michali]

Wat ik wel een goed idee vind, is om de condities buiten de if te halen. Je vangt dat het resultaat op in een variabel die beschrijft wat je nu eigenlijk wil controleren (of je maakt er zelfs een functie van waarvan de naam dat beschrijft, echter is een variabel meestal wel afdoende). Op deze manier wordt het al iets beter:

PHP:

<?php
if (!empty($fieldDefs)) {
    foreach ($fieldDefs as $key=>$value_array) {
        $isImportAllowed =  !(
            (
                isset($value_array['importable'])
                && $value_array['importable'] == 'false'
            )
            || (
                isset($value_array['type'])
                && $value_array['type'] == 'link'
            )
            || (
                isset($value_array['auto_increment'])
                && ($value_array['type'] == true || $value_array['type'] == 'true')
            )
        );
        if ($isImportAllowed) {
            $importableFields[$key]=$value_array;
        }
    }
}
?>

Nu heb ik alleen dat verbetert, er zitten nog wel meer eigenaardigheden in dus.

[bat266]

en dan de conditiecheck in een aparte functie gieten. Wordt het een stuk leesbaarder van.

[Janoz]

Nee, gewoon opdonderen met die array meuk. Ik ben het compleet eens met MBV

Sowieso heb ik een hekel aan die ongedefinieerde 'structs' in PHP

waarom niet gewoon een simpele class als struct gebruiken? Of zelfs een echte class met hierin de controle die je nodig hebt:

PHP:

class fieldDef {
  var $importable = true;
  var $auto_increment = false;
  var $type

/* ... */

  function isImportalble() {
    return $importable == true && $auto_increment == false && $type != 'link';
  }
}

Geen gezeik meer met tikfouten, undefined array indexen enz enz...

[MBV]

Michali schreef op zondag 18 januari 2009 @ 23:19:
Die @ operator zo gebruiken, dat is pas een vieze hack. Je weet dat je iets fout doet, maar je negeert het en onderdrukt de foutmelding. Bovendien is het erg slecht voor de performance.

Uhm, nee? Ik doe niks fout, ik weet alleen niet zeker of het er is. Voorbeeld:

PHP:

if (isset($_GET['Id']) && $_GET['id'] == 42))
  echo "blaat";

PHP:

if (@$_GET['id'] == 42))
  echo "blaat";

Het boeit me totaal niet of die parameter is meegegeven, maar als hij is meegegeven wil ik er een check op doen. Daar hebben ze '@' voor uitgevonden, in dit voorbeeld is dat compleet equivalent aan 'isset($_GET['id']) &&'. Met als voordeel dat je dus niet zoals hierboven een hoofdletter i gebruikt bij de check of hij er wel is...

En zoals Janoz zegt: arrays zijn om als array te gebruiken, of als map, maar niet om een lijst gegevens door te geven door je hele programma heen (Joomla? ). Voor $_GET en $_POST zie ik niet veel alternatieven, maar je gebruikt toch ook in C++ of Java geen hashmap om persoonsgegevens door je programma heen te slingeren?

[ Voor 5% gewijzigd door MBV op 19-01-2009 10:31 ]

[Michali]

De @ operator is in het leven geroepen om errors te ondrukken, als je hier geen andere mogelijkheid toe hebt. Bijvoorbeeld bij het aanroepen van een standaard functie (zoals mail of een db connect), als error reporting op een dergelijk niveau ingesteld staat dat het anders in de output terecht zou komen.

De manier waarop jij het gebruikt is echt smerig. Je probeert de variabel te gebruiken, en als dit niet lukt (dus je probeert een variabel te gebruiken die niet bestaat, dat is gewoon fout wmb.), dan onderdruk je de notice die anders opgegooid zou worden.

Ik heb het ook even getest met een simpel for loopje en dan beide constructies, en het verschil gaat richting het 10 voudige qua snelheid.

[MBV]

In het voordeel van isset? Verbaasd me. Misschien moet ik toch het volgende dat ik schrijf maar in .NET ofzo gaan doen, de wazigheden van PHP komen me mijn neus uit

[Luqq]

Haha dankje MBV, die kende ik nog niet wel paar keer tegengekomen, maar nu weet ik wat hij betekent

[BramT]

Ik zag dit topic net even voorbij komen in de tracker en ik heb er ook nog wel eentje.

Sorry als hij al voorbijgekomen is, ik heb niet de moeite genomen alle posts even na te lopen. Hij is echter zo ontzettend dat ik 'm jullie niet wil onthouden. We hebben er zelfs over gedacht om er T-shirtjes mee te bedrukken.

PHP:

extract($GLOBALS);

Volgens mij kan je dan gewoon gaan inpakken....

[Michali]

Haha, vrij nutteloos toch? $GLOBALS wordt gewoon weer in $GLOBALS gekopieerd.

(Als hij niet in een functie oid. wordt aangeroepen dus.)

[ Voor 28% gewijzigd door Michali op 19-01-2009 13:53 ]

[rednek]

isset is puur bedoelt om te kijken of een var geset is. Php is niet strikt typed, array's kunnen strings als key's bevatten. en er zijn nog veel meer van zulke dingen.

Probeer dit eens .

PHP:

$objNew = (object) array( 'foo' => 'Bar', 'foo2' => '2Bar');
var_dump( $objNew );

Resultaat is een stdClass met 2 vars.

[Voutloos]

Michali schreef op maandag 19 januari 2009 @ 13:51:
Haha, vrij nutteloos toch? $GLOBALS wordt gewoon weer in $GLOBALS gekopieerd.

(Als hij niet in een functie oid. wordt aangeroepen dus.)

Nee, extract() is in 99% van de gevallen (lage schatting) een groot security lek en de ultieme global spaghetti tool en dat is hier niet anders.

[compufreak88]

Voutloos schreef op maandag 19 januari 2009 @ 13:56:
[...]
Nee, extract() is in 99% van de gevallen (lage schatting) een groot security lek en de ultieme global spaghetti tool en dat is hier niet anders.

En in het geval dat je het in een templating system gebruikt, waar de extractfunctie wordt aangeroepen vanuit een andere functie?

[Michali]

@Voutloos: Dat is zeker zo, maar je importeert de variabelen toch in de huidige scope. Dus als je op global niveau dit aanroept, dan gebeurt er in feite toch niets?

[ Voor 3% gewijzigd door Michali op 19-01-2009 14:04 ]

[Woy]

Michali schreef op maandag 19 januari 2009 @ 14:04:
@Voutloos: Dat is zeker zo, maar je importeert de variabelen toch in de huidige scope. Dus als je op global niveau dit aanroept, dan gebeurt er in feite toch niets?

http://nl2.php.net/extract

Hij creert variabelen, dus als jij verderop ergens controleert of een var gezet is of niet kan die code incorrecte dingen doen aangezien iemand dus variabelen kan injecten in je code ( Disclaimer: dat is wat ik opmaak uit de documentatie )

[MBV]

rednek schreef op maandag 19 januari 2009 @ 13:51:
isset is puur bedoelt om te kijken of een var geset is. Php is niet strikt typed, array's kunnen strings als key's bevatten. en er zijn nog veel meer van zulke dingen.

Probeer dit eens .

PHP:

$objNew = (object) array( 'foo' => 'Bar', 'foo2' => '2Bar');
var_dump( $objNew );

Resultaat is een stdClass met 2 vars.

Oh ja, wat doet deze code:

PHP:

class test {
  var $test;
  
  function test($param) { $this->Test = $param;}
  function echo() {echo $this->test;}
}
$test = new test('blaat');
$test->echo();

_{Disclaimer: het gaat me niet om de syntax van classes etc, al een half jaar niks mee gedaan}

In elk geval geen syntax error geven op wat voor manier dan ook Enige voordeel van classes definiëren is dat je in een fatsoenlijke IDE de 'officiële' mogelijkheden ziet met code-completion.

[Michali]

rwb schreef op maandag 19 januari 2009 @ 14:18:
[...]

http://nl2.php.net/extract

Hij creert variabelen, dus als jij verderop ergens controleert of een var gezet is of niet kan die code incorrecte dingen doen aangezien iemand dus variabelen kan injecten in je code ( Disclaimer: dat is wat ik opmaak uit de documentatie )

Ja, dat weet ik. Maar $GLOBALS bevat alles in de global space, je kunt alle globale variabelen via die array bereiken, en vice versa. Dus als je op global niveau dus $GLOBALS extract, dan eindig je (voor zover ik me kan bedenken), met exact dezelfde staat als daarvoor. Alles wat geextract wordt bestond namelijk al. Binnen een functie is het een ander verhaal, aangezien je daar een andere symbol table hebt, je hebt daar niet zomaar direct toegang tot globale variabelen en als je daar een extract doet, dan komen de variabelen niet in de global space terecht.

Misschien begrijp ik het verkeerd hoor.

[BramT]

Met register_globals=on wel ja. Maar dat staat dus ondertussen gelukkig uit.

Met extract($GLOBALS) (wat verder prima syntax e.d. is, nix depricated aan) implementeer je je eigen register_globals=on.

Het mooie is dat register_globals ondertussen echt niet meer kan en dat $GLOBALS en extract() beide slechts bij hoge uitzondering nodig/nuttig/beter zijn. Het is gewoon 3 enorme fouten maken met 2 woorden. Best een prestatie op zich

[Janoz]

@BramT: Ik vermoed dat je $GLOBALS verwart met $_REQUEST.

[BramT]

Nee, hoewel $_REQUEST wat "realistischer" zou zijn.

extract($GLOBALS) is nog een standje erger, vandaar.

Laten we nou ff niet in discussie gaan hoe slecht slecht nou is he

/edit:

Ok, wel in de war

Zo dan:

PHP:

foreach($GLOBALS as $ikWilRegisterGlobalsTerug) extract($ikWilRegisterGlobalsTerug);

Jeumig, issie zo ranzig genoeg? Het ging om het idee

[ Voor 37% gewijzigd door BramT op 19-01-2009 14:56 ]

[rednek]

MBV schreef op maandag 19 januari 2009 @ 14:27:
[...]

Oh ja, wat doet deze code:

PHP:

class test {
  var $test;
  
  function test($param) { $this->Test = $param;}
  function echo() {echo $this->test;}
}
$test = new test('blaat');
$test->echo();

_{Disclaimer: het gaat me niet om de syntax van classes etc, al een half jaar niks mee gedaan}

In elk geval geen syntax error geven op wat voor manier dan ook Enige voordeel van classes definiëren is dat je in een fatsoenlijke IDE de 'officiële' mogelijkheden ziet met code-completion.

He???

Dat slaat toch helemaal nergens op iedereen snapt gelijk toch wel dat het dan zo moet.

PHP:

<?php
class test {
  var $test;
  
  function __construct($param) { $this->test = $param; }
  function __toString(){ return $this->test;}
}
$test = new test('blaat');
echo $test;
?>

[MBV]

Heb je de disclaimer gelezen? Het ging mij om de typfout in de constructor met $test->Test, en daar ben ik al 100x naar op zoek geweest
Sowieso, wie heeft verzonnen dat __construct een leuke naam is? Elke andere taal gebruikt de klassenaam voor de constructor (behalve perl geloof ik), dus waarom?

[Michali]

Python en Ruby hebben hiervoor ook speciale methods voor constructie. Het is even wennen, maar ik kan me er niet echt aan storen hoor.

[farlane]

MBV schreef op maandag 19 januari 2009 @ 15:27:
Sowieso, wie heeft verzonnen dat __construct een leuke naam is? Elke andere taal gebruikt de klassenaam voor de constructor (behalve perl geloof ik), dus waarom?

Had Python ook niet iets dergelijks? Is ook wel wat voor te zeggen, het scheelt typen als je je klassenaam verandert.

[.oisyn]

Als een klassenaam verandert is het aanpassen van de ctor wel het minst van je problemen vermoed ik zo

Maar ik vraag me eerlijk gezegd ook af waarom ze van de klassenaam-als-ctor zijn afgestapt. Het werkt overigens nog wel gewoon, waarschijnlijk om compatibel te blijven met PHP4. Wellicht vonden ze een magic method voor de ctor wat meer in lijn liggen met alle andere magic methods die je tot je beschikking hebt. Desalniettemin, ik vind de __ wel erg lelijk.

[ Voor 63% gewijzigd door .oisyn op 19-01-2009 15:55 ]

[mees]

Ik denk dat dit inderdaad puur is geweest om aan te geven dat het een magic method is.

Wat ook een reden zou kunnen zijn, is voor de gevallen van extending classes. Ik weet niet of het zo is, maar misschien creëerde het wel overhead op het moment dat class A met constructor A() ge-extend werd door B met constructor B().

Die __ zijn absoluut lelijk, maar in feite zul je nooit $oClass->__xxx() gebruiken. Het staat dus puur lelijk in de definitie, maar het onderscheid het wel van de rest van de methods in je class.

Daarbij is het ook handig als er een file-overview in je scriptproggie zit, die zet door de __ de magic methods bovenaan

[prototype]

Michali schreef op maandag 19 januari 2009 @ 15:43:
Python en Ruby hebben hiervoor ook speciale methods voor constructie. Het is even wennen, maar ik kan me er niet echt aan storen hoor.

In Ruby is het niet echt een "speciale constructie", maar eerder een conventie. Je doelt zeker op:

Ruby:

class EenKlass
# Wat veelal beschouwd wordt als zijnde de constructor
    def initialize
    end
end

In realiteit is dit gewoon een methode, niks speciaals aan. Heck, zelfs het instantieren van een object gebeurd niet eens via een speciale keywords maar gewoon via een factory method. In werkelijkheid doet deze methode niks anders dan allocate aan te roepen om geheugen voor het object te alloceren en de initialize methode aan te roepen. Kort door de bocht:

Ruby:

class EenKlass
    def initialize
    end

    def self.new
        result = EenKlass.allocate
        result.initialize
        return result
    end
end

En aangezien Ruby open classes ondersteund kan je ook dood leuk dus self.new voorzien van een andere implementatie. Case in point:

Ruby:

class EenKlass
  def initialize
    puts "Foo"
  end
  
    def constructor
        puts "dit is mijn constructor ipv initialize"
    end
    
    def self.new
        result = EenKlass.allocate
        result.constructor
        return result
    end
end

a = EenKlass.new
puts a.is_a? EenKlass

Je kan dit ook toepassen op Object class zelf en dan b.v. zeggen dat hij __constructor moet gebruiken ipv initialize om meer het PHP gevoel te krijgen

[ Voor 6% gewijzigd door prototype op 19-01-2009 16:22 ]

[.oisyn]

"Klass"

[ Voor 13% gewijzigd door .oisyn op 19-01-2009 16:45 ]

[litemotiv]

Uit de code van een grote, niet onbekende Nederlandse website, waar ik op dit moment dingen aan moet verbouwen:

code:

<table class="spacer5px" width="100%" cellspacing="0" cellpadding="0" border="0">
<tbody>
<tr>
<td><!-- leeg --></td>
</tr>
</tbody>
</table>

En ja inderdaad, het beoogde doel is om 5 pixels marge te creëren tussen 2 nieuwsberichten, en komt alleen al op de frontpage zo'n 30 keer voor.

[Brian]

.oisyn schreef op maandag 19 januari 2009 @ 16:45:
"Klass"

offtopic:
Dinglish

[TeeDee]

litemotiv schreef op maandag 19 januari 2009 @ 16:48:

code:

<table class="spacer5px" width="100%" cellspacing="0" cellpadding="0" border="0">
<tbody>
<tr>
<td><!-- leeg --></td>
</tr>
</tbody>
</table>

Och, vroegah was dat redelijk gebruikelijk, m.a.w.: legacy-code. Neemt natuurlijk niet weg dat het in deze tijd niet echt meer kan.

[ Voor 24% gewijzigd door TeeDee op 19-01-2009 16:53 ]

[rednek]

litemotiv schreef op maandag 19 januari 2009 @ 16:48:
Uit de code van een grote, niet onbekende Nederlandse website, waar ik op dit moment dingen aan moet verbouwen:

code:

<table class="spacer5px" width="100%" cellspacing="0" cellpadding="0" border="0">
<tbody>
<tr>
<td><!-- leeg --></td>
</tr>
</tbody>
</table>

En ja inderdaad, het beoogde doel is om 5 pixels marge te creëren tussen 2 nieuwsberichten, en komt alleen al op de frontpage zo'n 30 keer voor.

NU.NL

[litemotiv]

TeeDee schreef op maandag 19 januari 2009 @ 16:52:
[...]

Och, vroegah was dat redelijk gebruikelijk, m.a.w.: legacy-code. Neemt natuurlijk niet weg dat het in deze tijd niet echt meer kan.

dan moet je even wat beter kijken denk ik

[rednek]

Hier code waar mensen bij mijn vorige werk voor ontslagen zijn

PHP:

move_uploaded_file( $_FILES['name']['tmp_name'], PATH_NEWLOCATION . $name );

// comprimeer foto
$strFile = file_get_contents( PATH_NEWLOCATION . $name );
file_put_contents( PATH_NEWLOCATION . $name, md5( $strFile ) );

[.oisyn]

Ik mis nog een comment

PHP:

move_uploaded_file( $_FILES['name']['tmp_name'], PATH_NEWLOCATION . $name ); 

// comprimeer foto 
$strFile = file_get_contents( PATH_NEWLOCATION . $name ); 
file_put_contents( PATH_NEWLOCATION . $name, md5( $strFile ) ); // Jan Sloot was here

[kenneth]

[prototype]

Roflmao

[Dima_2005]

Ik lach me tot in tranen!
Echt wel het toppunt van PHP-code!

[XWB]

.oisyn schreef op maandag 19 januari 2009 @ 17:14:
Ik mis nog een comment

PHP:

move_uploaded_file( $_FILES['name']['tmp_name'], PATH_NEWLOCATION . $name ); 

// comprimeer foto 
$strFile = file_get_contents( PATH_NEWLOCATION . $name ); 
file_put_contents( PATH_NEWLOCATION . $name, md5( $strFile ) ); // Jan Sloot was here

Geweldig.

[netvor]

Misschien dat ze het nieuws gevolgd hebben de laatste paar jaren en dat ze geleerd hebben dat het mogelijk is MD5 collisions te vinden?

[Victor]

netvor schreef op maandag 19 januari 2009 @ 18:35:
Misschien dat ze het nieuws gevolgd hebben de laatste paar jaren en dat ze geleerd hebben dat het mogelijk is MD5 collisions te vinden?

Dat kan in ieder algoritme, alleen weten we van sommigen nog niet hoe

Maar inderdaad, met het gemak waarmee je een veiliger algoritme gebruikt (SHA 1 bijv.), zou het toch een kleine moeite moeten zijn om de switch te maken.

[Snake]

netvor schreef op maandag 19 januari 2009 @ 18:35:
Misschien dat ze het nieuws gevolgd hebben de laatste paar jaren en dat ze geleerd hebben dat het mogelijk is MD5 collisions te vinden?

Als je dat weet weet je ook dat er oneindig aantal combinaties zijn van bitvolgordes die dezelfde hash opleveren, dus no way dat je van die md5 een volledige foto kunt terugkrijgen...

[.oisyn]

Ach, je kunt iig nog goatse's terugkrijgen door die te padden met pixels totdat je een collision hebt

[Jaap-Jan]

Victor schreef op maandag 19 januari 2009 @ 19:05:
[...]

Dat kan in ieder algoritme, alleen weten we van sommigen nog niet hoe

Maar inderdaad, met het gemak waarmee je een veiliger algoritme gebruikt (SHA 1 bijv.), zou het toch een kleine moeite moeten zijn om de switch te maken.

Hee, je moet nog wel de mogelijkheid om de foto op te kunnen vissen, dus hoe onveiliger het algoritme, hoe beter.

[netvor]

Dat is precies wat ik bedoelde. Hoelang duurt het tegenwoordig om een MD5 collision te vinden? Uurtje op een consumentenprocessor? Als de klant dan zijn foto wilt ophalen zeg je gewoon, "kom maar over een uurtje terug" -- net als vroegah toen we nog fotorolletjes gebruikten.

[Snake]

http://md5.rednoize.com/

Test

[dev10]

Snake schreef op maandag 19 januari 2009 @ 19:48:
http://md5.rednoize.com/

Test

Dat is geen reverse-engineren, maar gewoon een groot databeest met hashes en strings aan elkaar gekoppeld.

[Victor]

Jaap-Jan schreef op maandag 19 januari 2009 @ 19:19:
[...]
Hee, je moet nog wel de mogelijkheid om de foto op te kunnen vissen, dus hoe onveiliger het algoritme, hoe beter.

Ah, ik begreep zijn opmerking verkeerd. Ik had wel gezien dat ze alleen de checksum van de file opslaan, maar ik had niet door dat netvor z'n opmerking bedoeld was als oplossing voor die stunt, maar eerder om even md5 te kunnen bashen

[netvor]

Bashen? Integendeel, ik gaf een voorbeeld van hoe MD5, dankzij haar zwakte, als een efficiënt (lossy) compressie-algoritme kan worden gebruikt. In de onsterfelijke woorden van nummer 14, "elk nadeel hep z'n voordeel."

[Woy]

netvor schreef op dinsdag 20 januari 2009 @ 10:25:
Bashen? Integendeel, ik gaf een voorbeeld van hoe MD5, dankzij haar zwakte, als een efficiënt (lossy) compressie-algoritme kan worden gebruikt. In de onsterfelijke woorden van nummer 14, "elk nadeel hep z'n voordeel."

Helaas dat je nogal een grote kans heb dat je een compleet andere afbeelding terug krijgt

[YopY]

rwb schreef op dinsdag 20 januari 2009 @ 10:45:
[...]

Helaas dat je nogal een grote kans heb dat je een compleet andere afbeelding terug krijgt

Maakt niet uit, een compressie van 99.9% haal je met geeneen ander algoritme, .

[Voutloos]

Nee, voor een dusdanig lossy algoritme doe je het nog slecht.

[Jaap-Jan]

Voutloos schreef op dinsdag 20 januari 2009 @ 11:00:
Nee, voor een dusdanig lossy algoritme doe je het nog slecht.

Dit lijkt me dan het efficiëntste lossy algoritme:

PHP:

unlink(PATH_NEWLOCATION . $name);

100% compressie.

[EdwinG]

Jaap-Jan schreef op dinsdag 20 januari 2009 @ 17:13:
[...]


Dit lijkt me dan het efficiëntste lossy algoritme:

PHP:

unlink(PATH_NEWLOCATION . $name);

100% compressie.

Waarom zelf doen wat het besturingsysteem ook voor je kan regelen? Sla het bestand gewoon op in /dev/null

Voor een heel ander verhaal, kan iemand mij het nut uitleggen van de volgende reguliere expressie?

code:

/[0-9,.]+$/

[--MeAngry--]

EdwinG schreef op dinsdag 20 januari 2009 @ 21:35:
[...]

Voor een heel ander verhaal, kan iemand mij het nut uitleggen van de volgende reguliere expressie?

code:

/[0-9,.]+$/

Ik heb zo een vaag vermoeden dat het van iemand is die niet weet waar de punt voor dient in een Regex

_{Maar het zou me niet verbazen als ik die fout vroegah ook nog wel eens gemaakt zou hebben...}

[ Voor 10% gewijzigd door --MeAngry-- op 20-01-2009 21:42 ]

[Sebazzz]

EdwinG schreef op dinsdag 20 januari 2009 @ 21:35:
[...]


Waarom zelf doen wat het besturingsysteem ook voor je kan regelen? Sla het bestand gewoon op in /dev/null

Voor een heel ander verhaal, kan iemand mij het nut uitleggen van de volgende reguliere expressie?

code:

/[0-9,.]+$/

Dat werkt natuurlijk niet op een niet-Unix systeem. Als je Windows draait voor een server verdien je toch niet beter

[Jaap-Jan]

--MeAngry-- schreef op dinsdag 20 januari 2009 @ 21:41:
[...]

Ik heb zo een vaag vermoeden dat het van iemand is die niet weet waar de punt voor dient in een Regex

_{Maar het zou me niet verbazen als ik die fout vroegah ook nog wel eens gemaakt zou hebben...}

Die punt maakt niet uit, elk teken verliest zijn speciale betekenis binnen een character class, dus die punt is gewoon een letterlijke punt.

Wat is de fout eigenlijk?

De regex is gewoon [0-9,.]+$, wat niets meer betekent dan: één of meer tekens (0-9, een ',' of een '.') aan het eind van een zin.

[ Voor 14% gewijzigd door Jaap-Jan op 20-01-2009 22:14 ]

[Michali]

Jaap-Jan schreef op dinsdag 20 januari 2009 @ 22:02:
elk teken verliest zijn speciale betekenis binnen een character class

Muggenziften (): Niet helemaal waar. Het verbindingsstreepje heeft een betekenis mits niet aan het begin of eind gebruikt, en een dakje aan het begin van de class maakt hem negatief oid.

edit: Hoewel een streepje buiten de class geen speciale betekenis heeft, dus die valt af, bedenk ik me net

[ Voor 16% gewijzigd door Michali op 20-01-2009 22:21 ]

[Jaap-Jan]

Michali schreef op dinsdag 20 januari 2009 @ 22:17:
[...]

Muggenziften (): Niet helemaal waar. Het verbindingsstreepje heeft een betekenis mits niet aan het begin of eind gebruikt, en een dakje aan het begin van de class maakt hem negatief oid.

Had ik expres weggelaten, want dat vond ik niet echt spannend.

^ binnen [] matcht elk teken (of groep), behalve degene die daarna genoemd worden.

Ik heb net een scanner geschreven in flex, dus mijn kennis van regexps is de afgelopen tijd een stuk verbeterd.

[Michali]

Jaap-Jan schreef op dinsdag 20 januari 2009 @ 22:40:
Had ik expres weggelaten, want dat vond ik niet echt spannend.

^ binnen [] matcht elk teken (of groep), behalve degene die daarna genoemd worden.

Wat ook niet echt bedoeld om je te verbeteren ofzo, meer een nutteloze aanvulling.

Ik heb net een scanner geschreven in flex, dus mijn kennis van regexps is de afgelopen tijd een stuk verbeterd.

Zeker leerzaam ja. Maar bij de implementatie van een scanner schieten alleen regular expression vaak al snel te kort.

[Tanuki]

Jaap-Jan schreef op dinsdag 20 januari 2009 @ 22:02:
[...]
Die punt maakt niet uit, elk teken verliest zijn speciale betekenis binnen een character class, dus die punt is gewoon een letterlijke punt.

Wat is de fout eigenlijk?

De regex is gewoon [0-9,.]+$, wat niets meer betekent dan: één of meer tekens (0-9, een ',' of een '.') aan het eind van een zin.

Waarschijnlijk de ^ vergeten aan het begin van de regexp?

Heb ik ook regelmatig gedaan bij het valideren van input binnen een PHP script:

PHP:

if (preg_match('[0-9]+', $_GET['id']))
{
    // Doe iets
}

En dan testen en het werkt. Maar zonder ^ en $ heeft dat weinig zin.

[Verwijderd]

Vrij groot financieel/ERP pakket wat floats gebruikt voor de verwerking van bedragen

[Swaptor]

Ach, het is beter dan te klein gedimensionaliseerde integers.

[Janoz]

Swaptor schreef op woensdag 21 januari 2009 @ 09:52:
Ach, het is beter dan te klein gedimensionaliseerde integers.

Oneens. Floats zijn erger. Bij te klein gedimensionaliseerde integers krijg je overflow errors. Men merkt dus net wat eerder wanneer het fout gaat.

[Verwijderd]

Swaptor schreef op woensdag 21 januari 2009 @ 09:52:
Ach, het is beter dan te klein gedimensionaliseerde integers.

Afrondingsverschillen in financiële verwerkingen zijn niet grappig. Dan kun je nog beter een exception krijgen door een slecht gedimensionaliseerde integer (of gewoon een geschikt datatype gebruiken ).

[-Paul-]

rwb schreef op dinsdag 20 januari 2009 @ 10:45:
[...]

Helaas dat je nogal een grote kans heb dat je een compleet andere afbeelding terug krijgt

Maar misschien is het wel mogelijk om alle "verkeerde" afbeeldingen af te schrijven, omdat het uiteindelijke bestand niet aan afbeeldingsspecificaties voldoet.

[Woy]

-Paul- schreef op woensdag 21 januari 2009 @ 10:14:
[...]

Maar misschien is het wel mogelijk om alle "verkeerde" afbeeldingen af te schrijven, omdat het uiteindelijke bestand niet aan afbeeldingsspecificaties voldoet.

Ik denk niet dat het mogenlijk is om alle verkeerde afbeeldingen af te schrijven! Zelfs als je de dimensies en het formaat als parameters hebt, heb je nog een enorm groote hoeveelheid mogelijkheden, waardoor je zo goed als zeker nog wel wat collisions kunt vinden.

[Swaptor]

Verwijderd schreef op woensdag 21 januari 2009 @ 10:12:
[...]

Afrondingsverschillen in financiële verwerkingen zijn niet grappig. Dan kun je nog beter een exception krijgen door een slecht gedimensionaliseerde integer (of gewoon een geschikt datatype gebruiken ).

True @ geschikt datatype.
En inderdaad, ongemerkte afrondingsverschillen zuigen in een financieel pakket behoorlijk hard.
I stand corrected.

[Voutloos]

-Paul- schreef op woensdag 21 januari 2009 @ 10:14:
[...]

Maar misschien is het wel mogelijk om alle "verkeerde" afbeeldingen af te schrijven, omdat het uiteindelijke bestand niet aan afbeeldingsspecificaties voldoet.

OK. Stel dat je weet dat de afbeelding 3000 x 2000 pixels is. Reken maar het uit hoeveel meer mogelijke afbeeldingen er zijn tov mogelijke hashes en kom tot de conclussie dat je niet langer serieus moet in gaan op de Jan Sloot grap.

[Face_-_LeSS]

Nog een leuke die ik tegen kwam:

C#:

        public void Approve()
        {
            if (this == null)
                throw new ArgumentNullException("this");

            if (!this.IsApprovableByCurrentUser())
            {
                throw new UnauthorizedAccessException();
            }
            this.SetStatusApproved();
            this.DateOfApproval = DateTime.Now;
            this.RegisterStatusMutation();
        }

Het erge is dat ik deze zelf heb gemaakt...

[Zoijar]

altijd even checken of je zelf wel bestaat

[netvor]

Sum ergo cogito.

[Face_-_LeSS]

Face_-_LeSS schreef op maandag 26 januari 2009 @ 09:59:
Nog een leuke die ik tegen kwam:

C#:

        public void Approve()
        {
            if (this == null)
                throw new ArgumentNullException("this");

            if (!this.IsApprovableByCurrentUser())
            {
                throw new UnauthorizedAccessException();
            }
            this.SetStatusApproved();
            this.DateOfApproval = DateTime.Now;
            this.RegisterStatusMutation();
        }

Het erge is dat ik deze zelf heb gemaakt...

Ah, ik weet alweer hoe de "fout" ontstaan is. De methode zat eerst in een andere klasse en kreeg toen een parameter mee. Ik heb toen de methode verplaatst en "parameternaam" veranderd in "this' d.m.v. Find & Replace en vervolgens de parameter weggehaald.

[MBV]

Zal ik mijn ex-collega's eens vertellen Ik zal daarover trouwens niet al te veel strooien met termen als 'professioneel' en 'goed' enzo, de website die afgelopen 1 april klaar zou zijn staat er nog steeds niet
"Skip the requirements, just start writing code" was de oplossing van het management. Ziehier het resultaat...

Eind van de week eens langsfietsen, even lachen

[ Voor 7% gewijzigd door MBV op 26-01-2009 15:01 ]

[Verwijderd]

Progress: wanneer je een veldlengte definieert van, bijvoorbeeld, 25, kun je gewoon vrolijk ná die 25 door blijven typen. Vervolgens zijn er ook weer programmeurs die dat niet afvangen in hun code waardoor weer een hoop interessante dingen ontstaan.

[cowgirl]

Je definieert in Progress dan ook geen veldlengte maar een default display-lengte

[Eskimootje]

Zit hier op mn werk naar een intranet pagina te kijken, bestaat uit slechts 2363 tabellen. Pagina is dus ook 3,8 MB groot.Vroeg me al af waarom internet explorer niet zo lekker meer reageerde.

[MBV]

Ik heb heel vroeger wel eens naar de voorpagina van hotmail gekeken (10 jaar geleden). Table in table in table in table, om uiteindelijk 1 kopje op de goede plek te zetten

Met 1 table per kopje, en een 1x1 doorzichtige gif, kan je toch zonder CSS al alle absolute positionering doen die je wilt, of zie ik dat te simpel

[ Voor 30% gewijzigd door MBV op 28-01-2009 12:21 ]

[Zoijar]

MBV schreef op woensdag 28 januari 2009 @ 12:20:
Ik heb heel vroeger wel eens naar de voorpagina van hotmail gekeken (10 jaar geleden). Table in table in table in table, om uiteindelijk 1 kopje op de goede plek te zetten

Met 1 table per kopje, en een 1x1 doorzichtige gif, kan je toch zonder CSS al alle absolute positionering doen die je wilt, of zie ik dat te simpel

Modulaire opzet denk ik. Een tabel voor je master layout, daarin een table voor je content, daarin een table voor je heading, en dan een table om je daadwerkelijk content. Met CSS doe je dat ook min of meer toch, div in div in div.

[Verwijderd]

cowgirl schreef op dinsdag 27 januari 2009 @ 21:49:
Je definieert in Progress dan ook geen veldlengte maar een default display-lengte

Dat maakt het nog steeds niet handiger

[Zyppora]

Ik heb in de beginjaren van mijn HTML kennis ook de <table> tags misbruikt voor positionering van content, maar dat probeerde ik dan toch een beetje 'slim' aan te pakken en zo min mogelijk tabellen te nesten. Ik was er stiekem wel trots op dat ik zelfs de meest complexe layouts toch met een minimum aan tabellen kon realiseren

[Config]

PHP:

                if($can_install){
                    // handle manifest.php
                    $target_manifest = remove_file_extension( $file ) . '-manifest.php';
                    if($type = 'langpack'){
                        $_REQUEST['manifest'] = $target_manifest;
                        $_REQUEST['zipFile'] = $file;
                        commitLanguagePack();
                        continue;
                    }
                    if($type = 'langpack'){
                        $_REQUEST['manifest'] = $target_manifest;
                        $_REQUEST['zipFile'] = $file;
                        commitLanguagePack();
                        continue;
                    }
                    include($target_manifest);
//even more code