BorlandTalk.com Borland discussion newsgroups   Archives   FAQ   Search   Memberlist   Usergroups   Register   Profile   Log in to check your private messages   Log in  Is this good programming practice ? Goto page 1, 2, 3  Next          BorlandTalk.com Forum Index -> comp.lang.pascal.delphi.misc View previous topic :: View next topic   Author Message Magnus Hald Guest Posted: Wed Dec 10, 2003 9:04 am    Post subject: Is this good programming practice ? I found this during a code review: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^.Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.Source]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.Data[0],false,false,DataString); This seems lile one method call with deeply nested arguments. ( and it was not the longest one ...). Quite awkward ro read. As I'm new to Delphi/Object Pascal I wonder if there exist any Coding Standards or rules for good programming practice. Thanks for your opinions Magnus Back to top Jan Schnackenberg Guest Posted: Wed Dec 10, 2003 9:20 am    Post subject: Re: Is this good programming practice ? Hi Magnus Hald wrote: Quote: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^.Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.Source]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.Data[0],false,false,DataString); This seems lile one method call with deeply nested arguments. ( and it was not the longest one ...). Quite awkward ro read. You are right when saying that this looks like a method-call, but I'm more concerned about the 5(!) Typecasts in this little line. I, for myself, wouldn't consider this "good practice". I wonder where you found that artefact. ;> Ciao, Jan -- /" ASCII RIBBON PS2-Assistent / CAMPAIGN Antworten bitte ausschliesslich an die Newsgroup X AGAINST HTML Links zu Delphi und dem verhalten in Newsgroups: / IN MAIL AND NEWS ---> http://www.fh-wedel.de/~snb/ <--- Back to top Duncan McNiven Guest Posted: Wed Dec 10, 2003 9:37 am    Post subject: Re: Is this good programming practice ? On 10 Dec 2003 01:04:30 -0800, [email]magnushald (AT) yahoo (DOT) co.uk[/email] (Magnus Hald) wrote: Quote: I found this during a code review: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^.Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.Source]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.Data[0],false,false,DataString); You poor sod! That is horrendous. I don't know of any written programming standards for Delphi, but if any standard accepted that code, I would look for a new standard. -- Duncan Back to top Jeremy Collins Guest Posted: Wed Dec 10, 2003 10:00 am    Post subject: Re: Is this good programming practice ? Magnus Hald wrote: Quote: I found this during a code review: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^.Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.Source]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.Data[0],false,false,DataString); This seems lile one method call with deeply nested arguments. ( and it was not the longest one ...). Quite awkward ro read. As I'm new to Delphi/Object Pascal I wonder if there exist any Coding Standards or rules for good programming practice. This is worth a look: http://delphi.about.com/cs/standards/ Although general good programming practices apply to Delphi of course. (Code Complete is a good place to start: http://tinyurl.com/ykd9 ) And yup, that is a truly horrible line of code. Its ONLY saving graces are correct capitalisation and descriptive naming of variables and methods, making it SLIGHTLY easier to break apart. "Quite awkward" indeed! I take it you don't need any assistance in breaking this down? -- jc Remove the -not from email Back to top Dodgy Guest Posted: Wed Dec 10, 2003 11:26 am    Post subject: Re: Is this good programming practice ? On 10 Dec 2003 01:04:30 -0800, [email]magnushald (AT) yahoo (DOT) co.uk[/email] (Magnus Hald) waffled on about something: Quote: I found this during a code review: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^.Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.Source]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.Data[0],false,false,DataString); This seems lile one method call with deeply nested arguments. ( and it was not the longest one ...). Quite awkward ro read. As I'm new to Delphi/Object Pascal I wonder if there exist any Coding Standards or rules for good programming practice. Thanks for your opinions Magnus Good practice and easy to read go hand in hand... Someone might have to modify the code some day, and a line like that would have me reaching for the JD bottle! But congratulations on finding it, that has to be the worst line of Delphi code I have ever seen! It's good to know that we can keep up with the C++ brigade when it comes to making a complete mess of source code! Dodgy. -- MUSHROOMS ARE THE OPIATE OF THE MOOSES Back to top Ante Smolcic Guest Posted: Wed Dec 10, 2003 11:33 am    Post subject: Re: Is this good programming practice ? In article <se0etvcc3udbi0s4dvd4ftfq2qgvnojmi2 (AT) 4ax (DOT) com>, [email]Dodgy (AT) earth (DOT) planet.univ[/email]erse says... Quote: Good practice and easy to read go hand in hand... Someone might have to modify the code some day, and a line like that would have me reaching for the JD bottle! Exactly, readability is a must nowadays. Back to top J French Guest Posted: Wed Dec 10, 2003 12:42 pm    Post subject: Re: Is this good programming practice ? On 10 Dec 2003 01:04:30 -0800, [email]magnushald (AT) yahoo (DOT) co.uk[/email] (Magnus Hald) wrote: Quote: I found this during a code review: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^.Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.Source]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.Data[0],false,false,DataString); This seems lile one method call with deeply nested arguments. ( and it was not the longest one ...). Quite awkward ro read. Whoever wrote that should be retrained as a chicken plucker - not only is it horrible to read - but it is using multiple variable coercion Quote: As I'm new to Delphi/Object Pascal I wonder if there exist any Coding Standards or rules for good programming practice. Thanks for your opinions Magnus Back to top Eden Kirin Guest Posted: Wed Dec 10, 2003 1:12 pm    Post subject: Re: Is this good programming practice ? On 10 Dec 2003 01:04:30 -0800, Magnus Hald wrote: Quote: I found this during a code review: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^.Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.Source]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.Data[0],false,false,DataString); This seems lile one method call with deeply nested arguments. ( and it was not the longest one ...). Quite awkward ro read. Horrible! I suggest that you change it to something like: var ScriptElement: TScriptElementPtr; Loop: TLoop; LUnit: TUnit; begin ... ScriptElement:=TScriptElementPtr(FScriptElements[FCurrentItem]); Loop:=TLoop(FConfig.Floops[ScriptElement^.Loop]); LUnit:=TUnit(Loop.GetLoopUnitList[ScriptElement^.Source]); LUnit.SendCustomMessage(ScriptElement^.Data[0], false, false, DataString); ... Back to top Duncan McNiven Guest Posted: Wed Dec 10, 2003 1:15 pm    Post subject: Re: Is this good programming practice ? On Wed, 10 Dec 2003 12:33:14 +0100, Ante Smolcic <smolaDEATH_TO_SP (AT) MMERSpost (DOT) hinet.hr> wrote: Quote: In article <se0etvcc3udbi0s4dvd4ftfq2qgvnojmi2 (AT) 4ax (DOT) com>, [email]Dodgy (AT) earth (DOT) planet.univ[/email]erse says... Good practice and easy to read go hand in hand... Someone might have to modify the code some day, and a line like that would have me reaching for the JD bottle! Exactly, readability is a must nowadays. True. Another issue is tracability. You just cannot step through code like this in the debugger in any meaningful way. Making individual steps would both aid readability and allow intermediate results to be checked in the debugger. -- Duncan Back to top David Reeve Guest Posted: Wed Dec 10, 2003 2:19 pm    Post subject: Re: Is this good programming practice ? "Magnus Hald" <magnushald (AT) yahoo (DOT) co.uk> wrote Quote: I found this during a code review: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^ ..Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.So urce]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.D ata[0],false,false,DataString); Quote: Hey what's the problem? Has everyone lost their sense of adventure? Some decades ago a student of mine went to work for a company where coding like this was obligatory company policy, and in C as well, nice and compact it was. Apparently the reason was to save on the cost of printouts. I jest not. I think they went bust before long..... I guess the saving in paper and ribbons was insufficient to offset the fact they never really got a product. Dave Back to top Rob Kennedy Guest Posted: Wed Dec 10, 2003 7:56 pm    Post subject: Re: Is this good programming practice ? Jamie wrote: Quote: i sense a real hard core programmer there! that kind of aprouch is good if you need to access class members to get at the private members that normally wouldn't have published properties for. a little casting to get at inherited classes can go a long ways! The type casting, in this case, is probably not to get at protected members. Rather, it's to get at any members at all. All the brackets are probably accessing TLists, and so the type casting is to get from TList's Pointer elements to the real type of what's being storing in those lists. That can be solved by taking 10 minutes and writing a typed list class -- easy to do by descending from TList or TObjectList. Then the Items property will return the proper type for the list's contents. -- Rob Back to top Bruce Roberts Guest Posted: Wed Dec 10, 2003 9:05 pm    Post subject: Re: Is this good programming practice ? "Magnus Hald" <magnushald (AT) yahoo (DOT) co.uk> wrote Quote: I found this during a code review: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^ ..Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.So urce]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.D ata[0],false,false,DataString); Quote: This seems lile one method call with deeply nested arguments. ( and it was not the longest one ...). Quite awkward ro read. Truely. Horribly written code. This is one of the few situations where I'd probably use a With With tScriptElementPtr (fScriptElements [fCurrentItem])^ do tUnit (tLoop (fConfig.Floops [Loop]).GetLoopUnitList [Source]).SendCustomMessage (Data [0], false, false, DataString); Which is slightly easier to read. The suggestion to use temporary variables, (made by Eden Kirin), would help var theUnit : tUnit; theLoop : tLoop; with tScriptElementPtr (fScripElements [fCurrentItem])^ do begin theLoop := tLoop (fConfig.fLoops [Loop]); theUnit := tUnit (theLoop.GetLoopUnitList [Source]); theUnit.SendCustomMessage (Data [0], false, false, DataString); end; Of course using a With can be problematic and one has to be and insure that one is referencing the desired fields/properties. Back to top Jamie Guest Posted: Wed Dec 10, 2003 10:33 pm    Post subject: Re: Is this good programming practice ? i sense a real hard core programmer there! that kind of aprouch is good if you need to access class members to get at the private members that normally wouldn't have published properties for. a little casting to get at inherited classes can go a long ways! ) Magnus Hald wrote: Quote: I found this during a code review: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^.Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.Source]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.Data[0],false,false,DataString); This seems lile one method call with deeply nested arguments. ( and it was not the longest one ...). Quite awkward ro read. As I'm new to Delphi/Object Pascal I wonder if there exist any Coding Standards or rules for good programming practice. Thanks for your opinions Magnus Back to top Terry Russell Guest Posted: Thu Dec 11, 2003 2:55 am    Post subject: Re: Is this good programming practice ? "Jamie" <jamie_5_not_valid_after_5 (AT) charter (DOT) net> wrote Quote: i sense a real hard core programmer there! that kind of aprouch is good if you need to access class members to get at the private members that normally wouldn't have published properties for. a little casting to get at inherited classes can go a long ways! ) It looks more like the output of a code generator. There may be a file somewhere with the real source meta-code for that line. It could look like a perfectly comprehensible series of vectors in a graphical interface. Quote: Magnus Hald wrote: I found this during a code review: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^ ..Loop]).GetLoopUnitList[TScriptElementPtr> Back to top Jud McCranie Guest Posted: Thu Dec 11, 2003 5:18 am    Post subject: Re: Is this good programming practice ? On Wed, 10 Dec 2003 16:05:09 -0500, "Bruce Roberts" <ber (AT) bounceitattcanada (DOT) xnet> wrote: Quote: TUnit(TLoop(FConfig.Floops[TScriptElementPtr(FScriptElements[FCurrentItem])^ .Loop]).GetLoopUnitList[TScriptElementPtr(FScriptElements[FCurrentItem])^.So urce]).SendCustomMessage(TScriptElementPtr(FScriptElements[FCurrentItem])^.D ata[0],false,false,DataString); Truely. Horribly written code. It is the biggest mess I've ever seen in Pascal or Delphi. Back to top Display posts from previous: All Posts1 Day7 Days2 Weeks1 Month3 Months6 Months1 Year Oldest FirstNewest First         BorlandTalk.com Forum Index -> comp.lang.pascal.delphi.misc All times are GMT Goto page 1, 2, 3  Next Page 1 of 3   Jump to: Select a forum C++ Builder Newsgroups----------------C++ BuilderC++ Builder (ActiveX)C++ Builder (Announcements)C++ Builder (Command Line Tools)C++ Builder DatabasesC++ Builder Databases (ADO)C++ Builder Databases (Desktop)C++ Builder Databases (InterBase Express)C++ Builder Databases (Multi-Tier)C++ Builder Databases (SQL Servers)C++ Builder (Graphics)C++ Builder (IDE)C++ Builder (Internet)C++ Builder (Internet Socket)C++ Builder (Internet Web)C++ Builder (Jobs)C++ Builder (Language)C++ Builder (Language C++)C++ Builder (Compatibility with MFC)C++ Builder (Native API)C++ Builder (Non-Technical)C++ Builder (Students)C++ Builder (Third Party Tools)C++ Builder (Upgrade)C++ Builder (VCL)C++ Builder (VCL Components Usage)C++ Builder (VCL Components Development)C++ Builder (VCL Usage)C++ Builder (WinAPI)C++ Builder Installation Delphi Newsgroups----------------Delphi (General)Delphi ActiveX Controls UsageDelphi ActiveX Controls DevelopmentDelphi As400Delphi DatabasesDelphi Databases (ADO)Delphi Databases (dbExpress)Delphi Databases (Desktop)Delphi Databases (InterBase Express)Delphi Databases (Multi-Tier)Delphi Databases (SQL Servers)Delphi DeploymentDelphi GraphicsDelphi IDEDelphi InternationalizationDelphi InternetDelphi Internet Isapi-WebBrokerDelphi Internet WebSnapDelphi Internet WinsockDelphi JobsDelphi Language BASMDelphi Language ObjectPascalDelphi Model Driven Architecture (General)Delphi Model Driven Architecture (Third Party)Delphi MultimediaDelphi Native APIDelphi Non-TechnicalDelphi ObjectPascalDelphi OLE AutomationDelphi OO designDelphi OpenToolsAPIDelphi Reporting-ChartingDelphi RTLDelphi StudentsDelphi TeamSourceDelphi Thirdparty ToolsDelphi Thirdparty Tools (General)Delphi Thirdparty Tools (IntraWeb)Delphi Thirdparty Tools (RaveReports)Delphi UpgradeDelphi VCLDelphi VCL ComponentsDelphi VCL Components UsingDelphi VCL Components WritingDelphi WebServices SOAPDelphi WinAPIDelphi XMLDelphi Installationcomp.lang.pascal.delphi.components.misccomp.lang.pascal.delphi.components.usagecomp.lang.pascal.delphi.components.writingcomp.lang.pascal.delphi.databasescomp.lang.pascal.delphi.misccomp.sources.delphi JBuilder Newsgroups----------------JBuilderJBuilder AnnouncementsJBuilder Applet IssuesJBuilder As400JBuilder CompilerJBuilder CORBA/RMIJBuilder DatabasesJBuilder DB SwingJBuilder DebuggerJBuilder DeploymentJBuilder DocumentationJBuilder EnterpriseJBuilder HandheldJBuilder IDEJBuilder JavaJBuilder JavaAPIJBuilder Java LanguageJBuilder Java SwingJBuilder Java BeansJBuilder Java Beans UsageJBuilder Java Beans DevelopmentJBuilder JBCLJBuilder JDataStoreJBuilder JobsJBuilder LinuxJBuilder MacintoshJBuilder & MobileSetJBuilder MultiLingual AppsJBuilder Non-TechnicalJBuilder OpenToolsJBuilder Servlets (JSP)JBuilder StudentsJBuilder Team DevelopmentJBuilder Thirdparty ToolsJBuilder UpgradeJBuilder Installation InterBase Newsgroups----------------InterBaseInterBase Bug DiscussionInterBase GeneralInterBase IBConsoleInterBase OpenSourceInterBase ReplicationInterBase SQLInterBase ThirdpartyInterBase Installation Miscellaneous Borland Newsgroups----------------AppCenterAppServerEnteraVisiBroker  You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum Powered by phpBB © 2001, 2006 phpBB Group SEO toolkit © 2004-2006 webmedic.