Results 1 to 14 of 14

Thread: Wanneer MOET iets aangepast?

  1. #1
    Reader
    Join Date
    May 2002
    Location
    Holland
    Posts
    3,382

    Wanneer MOET iets aangepast?

    Wanneer je dit soort code tegenkomt in een programma, valt dat dan in de categorie dit MOET aangepast? In mijn ogen wel. Wat vinden de NLDelphiers?
    Code:
    procedure TFormRapportage.SetInsertMode();
    begin
      FormRapportage.btnXXX.Enabled := true;
      FormRapportage.btnYYY.Enabled := true;
      FormRapportage.dbeZZZ.Enabled := true;
      DisableNavigation;
    end;

  2. #2
    Senior Member Antoine's Avatar
    Join Date
    Apr 2011
    Location
    Molenwaard
    Posts
    2,399
    Tuurlijk!

    Onmiddelijk gebruik maken van TActions!!!
    Veel leesbaarder en alles centraal te regelen...

    Gr Anton
    " De waarde van het leven is niet in geld uit te drukken "

  3. #3
    Reader
    Join Date
    May 2002
    Location
    Holland
    Posts
    3,382
    ??

  4. #4
    Ik weet niet wat je bedoelt met MOET aangepast worden. Kennelijk hebben ze al een routine gemaakt die voor een bepaalde status (insertmode in je voorbeeld) de controls visible zet of juist niet. Dat is op zich prima.

    Zelf zou ik alleen de referentie FormRapportage vervangen voor self.

  5. #5
    Fornicatorus Formicidae VideoRipper's Avatar
    Join Date
    Mar 2005
    Location
    Vicus Saltus Orientalem
    Posts
    5,708
    Quote Originally Posted by EricLang View Post
    Code:
    procedure TFormRapportage.SetInsertMode();
    begin
      FormRapportage.btnXXX.Enabled := true;
      FormRapportage.btnYYY.Enabled := true;
      FormRapportage.dbeZZZ.Enabled := true;
      DisableNavigation;
    end;
    JA!

    FormRapportage is een verwijzing naar een instantie van TFormRapportage en hard verwijzen naar de
    eigen instantie (in dit geval) gaat je op een gegeven moment in de staart bijten, wanneer je op een gegeven
    moment twee (of meer) instanties van eenzelfde form (of klasse) hebt.

    Sterker nog: het is nergens voor nodig; je kunt (ik vind zelfs dat het moet) gewoon de verwijzing weglaten
    als het naar zichzelf verwijst.
    En mocht je ergens in je code moeten verwijzen naar jezelf, bijvoorbeeld in een With..Do constructie, dan
    gebruik je Self voor de eigen instantie.

    Wat dat betreft behandel ik een TForm-afgeleide net zoals alle andere klassen; ik zou niet weten waarom een
    TForm een andere behandeling verdient.

    De globale variabele FormRapportage wordt door Delphi zelf aangemaakt en gebruikt in Application.CreateForm
    (in de DPR) en eigenlijk zou je daar zelf zo min mogelijk direct naar moeten roepen (als het even kan), om
    eventuele problemen in de toekomst te voorkomen (wanneer je form bijvoorbeeld een dialoogvenster wordt in
    een latere versie oid.).

    Buiten al deze (mijn) visies: waarom zou je meer typen als het niet hoeft en ook nog eens voor verwarring kan
    gaan zorgen?

    Greetz,

    Peter.
    TMemoryLeak.Create(Nil);

  6. #6
    Reader
    Join Date
    May 2002
    Location
    Holland
    Posts
    3,382
    Blij dat je JA! zegt
    Ik denk er precies zo over.
    Het gaat om een oud programma met veel legacy, dit soort vreemdsoortige foutjes en veel gekopieerde code in -tig forms. maar hoe overtuig je niet-delphiers van de noodzaak van dit soort aanpassingen?
    ("Het werkt toch?")

  7. #7
    Silly member NGLN's Avatar
    Join Date
    Aug 2004
    Location
    Werkendam
    Posts
    5,133
    Quote Originally Posted by EricLang
    ... maar hoe overtuig je niet-delphiers van de noodzaak van dit soort aanpassingen?
    Indien normale rede niet werkt, dan bedenk of creëer je de noodzaak voor een tweede instantie van dat formulier. Maar jij als Delphispecialist zou eigenlijk niet te hoeven moeten overtuigen. Bovendien denk ik dat er dan een makkelijkere weg is.

    Je kunt hetzelfde argument namelijk ook tegen de niet-Delphiërs gebruiken door te zegen dat het op jouw manier "toch ook werkt". Als je dan vervolgens aanvoert dat jij er veel gelukkiger van wordt dan dat zij er ongelukkiger van worden, dan kan dat soort omgekeerde psychologie prima werken. Klaar. What's next?

    Overigens horen die globale formvariabelen gewoon verwijderd te worden, en voor auto-create forms verhuisd te worden naar de project file. Als het echt noops zijn, dan vertel je dat dat in minder code resulteert en het programma dus veel sneller wordt.
    (Sender as TNLDUser).Signature := 'Groeten van Albert';

  8. #8
    Jan
    Join Date
    Oct 2007
    Location
    Mijdrecht
    Posts
    906
    In ieder geval wordt de onderhoudbaarheid van de code er beter op.
    Maar hoeveel bespaar je dan in de toekomst is de vraag ?

  9. #9
    Reader
    Join Date
    May 2002
    Location
    Holland
    Posts
    3,382
    Het is natuurlijk logisch dat een bedrijf in uren en geld denkt.
    En helaas een stuk minder in "software moet foutloos, onderhoudbaar, goed leesbaar, robuust en snel zijn en er cool uitzien".
    Het hele programma bevat bovengenoemde "fout" van globale verwijzingen in klassen.
    Ook is het zo dat - met die globalen - er regelmatig nieuwe instanties (kopieren code en onwetendheid is de oorzaak) van een datamodule worden aangemaakt zonder dat de oude instanties ooit nog gebruikt worden.
    Pas als het programma sluit worden deze datamodules door het ownership van de application vrijgegeven.
    "Boeit niet echt" is de reactie meestal als ik dit soort dingen aankaart. Doorbouwen, doorbouwen

  10. #10
    Er is heel veel dat aangepast *moet* worden, maar het kan niet altijd zomaar. Soms kost het te veel tijd, en die tijd is vaak toch al kostbaar. Je kunt FormRapportage hier niet zomaar aanpassen naar Self, zonder goed te controleren of je daarmee niets beïnvloed.

    Daarbij hangt het er ook vanaf wat er met die software gaat gebeuren. Het is legacy zeg je. Is het de bedoeling dat het in een staat wordt gebracht waarin het nog jaren mee kan, of onderhoud je het terwijl er aan een vervanging gewerkt wordt? In het laatste geval is het extra zonde van de tijd om dit soort dingen aan te pakken. Het zal namelijk waarschijnlijk bij lange na niet het enige issue in deze code zijn.

    Ik heb ergens ook nog een legacy pakket waarvoor ik per uur betaald word voor aanpassingen. Over een paar jaar is dat hele pakket waarschijnlijk niet meer gebruikt, en tot die tijd wordt er af en toe nog wat aan bijgeschaafd of toegevoegd. Terwijl ik dat doe probeer ik nieuwe dingen zo netjes mogelijk te maken en geen nieuwe troep te introduceren. Als ik makkelijk kleine fixes kan meenemen, dan zal ik dat ook doen. Terwijl vertel ik wel aan mijn opdrachtgever wat de issues zijn met die code. In jouw geval zou dat dus zijn dat je a) geen tweede instantie van dat scherm kunt maken zonder bijeffecten, en b) dat het scherm (als het auto-create is) al geheugen gebruikt zonder dat het überhaupt geopend wordt. Dan heb je ze in ieder geval voorbereid. Als ze er dan achter komen dat het programma traag is of heel veel geheugen gebruikt, dan kun je zeggen dat dat daardoor komt, en voorstellen het aan te passen. Maar zolang er geen problemen zijn, is er waarschijnlijk ook geen geld. Wel om een uurtje te hotfixen, maar niet om even een paar dagen de code op te schonen.

    Het derde probleem c) is dat de structuur van het programma niet netjes is, en dat dat voor je gevoeld, en misschien zelfs voor de concrete onderhoudbaarheid van je programma, niet zo goed is. Dat is echter een lastig argument om hard te maken. Het kost tenslotte ook al veel geld om het nu glad te strijken, en het is dus maar de vraag of dat geld op de lange termijn terugverdiend wordt.

    Dus mijn antwoord is 'Nee, dit "moet" niet opgelost worden', al zal ik het wel doen als ik de kans krijg.

    En als je er zelf heel erg mee zit, dan moet je het gewoon niet zien als globale variabelen, maar als singletons.
    Last edited by GolezTrol; 14-Dec-14 at 13:55.
    1+1=b

  11. #11
    Reader
    Join Date
    May 2002
    Location
    Holland
    Posts
    3,382
    De exacte strategie weet ik niet.
    Ik weet wel dat het pakket gemoderniseerd is (overgenomen van ander bedrijf) vanuit Delphi 4 + BDE naar Delphi XE t/m XE6 + DbExpress.
    (wat mij overigens het e.e.a. geleerd heeft over DBExpress Wat een gedoe voor 1 simpele designtime query zeg: datasetprovider + query + clientdataset (en dan nog de datasource))
    Het programma wordt actief gebruikt en er zijn geen plannen om het te vervangen, vziw.
    Vandaar dat ik als programmeur / onderhouder af en toe tegen dit soort dingen aanloop: wat is echt fout en moet anders, en wat niet?

    In het geval van deze "Self" aanpassing gaat het altijd om dezelfde globale.
    Wordt er een nieuwe instantie van het form gemaakt, dan wordt *altijd* de globale var overschreven (all over the place).
    Dit overschrijven gebeurt overigens gelukkig vrij zelden (1 a 3 forms / datamodules) dus wat dat betreft leek het mij veilig (ook na testen) er "Self" neer te zetten.

  12. #12
    Marius
    Join Date
    Jul 2013
    Location
    Groningen
    Posts
    178
    De referentie FormRapportage kan in dit geval gewoon weg (totaal overbodige code), acties zou inderdaad mooier zijn maar is meer sugarcode in zulke oude code. Het gebruik van "Self" kan ik niet aanraden want als programmeur ga je dan nadenken waarom iemand dat doet en wat het conflict is/was (je bedoeld waarschijnlijk de variabele).

    Dus ja, het is tamelijk slordige code maar ik ben het helemaal eens met GolezTrol. Namelijk afblijven want anders zul je echt alles bij langs moeten en dat is al die moeite (en testwerk) niet waard. De geschetste problemen zijn bovendien slechts kleinigheden. Het het geeft je echter wel te denken over de globale stijl en opzet van het programma want als ze daar net zo slordig zijn/waren berg je dan maar. Dus tenzij je net toevallig in die hoek een aanpassing moet maken zou ik er voorlopig vanaf blijven (tenzij je alles opnieuw wilt gaan testen of jouw aanpassingen ongewenste effecten hebben met mopperende gebruikers tot gevolg).

  13. #13
    Reader
    Join Date
    May 2002
    Location
    Holland
    Posts
    3,382
    Ik zal voorzichtig zijn. Problematisch is dat de verwijzing naar de globale (ipv Self) uit onwetendheid is gedaan. Wat het eigenlijk des te trickier maakt

  14. #14
    Senior Member Thaddy's Avatar
    Join Date
    Dec 2004
    Location
    Amsterdam
    Posts
    2,211
    Niet voorzichtig zijn. Ook een redelijk geschoolde C++ collega zal gek worden van zulke code. Het heeft nix met de taal te maken maar met onbegrip en onkunde.
    En het draagt zoals je zelf al vermoedde bij aan ononderhoudbare code. Ik geef onder meer les in code review en maak dankbaar gebruik van je voorbeeld
    Werken aan Ansi support voor Windows is verspilde tijd, behalve voor historici.

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •