Funny Delphi code of the day: on literals
Posted by jpluimers on 2013/12/09
The fact that the [WayBack] 31-character limit on ClientDataSet field and index names is worse, but I just encountered this GetParamNameWODog function in a Delphi 3rd party library:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| function GetParamNameWODog(const ParamName: _string): _string; | |
| begin | |
| if (ParamName <> '') and (ParamName[1] = '@') then | |
| Result := Copy(ParamName, 2, 1000) | |
| else | |
| Result := ParamName; | |
| end; |
What’s wrong with using the Length function here?
Yes, SQL Parameter names will probably less than 1000 characters, but then the 1000 literal should be a constant with a meaningful name, and the '@' literal should be too.
Apparently, Dog is a meaningful name for the at-sign, as it is not the name for a SQL parameter prefix, but it is not a named const either in the rest of their code:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| DogPresent := St = '@'; | |
| if DogPresent then | |
| //… | |
| if //… | |
| then begin | |
| if DogPresent then | |
| ParamName := '@' + St | |
| else | |
| ParamName := St; | |
| //… | |
| if DogPresent then | |
| ParsedSQL.Append('@'); | |
| //… | |
| end; |
–jeroen
The Wiert Corner - irregular stream of stuff 
Ritsaert Hornstra said
The question is not what’s wrong with the length function, but why MaxInt is not used as the length.
jpluimers said
That’s because, for various reasons, the maximum string length isn’t guaranteed to be MaxInt for all types of strings: http://docwiki.embarcadero.com/RADStudio/XE5/en/String_Types_(Delphi)
Anthony Frazier said
There aren’t any cases where it’s greater than MaxInt, is it? The Copy() routine will clip the value to Length() anyway. Since Copy() is going to check length no matter what, I always pass in MaxInt if I want the rest of the string.
jpluimers said
So you rather depend on an implementation dependency than doing this little extra effort of making it right?
I’ve bitten by too many implementation dependencies in the past, so I don’t rely on those any more.
Anthony Frazier said
It’s not an implementation dependency, it’s a documented behavior.
From: http://docwiki.embarcadero.com/Libraries/XE5/en/System.Copy
“If Count specifies more characters or array elements than are available, only the characters or elements from S[Index] to the end of S are returned.”
There’s also…
“If Index is larger than the length of S, Copy returns an empty string or array.”
Copy is a very safe thing to call with all kinds of parameters.
jpluimers said
It is documented behavior for the current version. Just like the string length was 255 for Delphi < 2 and strings were single byte for Delphi < 2009. The behavior comes from processor and/or memory architecture. Those change over time and have caused lots of issues. It is really simple to prepare for those and developers should as the cost will be much lower than finding the bugs and fixing them afterwards.
Stefan Glienke said
Actually you can omit the 3rd parameter on Copy which means “til the end”.
P.S. WTF is _string?
jpluimers said
Cool, I learned yet another new thing today.
Anthony Frazier said
Wait, what? I knew you could omit both the second and third parameters, but never just the third.
jpluimers said
I’m not sure when it got added, but in the past there were fewer overloads. Good to know they are there. Will surely refactor some old code when I come across it.
ain said
“Apparently, Dog is a meaningful name for the at-sign”
The code is probably written by russian as AFAIK in russian they call at-sign “sobatska” which means “doggy”
jpluimers said
Thanks for that clarification. Learned something new today (: