Maxime Janvier 0 Posted July 25 Hi, I have quite a tricky problem, and can't figure out a proper solution. You see, I want to log debug strings in my application, so I wrote a module where the only entry point is a pure method "procedure Log(const LogLevel: TLogLevel; const Str: String);". So the goal would be to only log strings if the logger is set to log at least that log level. That way, I can easily change the logger to only log error strings for example. So far so good. Problem is, when I write things like "Log(llTrace, 'Test ' + GetLastError + BuildStringFromContext); for example, the string will be evaluated wether I log it or not, because the log filter is done inside the function. I was wandering if there was a way to avoid the evaluation of the string when I know I won't log it? Of course the obvious solution would be to write "if CanLog(llTrace) then Log('Test ' + GetLastError + BuildStringFromContext);" But that would be tedious to write, and make the code harder to read. So I though of two possibilities: - First: Using inline function, to replace at compile time the first Log line by the if/then statement. Unfortunately, even with inline function, the arguments are fully eveluated when the function is called (tested). So it's a dead end. - I also though to use a preprocessor macro, to replace at compile time the fist line by the if/then statement, but Delphi doesn't support C/C++ like defines, it can only set/evaluate booleans.  So do you think of a solution i could've overlooked? Without any solution, I'll stick to my first Log version, as I value code cleanliness over performance, but it's still a bit frustrating knowing my code will execute instructions for nothing at all if I disable my log.... Regards, Share this post Link to post
Vandrovnik 214 Posted July 25 May be something like this, but it looks even more complicated then the test with if CanLog... 😄  type tLogFunction = reference to function: string; procedure Log(aLevel: integer; aGetLogText: tLogFunction); var s: string; begin if aLevel>1 then begin s:=aGetLogText; writeln(s); end; end; ... Log(0, function:string begin result:='abc ' + IntToStr(random(100)); end); Log(2, function:string begin result:='efg ' + IntToStr(random(100)); end);  Share this post Link to post
Maxime Janvier 0 Posted July 25 That would be a solution indeed. But performance-wise, I don't know the cost of anonymous functions.. What if that's called in a loop? Even if it' not called, some memory has to be dynamically allocated for the function code + the captured variables + the cost of capturing those variables... I might do a benchmark for that... Share this post Link to post
Kas Ob. 121 Posted July 25 There is few ways i can think of like these 2 approaches type TLogLevel = (llInfo, llTrace, llError); procedure Log(LogLevel: TLogLevel; const LogText: string); overload; begin end; procedure Log(LogLevel: TLogLevel; const Text: array of string); overload; var Count: Integer; begin Count := Length(Text); case LogLevel of llInfo: ; llTrace: begin if Count > 0 then begin end; end; llError: ; end; end; procedure Log(LogLevel: TLogLevel; const LogText: string = ''; const Error: string = ''; const Additional: string = ''); overload; begin end; procedure Log(LogLevel: TLogLevel; const LogText: string = ''; ErrorCode: Integer = 0; const Additional: string = ''); overload; begin end; procedure TForm10.FormCreate(Sender: TObject); var BuildStringFromContext: string; begin Log(llTrace, ['Test', IntToStr(GetLastError), BuildStringFromContext]); Log(llError, 'Test', GetLastError, BuildStringFromContext); Log(llError, 'Simple Text', 0); Log(llInfo, 'Simple Text', 0, 'Additional Info'); Log(llInfo, 'Simple Text', ''); end; Just food for thoughts for you, using array of string is easy and will make you life easier, but you should remember the order, while the overloaded versions with default empty values, are more readable and harder to confuse.  Each had its own shortcoming, like i had to add 0 do the ambiguity, but if you make you mind you can build them right and remove the need for these extra 0 or ''. It is up to your need to think of a solution that you are comfortable with. Share this post Link to post
Vandrovnik 214 Posted July 25 4 minutes ago, Maxime Janvier said: That would be a solution indeed. But performance-wise, I don't know the cost of anonymous functions.. What if that's called in a loop? Even if it' not called, some memory has to be dynamically allocated for the function code + the captured variables + the cost of capturing those variables... I might do a benchmark for that... In a loop, I would store the result of CanLog to a local boolean variable and then use "if StoredValueOfCanLog>=..." and direct call to your Log procedure (without anonymous function). Share this post Link to post
Maxime Janvier 0 Posted July 25 28 minutes ago, Kas Ob. said: There is few ways i can think of like these 2 approaches type TLogLevel = (llInfo, llTrace, llError); procedure Log(LogLevel: TLogLevel; const LogText: string); overload; begin end; procedure Log(LogLevel: TLogLevel; const Text: array of string); overload; var Count: Integer; begin Count := Length(Text); case LogLevel of llInfo: ; llTrace: begin if Count > 0 then begin end; end; llError: ; end; end; procedure Log(LogLevel: TLogLevel; const LogText: string = ''; const Error: string = ''; const Additional: string = ''); overload; begin end; procedure Log(LogLevel: TLogLevel; const LogText: string = ''; ErrorCode: Integer = 0; const Additional: string = ''); overload; begin end; procedure TForm10.FormCreate(Sender: TObject); var BuildStringFromContext: string; begin Log(llTrace, ['Test', IntToStr(GetLastError), BuildStringFromContext]); Log(llError, 'Test', GetLastError, BuildStringFromContext); Log(llError, 'Simple Text', 0); Log(llInfo, 'Simple Text', 0, 'Additional Info'); Log(llInfo, 'Simple Text', ''); end; Just food for thoughts for you, using array of string is easy and will make you life easier, but you should remember the order, while the overloaded versions with default empty values, are more readable and harder to confuse.  Each had its own shortcoming, like i had to add 0 do the ambiguity, but if you make you mind you can build them right and remove the need for these extra 0 or ''. It is up to your need to think of a solution that you are comfortable with. Thanks for these approaches! Unfortunately, the strings (or the functions that'll generate the strings) will still be individually evaluated, if I call Log(ttTrace, [SysErrorMessage(GetLastError)]);, the SysErrorMessage function will still be called. Share this post Link to post
Maxime Janvier 0 Posted July 25 29 minutes ago, Vandrovnik said: In a loop, I would store the result of CanLog to a local boolean variable and then use "if StoredValueOfCanLog>=..." and direct call to your Log procedure (without anonymous function). That's actually good! I think that I'll normally use the Log(LogLevel, MyString), but when obvious optimizations appear, I could add a CanLog to avoid evals in loops... Â In that way, I can have a proper balance between code cleanliness/ease of reading and performances. Share this post Link to post
Kas Ob. 121 Posted July 25 14 minutes ago, Maxime Janvier said: if I call Log(ttTrace, [SysErrorMessage(GetLastError)]);, the SysErrorMessage function will still be called. Well my point was about to not call SysErrorMessage in the first place, just pass the GetLastError, even more on this, if the code between Log and where actually it implemented can't modify The GetLastError, then you could use ttTraceIWithError and there you can call SysErrorMessage. In case you care about performance then just use a method and track the generated assembly  And by replacing the most repeated or even the specific phrases like 'Test' and 'Exception'... with LOG_String_XX you can short and fast logging.  Again food for thoughts. Share this post Link to post
Maxime Janvier 0 Posted July 25 5 minutes ago, Kas Ob. said: Well my point was about to not call SysErrorMessage in the first place, just pass the GetLastError, even more on this, if the code between Log and where actually it implemented can't modify The GetLastError, then you could use ttTraceIWithError and there you can call SysErrorMessage. In case you care about performance then just use a method and track the generated assembly  And by replacing the most repeated or even the specific phrases like 'Test' and 'Exception'... with LOG_String_XX you can short and fast logging.  Again food for thoughts. Good points!  That would require a bit of work, but might worth it!  Thank you very much for your time and thoughts, much appreciated! :) Share this post Link to post
Remy Lebeau 1396 Posted July 25 8 hours ago, Maxime Janvier said: - I also though to use a preprocessor macro, to replace at compile time the fist line by the if/then statement, but Delphi doesn't support C/C++ like defines, it can only set/evaluate booleans. FreePascal supports preprocessor macros; https://www.freepascal.org/docs-html/3.2.0/prog/progse5.html  For Delphi, you would have to write/find an external tool to preprocess your code before compiling it. Share this post Link to post
Brian Evans 105 Posted July 25 Having the logging expressions compiled and executed on a regular basis means they are less likely to break unnoticed and cause issues when you need them.  Building logging strings is usually a small fraction of execution time so keeping things simple is a valid choice. Share this post Link to post
Remy Lebeau 1396 Posted July 25 11 hours ago, Kas Ob. said: procedure Log(LogLevel: TLogLevel; const Text: array of string); overload;  Since performance is in question, I would suggest taking this approach a step further and use 'array of const' instead of 'array of string'. That way, you can pass in integers and other fundamental types as-is and not invoke the overhead of converting them to strings unless you are sure you actually need to, eg: procedure Log(LogLevel: TLogLevel; const Args: array of const); overload; begin if CanLog(LogLevel) then begin // convert Args to a log message and write it out as needed... end; end; Log(llTrace, ['Test', GetLastError(), ...]);  3 Share this post Link to post
Kas Ob. 121 Posted July 26 9 hours ago, Remy Lebeau said: use 'array of const' instead of 'array of string'. I never thought about this.  Thank you ! Share this post Link to post
Kas Ob. 121 Posted July 26 Can't shake it off without trying to validate the type, so here what i tried and it is working perfectly {$APPTYPE CONSOLE} {$R *.res} uses System.SysUtils; procedure DoProcess(const Args: array of const); var i, Count: Integer; begin Count := Length(Args); if Count > 0 then for i := 0 to Pred(Count) do begin case Integer(Args[i].VType) of vtString, vtChar, vtWideChar, vtAnsiString, vtWideString, vtUnicodeString: Writeln(string(Args[i].VString)); vtInteger: Writeln(Args[i].VInteger); vtInt64: Writeln(Int64(Args[i].VInt64)); else Writeln('Type handling not implemented yet ' + IntToStr(Args[i].VType)); end; end; end; procedure Test; var st: string; begin st := 'Param1'; DoProcess([st, 'Param2', 5]); end; begin Test; Readln; end. output Quote Param1 Param2 5  The generated assembly is nice and short, one thing though, don't forget to declare Args itself as const as in procedure DoProcess(const Args: array of const) or it will be copied locally, in other words it will be ugly. Share this post Link to post
timfrost 78 Posted July 26 I cannot believe how complex this discussion has become! All to avoid having a tiny unit with four boolean variables defined in the interface, along with a GetLogLevel function which can be called when needed, and then writing, for example: if LOGINFO then log(MyComplexStringExpression); if LOGALL then Log(AnotherExpression); This seems to meet all the criteria of readability, simplicity and efficiency. Share this post Link to post
dummzeuch 1505 Posted July 26 2 hours ago, timfrost said: This seems to meet all the criteria of readability, simplicity and efficiency. It's cumbersome to have that if statement all the time. Share this post Link to post
Vandrovnik 214 Posted July 26 48 minutes ago, dummzeuch said: It's cumbersome to have that if statement all the time. If log level is defined in compile time, he could also use $IFDEF or $IF instead 😄 Share this post Link to post
Remy Lebeau 1396 Posted July 26 (edited) 8 hours ago, Kas Ob. said: Can't shake it off without trying to validate the type, so here what i tried and it is working perfectly Not quite. You have implemented DoProcess() incorrectly, causing undefined behavior. The code compiles, but it will not behave correctly at runtime for certain types. For instance, try passing a single Char into it and your code will crash. 8 hours ago, Kas Ob. said: case Integer(Args[i].VType) of vtString, vtChar, vtWideChar, vtAnsiString, vtWideString, vtUnicodeString: Writeln(string(Args[i].VString));  That approach is wrong. You cannot treat all of those types the same way, as they are stored in different ways in the array. vtChar and ctWideChar are stored as individual values in the array itself (like integers are), whereas vtString is a pointer to a ShortString variable, whereas vtAnsiString and vtUnicodeString are copies of the data block pointer from inside of an AnsiString/UnicodeString instance, etc.  You need to handle the various types individually and correctly, eg: procedure DoProcess(const Args: array of const); var i: Integer; begin for i := Low(Args) to High(Args) do begin case Args[i].VType of vtString: Writeln(Args[i].VString^); vtChar: Writeln(Args[i].VChar); vtPChar: Writeln(Args[i].VPChar); vtWideChar: Writeln(Args[i].VWideChar); vtPWideChar: Writeln(Args[i].VPWideChar); vtAnsiString: Writeln(AnsiString(Args[i].VAnsiString)); vtWideString: Writeln(WideString(Args[i].VWideString)); vtUnicodeString: Writeln(UnicodeString(Args[i].VUnicodeString)); vtInteger: Writeln(Args[i].VInteger); vtInt64: Writeln(Int64(Args[i].VInt64)); else Writeln('Type handling not implemented yet ' + IntToStr(Args[i].VType)); end; end; end; Also, see the MakeStr() example in this earlier post:  Edited July 26 by Remy Lebeau 1 1 Share this post Link to post
dummzeuch 1505 Posted July 26 4 hours ago, Vandrovnik said: If log level is defined in compile time, he could also use $IFDEF or $IF instead 😄 Yes, some special characters plus if plus ifend are a lot more convenient than an if then statement. I'm sold. 2 Share this post Link to post
David Schwartz 426 Posted July 27 On 7/26/2024 at 4:08 AM, timfrost said: I cannot believe how complex this discussion has become! All to avoid having a tiny unit with four boolean variables defined in the interface, along with a GetLogLevel function which can be called when needed ... Delphi is a few decades behind contemporary language features today. The last several language additions have been around since the 90's. I'm not sure how far into the 2000's they are yet. I think I'll be long dead before they catch up with the more popular language features that most languages already support today in 2024.  I mean ... hey ... the only thing added in D12 was multi-line string literals. WOOT! WOOT!  I'm not moving off of D10.4.2 until they start focusing on real language improvements. Given how many emails I'm getting lately trying to urge people to get back onto maintenance plans, I'm guessing the user base is shrinking ... maybe because nobody wants to use a language that's still stuck in the 90's when most contemporary languages are still evolving quite steadily.  I think Delphi introduced anonymous methods and closures before Java did. But since then, Java has updated them with nearly every release, and they look nothing like they did initially. I can't think of anything that has changed with Delphi's syntax. 1 Share this post Link to post
Lajos Juhász 293 Posted July 27 44 minutes ago, David Schwartz said: I mean ... hey ... the only thing added in D12 was multi-line string literals. WOOT! WOOT! do not forget that multi-line strings completely broke the IDE. For D13 they will have to change the code in the IDE to support them. Share this post Link to post
dummzeuch 1505 Posted July 27 1 hour ago, David Schwartz said: I think Delphi introduced anonymous methods and closures before Java did. No, you are wrong. Even anonymous methods were in Java first. Share this post Link to post
Remy Lebeau 1396 Posted July 27 (edited) 3 hours ago, dummzeuch said: No, you are wrong. Even anonymous methods were in Java first. Java introduced anonymous classes in Java 1.1 in 1997. Delphi introduced anonymous methods in 2009. Java introduced anonymous lambdas/closures in Java 8 in 2014 (but they were first proposed in 2006). Edited July 27 by Remy Lebeau Share this post Link to post
David Heffernan 2345 Posted July 28 On 7/27/2024 at 6:30 PM, Remy Lebeau said: Delphi introduced anonymous methods in 2009 What specific feature are you talking about here? Share this post Link to post
Remy Lebeau 1396 Posted July 29 On 7/28/2024 at 11:11 AM, David Heffernan said: What specific feature are you talking about here? Really? http://docwiki.embarcadero.com/RADStudio/en/Anonymous_Methods_in_Delphi Share this post Link to post