Lazarus

Programming => General => Topic started by: SymbolicFrank on July 15, 2019, 05:39:36 pm

Title: Exceptions
Post by: SymbolicFrank on July 15, 2019, 05:39:36 pm
Do you use them? I very rarely do. I do use try..except when needed, but often without anything in the except  :-[ I like to handle errors myself and gracefully close everything down if needed.

Half my code is testing values (all parameters, fields and globals used) and working around unexpected ones.
Title: Re: Exceptions
Post by: Thaddy on July 15, 2019, 06:06:31 pm
I use - in production code - exceptions, but only for  exceptions: unintended code paths or failures. With perfect programmers, perfect hardware and perfect compilers exceptions are nonsense, that is true.
Usually it is better to use errors. But almost nobody uses those  :o :( :(

I might add that I am a heavy user of asserts...
But exceptions are still a valid mechanism to protect users, not programmers:
Ever seen this?
Code: Pascal  [Select]
  1. try
  2.   // do something
  3. except
  4.  // if it fails do nothing.....
  5. end;.

Now, when I see such code I get really angry..... Happens all the time....
Title: Re: Exceptions
Post by: Handoko on July 15, 2019, 06:15:43 pm
I rarely use exception. I always think all the possible errors that may happen and I handle them myself. I know what assertion is but I never use it.

I'm not a good programmer.  :-[
Title: Re: Exceptions
Post by: Thaddy on July 15, 2019, 06:21:18 pm
I'm not a good programmer.  :-[
That's not true. :D
Title: Re: Exceptions
Post by: ASerge on July 15, 2019, 06:35:57 pm
Half my code is testing values (all parameters, fields and globals used) and working around unexpected ones.
Only if can get around it. For example in case of unexpected structure of external data (files, databases, responses from devices, etc) I raise an exception. Of course, this does not include cases where it can simply be ignored. Also when writing universal modules (used in more than one program).
Title: Re: Exceptions
Post by: engkin on July 15, 2019, 07:16:20 pm
Apart from digging in the source code, how do you find out all the exceptions a call might raise?
Title: Re: Exceptions
Post by: Kays on July 15, 2019, 08:35:28 pm
Apart from digging in the source code, how do you find out all the exceptions a call might raise?
Documentation.
Title: Re: Exceptions
Post by: engkin on July 15, 2019, 08:47:10 pm
Apart from digging in the source code, how do you find out all the exceptions a call might raise?
Documentation.
In the general case you get libraries with/without documentation. Let's try with TFPHTTPClient.Get?
Title: Re: Exceptions
Post by: mas steindorff on July 15, 2019, 09:24:51 pm
I use them whenever I use "outside" code, like any library call.  for example, format() is a function that can be tested and tested and then a strange value will sneak in and crash your program. 
Title: Re: Exceptions
Post by: SymbolicFrank on July 15, 2019, 09:42:23 pm
I use try..except when calling a function/method that throws an exception when something goes wrong. It's one way to flag an error, I guess.

But that raises an interesting question: what makes something an error? Say, if I have to calculate something according to three user inputs, do I flag it as an error when one or two of those aren't filled?

I never determine up front what path my code is going to take. I try to have my code give its best effort according to the available inputs. Try to guess what the optimal response would be in each case.

Compared to simply raising an exception and aborting execution whenever the predetermined path cannot be followed, I think my programs encounter 1000+% less bugs. Simply because they do check and ponder.
Title: Re: Exceptions
Post by: Handoko on July 15, 2019, 10:23:28 pm
I usually write code like this:

function MyFunction(val1, val2, ...): TOutput;
begin

  // set the default output value
  Result := 0;

  // make sure the inputs are valid
  if not(val1 valid) then Exit;
  if not(val2 valid) then Exit;

  // do the process
  if somethingbad then Exit;
  Result := process output;

end;

I always avoid try..except block, I personally think it is not good to show that 'panic' message to users. In the old DOS days I use IOResult, now I use all the think I can to check if the process may go wrong.

I am okay to use try..finally block to free the object/memory if I perform some hardware operations, like disk operation. But I usually will check the existence of the target/source file, the free disk space, etc first before performing that 'dangerous' operation.

Maybe not the best, but that it is what I usually do.
Title: Re: Exceptions
Post by: SymbolicFrank on July 15, 2019, 10:55:44 pm
You're halfway there  ;D

Just think if you could do something else useful instead of exiting.
Title: Re: Exceptions
Post by: VTwin on July 15, 2019, 11:31:24 pm
I suppose I am similar to Handoko, his function returns 0 if there is a problem. Any needed user message can be sent after calling it. I try to keep error messages out of lower level functions.

I always test for division by zero, etc. I find the behavior is different on Mac and Win, Mac seems more accepting of nan than Win. Sometimes I find bugs only after testing on more than one platform.
Title: Re: Exceptions
Post by: engkin on July 15, 2019, 11:42:23 pm
What about operators?

Let's assume the following operator:
Code: Pascal  [Select]
  1. operator /(A,B:String):double;
  2. var
  3.   dA,dB: double;
  4.   iCode: integer;
  5. begin
  6.   Val(A, dA, iCode);
  7.   if iCode<>0 then Error;
  8.  
  9.   Val(B, dB, iCode);
  10.   if iCode<>0 then Error;
  11.  
  12.   Result := dA/dB;
  13. end;

How do you handle the Error above?
What about division by zero?
Title: Re: Exceptions
Post by: VTwin on July 16, 2019, 12:37:11 am
What about operators?

Let's assume the following operator:
Code: Pascal  [Select]
  1. operator /(A,B:String):double;
  2. var
  3.   dA,dB: double;
  4.   iCode: integer;
  5. begin
  6.   Val(A, dA, iCode);
  7.   if iCode<>0 then Error;
  8.  
  9.   Val(B, dB, iCode);
  10.   if iCode<>0 then Error;
  11.  
  12.   Result := dA/dB;
  13. end;

How do you handle the Error above?
What about division by zero?

Good example, but I would not be comfortable using such a function. I have numerous functions to convert user strings to, and from, doubles, including including degrees or gradians as strings to radians, latitude and longitude strings (deg min sec, decimal deg, etc.) to radians, and other things, so check at that level (file, editbox, etc) for valid input. For division by zero, I check that Abs(b) > eps (such as 1e-9).

The closest is something like:

Code: Pascal  [Select]
  1. operator / (a: Matrix3; b: double) c: Matrix3;
  2. begin
  3.   result := Mul(a, 1.0/b);
  4. end;  

but I check b before calling. I'm not claiming it is the best way, it is just how I do things.
Title: Re: Exceptions
Post by: 440bx on July 16, 2019, 12:45:44 am
@SymbolicFrank

I use try..except when calling a function/method that throws an exception when something goes wrong. It's one way to flag an error, I guess.
Yes, a very poor way of flagging an error.  One that is unfortunately all too common in OOP.


@Handoko,

I always avoid try..except block, I personally think it is not good to show that 'panic' message to users. In the old DOS days I use IOResult, now I use all the think I can to check if the process may go wrong.
What you're doing is the right way of doing it, ensure the entry conditions are as they should be to guarantee the function/procedure can successfully perform its task (whatever it may be.)

I am okay to use try..finally block to free the object/memory
if the program is structured correctly, you don't need any try/finally blocks either.

Maybe not the best, but that it is what I usually do.
You're on the right track, from what you posted, you're quite close to doing it the best way.


I try to keep error messages out of lower level functions.
That's a good practice.  Prevents having error messages being set all over the program instead of in one centralized location where they can be managed.

exceptions are the right tool to use for a program that deals with resources it doesn't own. When used to "protect" against some mishap when dealing with resources it owns, they simply are an indication of poor programming/design.

Title: Re: Exceptions
Post by: VTwin on July 16, 2019, 01:17:08 am
@engkin

To expand a little, I define:

Code: Pascal  [Select]
  1. const
  2.   NAIOFloat : double  = -1.7e+308;   // explicit missing value, i/o = 'NaN'
  3.   NAFloat   : double  = -1.6e+308;   // missing value, i/o = ''
  4.   MaxFloat  : double  = 1.5e+308;    // use +/- as min-max range
  5.  
  6. { IsNA
  7.   'Is Not Available', returns true if number is not valid. }
  8. function  IsNA(r: double): boolean;
  9. begin
  10.   result := (r = NAFloat) or (r = NAIOFloat);
  11. end;
  12.  
  13. { IsNum
  14.   Uses NAFloat, a very large magnitude negative double whose magnitude
  15.   is bigger than MaxFloat. Use NAFloat to initialize doubles, and -MaxFloat for
  16.   smallest double. }
  17. function IsNum(r: double): boolean;
  18. begin
  19.   result := (r > NAFloat);
  20. end;

So I might return NAFloat for your function to indicate it did not work, and check it with IsNum. I find that NAN is not usable cross-platform. Initialized data that is not input yet is set to NAFloat, data that is explicitly not a number is set to NAIOFloat. A spreadsheet of data values displays '' or 'NaN' for those values.
Title: Re: Exceptions
Post by: engkin on July 16, 2019, 01:50:39 pm
@SymbolicFrank

I use try..except when calling a function/method that throws an exception when something goes wrong. It's one way to flag an error, I guess.
Yes, a very poor way of flagging an error.  One that is unfortunately all too common in OOP.
CPU exceptions included?

I am okay to use try..finally block to free the object/memory
if the program is structured correctly, you don't need any try/finally blocks either.
Can you elaborate a bit more here?
Title: Re: Exceptions
Post by: engkin on July 16, 2019, 01:57:14 pm
@engkin

To expand a little, I define:

Code: Pascal  [Select]
  1. const
  2.   NAIOFloat : double  = -1.7e+308;   // explicit missing value, i/o = 'NaN'
  3.   NAFloat   : double  = -1.6e+308;   // missing value, i/o = ''
  4.   MaxFloat  : double  = 1.5e+308;    // use +/- as min-max range
  5.  
  6. { IsNA
  7.   'Is Not Available', returns true if number is not valid. }
  8. function  IsNA(r: double): boolean;
  9. begin
  10.   result := (r = NAFloat) or (r = NAIOFloat);
  11. end;
  12.  
  13. { IsNum
  14.   Uses NAFloat, a very large magnitude negative double whose magnitude
  15.   is bigger than MaxFloat. Use NAFloat to initialize doubles, and -MaxFloat for
  16.   smallest double. }
  17. function IsNum(r: double): boolean;
  18. begin
  19.   result := (r > NAFloat);
  20. end;

So I might return NAFloat for your function to indicate it did not work, and check it with IsNum. I find that NAN is not usable cross-platform. Initialized data that is not input yet is set to NAFloat, data that is explicitly not a number is set to NAIOFloat. A spreadsheet of data values displays '' or 'NaN' for those values.
Thank you for this explanation. I see that you reduced the range of the type double using MaxFloat, and used two values outside that range to indicate NAIOFloat and NAFloat. Probably for accuracy reasons?

In the general case this is not possible, or not desirable. For instance, consider integer instead of double. How do you handle errors then?
Title: Re: Exceptions
Post by: engkin on July 16, 2019, 01:59:29 pm
I always avoid try..except block, I personally think it is not good to show that 'panic' message to users. In the old DOS days I use IOResult, now I use all the think I can to check if the process may go wrong.

This "panic" message is the result of not using, or not knowing how to use exceptions. It happens when you do not "catch" exceptions. That is why I asked how to find all possible exceptions from a call.
Title: Re: Exceptions
Post by: SymbolicFrank on July 16, 2019, 02:11:21 pm
I like this example:

Code: Pascal  [Select]
  1. program test;
  2.  
  3. var
  4.   s: string;
  5. begin
  6.   s := '12345';
  7.   s := Copy(s, -1, MAXINT);
  8.   WriteLn(s);
  9. end.
  10.  

Of course the index and length of the Copy() are invalid, but you still get a result. While many different languages (and different Pascal implementations) would raise an exception, because the result would be incorrect. While strictly true, I like the graceful result.

That raises another question: does your logic keeps working with partial results? That's why you check your inputs.

About returning errors: In C, atoi("bla!") returns 0 (zero). That is bad, because you don't know if an error occurred. You could do: 'BOOL atoi("bla!", ReturnVar)', but that makes for messy code. 'int atoi("bla!", DefaultValue)' is better. And otherwise do raise that exception, and catch it.
Title: Re: Exceptions
Post by: 440bx on July 16, 2019, 02:29:21 pm
CPU exceptions included?
In user mode - ring 3 - most CPU exceptions are caused by bugs in the program.  An exception (pun intended ;)) are the debug exceptions and those are handled by the OS and routed to the debugger as events.  In a bug-free program that only concerns itself with resources it owns, there should not be any exceptions.

Can you elaborate a bit more here?
Sure.  The most common use of try/finally is to ensure that resources that have been allocated are de-allocated when no longer necessary.

If the variables that hold the handles and/or pointers to those resources are initialized before they are used then a try/finally is unnecessary.  For instance
Code: Pascal  [Select]
  1. function FileMap (Filename : pchar) : pbyte;
  2.   // maps into memory the file to dump
  3. var
  4.   FileHandle           : THANDLE = 0;
  5.   FileMapping          : THANDLE = 0;
  6.  
  7.   p                    : pbyte   = nil;
  8.  
  9.   SCOPE                : integer;
  10.  
  11. const
  12.   FUNCTION_ID          = 'B10: ';
  13.  
  14.   // constants used by CreateFile
  15.  
  16.   NO_TEMPLATE_FILE     =       0;
  17.  
  18.   // constants used by CreateFileMapping
  19.  
  20.   NO_MAXIMUM_SIZE_HIGH =       0;      // 0 indicates to use the size of the
  21.   NO_MAXIMUM_SIZE_LOW  =       0;      //   file
  22.  
  23.   // constants used by MapViewOfFileEx
  24.  
  25.   FILE_OFFSET_HIGH     =       0;      //   file offset to map from
  26.   FILE_OFFSET_LOW      =       0;
  27.  
  28.   BEGINNING_TO_END     =       0;
  29.  
  30. begin
  31.   result := nil;              // default return value
  32.  
  33.   if IsBadStringPtrA(Filename, STRING_MAX_LENGTH)                   then exit;
  34.  
  35.   for SCOPE := 1 to 1 do      // trick to create a scope one can break out of
  36.   begin
  37.     FileHandle := CreateFileA(Filename,
  38.                               GENERIC_READ,
  39.                               FILE_SHARE_READ,
  40.                               nil,
  41.                               OPEN_EXISTING,
  42.                               FILE_ATTRIBUTE_NORMAL,
  43.                               NO_TEMPLATE_FILE);
  44.  
  45.     if FileHandle = INVALID_HANDLE_VALUE then
  46.     begin
  47.       OutputToDeviceA(FUNCTION_ID,
  48.                       'unable to open ', Filename);
  49.       Newline();
  50.       break;
  51.     end;
  52.  
  53.     // with the file handle, create a mapping for it
  54.  
  55.     FileMapping := CreateFileMappingA(FileHandle,
  56.                                       nil,
  57.                                       PAGE_READONLY,
  58.                                       NO_MAXIMUM_SIZE_HIGH,  // use file size
  59.                                       NO_MAXIMUM_SIZE_LOW,
  60.                                       nil);
  61.     if (FileMapping = 0) then
  62.     begin
  63.       OutputToDeviceA(FUNCTION_ID, 'unable to map ', Filename);
  64.       Newline();
  65.       break;
  66.     end;
  67.  
  68.     p := MapViewOfFileEx(FileMapping,
  69.                          FILE_MAP_READ,
  70.                          FILE_OFFSET_HIGH,                   // from beginning
  71.                          FILE_OFFSET_LOW,
  72.                          BEGINNING_TO_END,                   // to end
  73.                          nil);                               // map anywhere
  74.  
  75.     if p = nil then
  76.     begin
  77.       OutputToDeviceA(FUNCTION_ID,
  78.                       'unable to map view of file', Filename);
  79.       Newline();
  80.       break;
  81.  
  82.       // the note below applies only when a non nil address is specified to
  83.       // map at a specific address
  84.  
  85.       // NOTE: MapViewOfFileEx fails if it cannot map at the specified address.
  86.       //       in other words, it will NOT attempt to map to a different address
  87.       //       to succeed.  Because of this there is no need to check if the
  88.       //       address returned is equal to the address specified.
  89.  
  90.       // The documentation states:
  91.  
  92.       // Reserved and committed pages are released when the view is unmapped and
  93.       // the file mapping object is closed. For details, see the UnmapViewOfFile
  94.       // and CloseHandle functions.
  95.  
  96.       // The above means that the file mapping is still valid even after we
  97.       // close the handles for the file and the file mapping.
  98.     end;
  99.   end;
  100.  
  101.   if (FileHandle  <> INVALID_HANDLE_VALUE) then CloseHandle(FileHandle);
  102.   if (FileMapping <>                    0) then CloseHandle(FileMapping);
  103.  
  104.   result := p;
  105. end;
  106.  
The above function creates a scope and releases whatever resources were obtained once the scope is exited.  As long as the variables that hold resource handles/pointers are initialized, it will work fine.

That sequence of code is quite common and, I've seen it quite a few times (Jeffrey Richter's examples for instance) where, if something goes wrong, there are if statements that release whatever resources have been obtained so far but, that's a horrible way of doing it because as more resources are obtained, the number of resources released in every if statement increases.  That is bug prone, forget to release one and there is a leak.  Also, re-using the code is not straightforward because any needed modification may require all those if statements that release resources to be modified.

Using try/finally is very similar to using if statements.  The try/finally(ies) are nested to account for every resource obtained.

if(s) and try/finally are very poor solutions to releasing resources.  A little bit of structuring (with a scope) makes the function fully linear with a minimum amount of nesting (usually, 1 nesting level : the scope)
Title: Re: Exceptions
Post by: engkin on July 16, 2019, 02:38:13 pm
I like this example:

Code: Pascal  [Select]
  1. program test;
  2.  
  3. var
  4.   s: string;
  5. begin
  6.   s := '12345';
  7.   s := Copy(s, -1, MAXINT);
  8.   WriteLn(s);
  9. end.
  10.  
This is a fantastic example.

Of course the index and length of the Copy() are invalid, but you still get a result. While many different languages (and different Pascal implementations) would raise an exception, because the result would be incorrect. While strictly true, I like the graceful result.
Unless ignored, an exception does not necessarily mean ungraceful result.

That raises another question: does your logic keeps working with partial results? That's why you check your inputs.
I like the fact that this question was "raised". Checking your inputs is natural, with or without exceptions. The two are unrelated. You need to check your inputs no matter what.

About returning errors: In C, atoi("bla!") returns 0 (zero). That is bad, because you don't know if an error occurred. You could do: 'BOOL atoi("bla!", ReturnVar)', but that makes for messy code. 'int atoi("bla!", DefaultValue)' is better.
atoi has implicit DefaultValue of zero.

And otherwise do raise that exception, and catch it.
As you can see, sometimes it is a matter of style, but not always.
Title: Re: Exceptions
Post by: SymbolicFrank on July 16, 2019, 02:56:26 pm
atoi has implicit DefaultValue of zero.

Yes, but that is what prevents you from spotting errors in most cases. MININT would be a much better default. It depends on the strings you try to translate.
Title: Re: Exceptions
Post by: engkin on July 16, 2019, 04:03:39 pm
CPU exceptions included?
In user mode - ring 3 - most CPU exceptions are caused by bugs in the program. 
Did you say most?

An exception (pun intended ;)) are the debug exceptions and those are handled by the OS and routed to the debugger as events. 
I see exceptions on top of exceptions.

In a bug-free program that only concerns itself with resources it owns, there should not be any exceptions.
A bug-free program is a program that does nothing, whence it has no bugs. Joking aside, apart from small samples, programs are always dealing with resources they do not own, or depend on libraries that do use exceptions. Exceptions are just a tool. Like any tool, they need to be used right.

Can you elaborate a bit more here?
Sure.  The most common use of try/finally is to ensure that resources that have been allocated are de-allocated when no longer necessary.
While this usage is the common one, it is not the only reason. You can use try/finally to signal the end of a process, or change the status, or anything else that you want to make sure is done at the end of a try finally block.

If the variables that hold the handles and/or pointers to those resources are initialized before they are used then a try/finally is unnecessary.  For instance
Code: Pascal  [Select]
  1. function FileMap (Filename : pchar) : pbyte;
  2.   // maps into memory the file to dump
  3. var
  4.   FileHandle           : THANDLE = 0;
  5.   FileMapping          : THANDLE = 0;
  6.  
  7.   p                    : pbyte   = nil;
  8.  
  9.   SCOPE                : integer;
  10.  
  11. const
  12.   FUNCTION_ID          = 'B10: ';
  13.  
  14.   // constants used by CreateFile
  15.  
  16.   NO_TEMPLATE_FILE     =       0;
  17.  
  18.   // constants used by CreateFileMapping
  19.  
  20.   NO_MAXIMUM_SIZE_HIGH =       0;      // 0 indicates to use the size of the
  21.   NO_MAXIMUM_SIZE_LOW  =       0;      //   file
  22.  
  23.   // constants used by MapViewOfFileEx
  24.  
  25.   FILE_OFFSET_HIGH     =       0;      //   file offset to map from
  26.   FILE_OFFSET_LOW      =       0;
  27.  
  28.   BEGINNING_TO_END     =       0;
  29.  
  30. begin
  31.   result := nil;              // default return value
  32.  
  33.   if IsBadStringPtrA(Filename, STRING_MAX_LENGTH)                   then exit;
  34.  
  35.   for SCOPE := 1 to 1 do      // trick to create a scope one can break out of
  36.   begin
  37.     FileHandle := CreateFileA(Filename,
  38.                               GENERIC_READ,
  39.                               FILE_SHARE_READ,
  40.                               nil,
  41.                               OPEN_EXISTING,
  42.                               FILE_ATTRIBUTE_NORMAL,
  43.                               NO_TEMPLATE_FILE);
  44.  
  45.     if FileHandle = INVALID_HANDLE_VALUE then
  46.     begin
  47.       OutputToDeviceA(FUNCTION_ID,
  48.                       'unable to open ', Filename);
  49.       Newline();
  50.       break;
  51.     end;
  52.  
  53.     // with the file handle, create a mapping for it
  54.  
  55.     FileMapping := CreateFileMappingA(FileHandle,
  56.                                       nil,
  57.                                       PAGE_READONLY,
  58.                                       NO_MAXIMUM_SIZE_HIGH,  // use file size
  59.                                       NO_MAXIMUM_SIZE_LOW,
  60.                                       nil);
  61.     if (FileMapping = 0) then
  62.     begin
  63.       OutputToDeviceA(FUNCTION_ID, 'unable to map ', Filename);
  64.       Newline();
  65.       break;
  66.     end;
  67.  
  68.     p := MapViewOfFileEx(FileMapping,
  69.                          FILE_MAP_READ,
  70.                          FILE_OFFSET_HIGH,                   // from beginning
  71.                          FILE_OFFSET_LOW,
  72.                          BEGINNING_TO_END,                   // to end
  73.                          nil);                               // map anywhere
  74.  
  75.     if p = nil then
  76.     begin
  77.       OutputToDeviceA(FUNCTION_ID,
  78.                       'unable to map view of file', Filename);
  79.       Newline();
  80.       break;
  81.  
  82.       // the note below applies only when a non nil address is specified to
  83.       // map at a specific address
  84.  
  85.       // NOTE: MapViewOfFileEx fails if it cannot map at the specified address.
  86.       //       in other words, it will NOT attempt to map to a different address
  87.       //       to succeed.  Because of this there is no need to check if the
  88.       //       address returned is equal to the address specified.
  89.  
  90.       // The documentation states:
  91.  
  92.       // Reserved and committed pages are released when the view is unmapped and
  93.       // the file mapping object is closed. For details, see the UnmapViewOfFile
  94.       // and CloseHandle functions.
  95.  
  96.       // The above means that the file mapping is still valid even after we
  97.       // close the handles for the file and the file mapping.
  98.     end;
  99.   end;
  100.  
  101.   if (FileHandle  <> INVALID_HANDLE_VALUE) then CloseHandle(FileHandle);
  102.   if (FileMapping <>                    0) then CloseHandle(FileMapping);
  103.  
  104.   result := p;
  105. end;
  106.  
The above function creates a scope and releases whatever resources were obtained once the scope is exited.  As long as the variables that hold resource handles/pointers are initialized, it will work fine.

That sequence of code is quite common and, I've seen it quite a few times (Jeffrey Richter's examples for instance) where, if something goes wrong, there are if statements that release whatever resources have been obtained so far but, that's a horrible way of doing it because as more resources are obtained, the number of resources released in every if statement increases.  That is bug prone, forget to release one and there is a leak.  Also, re-using the code is not straightforward because any needed modification may require all those if statements that release resources to be modified.
Wait, this example is supposed to show how you do not need try/finally?
You used a for loop to create a scope so you can use break?
You want to use break but avoid try/finally?
You avoided using objects in your function. You do realize that you can achieve the same goal using objects, specially when this function get complicated, right?

Using try/finally is very similar to using if statements.  The try/finally(ies) are nested to account for every resource obtained.
You can replace one with the other, but they are not the same.

The try/finally(ies) are nested to account for every resource obtained.
Not necessarily.

if(s) and try/finally are very poor solutions to releasing resources.  A little bit of structuring (with a scope) makes the function fully linear with a minimum amount of nesting (usually, 1 nesting level : the scope)
Can you show how you do that with the function you showed above?
Why do you want to keep nesting at minimum?
Title: Re: Exceptions
Post by: engkin on July 16, 2019, 04:20:34 pm
atoi has implicit DefaultValue of zero.

Yes, but that is what prevents you from spotting errors in most cases. MININT would be a much better default. It depends on the strings you try to translate.
It is not there to help you spot errors, nor does is prevent you from spotting errors. It does what it is supposed to do: translate any string to number. Zero is the default. If you want to "spot" errors, use a different function.

Using the wrong tool and complaining it is not doing what you want is not right.
Title: Re: Exceptions
Post by: SymbolicFrank on July 16, 2019, 04:42:51 pm
atoi has implicit DefaultValue of zero.

Yes, but that is what prevents you from spotting errors in most cases. MININT would be a much better default. It depends on the strings you try to translate.
It is not there to help you spot errors, nor does is prevent you from spotting errors. It does what it is supposed to do: translate any string to number. Zero is the default. If you want to "spot" errors, use a different function.

Using the wrong tool and complaining it is not doing what you want is not right.

It was the example I used to explain the issue, and I did offer solutions. And yes, I did write my own function, because there were no functions that did what I wanted. Even more so: I have a hard time coming up with a use case where the default atoi function would make sense.
Title: Re: Exceptions
Post by: 440bx on July 16, 2019, 05:07:42 pm
Did you say most?
yes, I did.  Among the CPU exceptions are "invalid opcode" exceptions which can happen if there is a bug in the compiler one is using, not in the program.  Also, these days, programmers are raising exceptions because they buried themselves in a pile of function/procedure calls and the only way they have left to inform the original caller something went wrong is by raising an exception (extremely poor programming!.)

I see exceptions on top of exceptions.
I see lots of those in OOP programs.

apart from small samples, programs are always dealing with resources they do not own,
By resources they don't own, I mean resources whose existence they cannot predict.  For instance, if a program is attempting to read memory that is in another process (or the same process but, is not managed by the program), the program cannot make any assumptions as to the presence of memory at a particular address.  That said, that example with memory is not very good since using Read/WriteProcessMemory eliminates the need for an exception handler.  The basic rule is, when the proper operation of a set of instructions cannot be predicted then, exceptions are warranted, otherwise the programmer should write clean code, which means do all the necessary checking before carrying out the function.


or depend on libraries that do use exceptions.
That's defines the typical case: a poorly written library spreads its poor design to the program that wants to use it.  Apparently, it's too much work these days to return an indication of failure or success as a function result.  Using exceptions makes a programmer look "kewl" and knowledgeable.  Function results ?... way too prosaic these days.

Exceptions are just a tool. Like any tool, they need to be used right.
I agree with that.  These days they are grossly abused and misused.


While this usage is the common one, it is not the only reason. You can use try/finally to signal the end of a process, or change the status, or anything else that you want to make sure is done at the end of a try finally block.
You're right, it's not the only reason.  That was just an example and I picked something quite common for it.   I can write much simpler, easier to understand and follow code without using try/finally.

Wait, this example is supposed to show how you do not need try/finally?
That's one example. I'd much rather code it like that than use a try/finally construct.  It is a lot cleaner.  No nested try/finally and when debugging, the execution doesn't jump around into some finally(ies) if something didn't go as expected.

You used a for loop to create a scope so you can use break?
Yes, unfortunately.  There are no scopes in Pascal.   That is not as clean as I'd like it to be but, it's still way better than try/finally(ies.)

You want to use break but avoid try/finally?
That's only one of the reasons.  Another reason is that, that way, all the resource deallocation is in _one_ place, not scattered all over.   In more complex functions, that makes a very noticeable difference, particularly when debugging (the debugger isn't jumping around from one finally to another.)

You avoided using objects in your function. You do realize that you can achieve the same goal using objects, specially when this function get complicated, right?
I strongly believe that ANYTHING that can be written using objects can be written in a clearer, simpler, easier to maintain and more efficient way without objects.  If there is an exception to that, I am yet to see it.

That said, if you'd like to rewrite that function using objects and point out the reasons why yours is better than the one I posted, I would be very pleased to see it.

Using try/finally is very similar to using if statements.  The try/finally(ies) are nested to account for every resource obtained.
You can replace one with the other, but they are not the same.
That's true but, as a programming method, they have some flaws in common.

The try/finally(ies) are nested to account for every resource obtained.
Not necessarily.
True but, quite often, they are.

if(s) and try/finally are very poor solutions to releasing resources.  A little bit of structuring (with a scope) makes the function fully linear with a minimum amount of nesting (usually, 1 nesting level : the scope)
Can you show how you do that with the function you showed above?
Why do you want to keep nesting at minimum?
The function already uses a scope (unfortunately, with trickery since there is no support for it in the language.)  The reason I keep nesting at a minimum is because the lower the level of nesting the easier a construction is to understand.
Title: Re: Exceptions
Post by: engkin on July 16, 2019, 07:05:39 pm
Among the CPU exceptions are "invalid opcode" exceptions which can happen if there is a bug in the compiler one is using, not in the program.
Or using the program on a different CPU that lacks the "invalid opcode"?

  Also, these days, programmers are raising exceptions because they buried themselves in a pile of function/procedure calls and the only way they have left to inform the original caller something went wrong is by raising an exception (extremely poor programming!.)
Keeping all the code in one function and returning false or any value to indicate the exact error is not much different. Taking either extreme is simply poor programming practice, and does not help readability nor prevents bugs.

Using enough functions that return error codes, or objects that raise exceptions carrying error codes and messages, both are simply two styles of programming. You can abuse either one.

I see exceptions on top of exceptions.
I see lots of those in OOP programs.
You should, it is just a style of programming.

apart from small samples, programs are always dealing with resources they do not own,
By resources they don't own, I mean resources whose existence they cannot predict.  For instance, if a program is attempting to read memory that is in another process (or the same process but, is not managed by the program), the program cannot make any assumptions as to the presence of memory at a particular address.  That said, that example with memory is not very good since using Read/WriteProcessMemory eliminates the need for an exception handler.  The basic rule is, when the proper operation of a set of instructions cannot be predicted then, exceptions are warranted,
It is hard to imagine an applications that does not deal with the internet, json, networks, databases, acquired data from wired/wireless sources like images or ... etc.

otherwise the programmer should write clean code, which means do all the necessary checking before carrying out the function.
This applies when using OOP or not.

or depend on libraries that do use exceptions.
That's defines the typical case: a poorly written library spreads its poor design to the program that wants to use it.  Apparently, it's too much work these days to return an indication of failure or success as a function result.  Using exceptions makes a programmer look "kewl" and knowledgeable.  Function results ?... way too prosaic these days.
I hear you, you prefer to use a library and with each call to check the result: Initialize the library, check. A call, a check. Another call, another check, repeat... , finalize the library, then check.

You don't like to include all of that in one block and leave the checking to the library itself? It would keep your code focused on its own part. But again, this sounds like your own style of programming, I have nothing against it. I can imagine how if thisVar=someErrorValue then grows in your code.

I want to let you know that JAVA does have exceptions and you have to catch them, and, unfortunately, FPC can produce JAVA classes, run on JVMs, and interact with other JAVA classes.

Exceptions are just a tool. Like any tool, they need to be used right.
I agree with that.  These days they are grossly abused and misused.
Anything could be abused of misused, does not mean we need to avoid it, or consider it poor practice.

While this usage is the common one, it is not the only reason. You can use try/finally to signal the end of a process, or change the status, or anything else that you want to make sure is done at the end of a try finally block.
You're right, it's not the only reason.  That was just an example and I picked something quite common for it.   I can write much simpler, easier to understand and follow code without using try/finally.
Yes, until you are forced.

Wait, this example is supposed to show how you do not need try/finally?
That's one example. I'd much rather code it like that than use a try/finally construct.  It is a lot cleaner.  No nested try/finally and when debugging, the execution doesn't jump around into some finally(ies) if something didn't go as expected.
The debugger, when enough debugging data is available, is supposed to show you exactly where the exception had happened before it takes you anywhere else. But if you don't like try/finally for that reason, then you do not like else either because it jump somewhere far from the line the debugger was on.

You used a for loop to create a scope so you can use break?
Yes, unfortunately.  There are no scopes in Pascal.   That is not as clean as I'd like it to be but, it's still way better than try/finally(ies.)
It is not clean, and not supported by Pascal, but you used it. While Try/Finally is part of the language, the object oriented dialect, and you refuse to use it?

You want to use break but avoid try/finally?
That's only one of the reasons.  Another reason is that, that way, all the resource deallocation is in _one_ place, not scattered all over.   In more complex functions, that makes a very noticeable difference, particularly when debugging (the debugger isn't jumping around from one finally to another.)
"Scattered" or not, if deallocation happens in a logical and correct way, and always guaranteed there is no problem. The desire to have all the deallocations in one place came with extra price, at least, the chain of IF THEN. Again, a style of programming.

You avoided using objects in your function. You do realize that you can achieve the same goal using objects, specially when this function get complicated, right?
I strongly believe that ANYTHING that can be written using objects can be written in a clearer, simpler, easier to maintain and more efficient way without objects.  If there is an exception to that, I am yet to see it.
Yes, a call to member function of an object like AStringList.Add is actually a call to a function with the first parameter is the object itself: Add(AStringList,... see?
It is like adjectives are before/after the nouns in different languages:
English (Brown Horse)
Spanish (Horse Brown)
Which one do you prefer?

That said, if you'd like to rewrite that function using objects and point out the reasons why yours is better than the one I posted, I would be very pleased to see it.
I might give it a shot, not sure.

if(s) and try/finally are very poor solutions to releasing resources.  A little bit of structuring (with a scope) makes the function fully linear with a minimum amount of nesting (usually, 1 nesting level : the scope)
Can you show how you do that with the function you showed above?
Why do you want to keep nesting at minimum?
The function already uses a scope (unfortunately, with trickery since there is no support for it in the language.)  The reason I keep nesting at a minimum is because the lower the level of nesting the easier a construction is to understand.
When the level of nesting grows, it is time to consider another function/member function.

Now I believe that you have your own style of programming and refuse to change it, not because it protects you from bugs, but simply because you do not want to try something new. Nothing unusual here, but considering other ways as poor or more prone to bugs is not accurate.
Title: Re: Exceptions
Post by: Handoko on July 16, 2019, 08:07:48 pm
For instance
Code: Pascal  [Select]
  1. function FileMap (Filename : pchar) : pbyte;
  2.   // maps into memory the file to dump
  3. var
  4.   FileHandle           : THANDLE = 0;
  5.   FileMapping          : THANDLE = 0;
  6.  
  7.   p                    : pbyte   = nil;
  8.  
  9.   SCOPE                : integer;
  10.  
  11. const
  12.   FUNCTION_ID          = 'B10: ';
  13.  
  14.   // constants used by CreateFile
  15.  
  16.   NO_TEMPLATE_FILE     =       0;
  17.  
  18.   // constants used by CreateFileMapping
  19.  
  20.   NO_MAXIMUM_SIZE_HIGH =       0;      // 0 indicates to use the size of the
  21.   NO_MAXIMUM_SIZE_LOW  =       0;      //   file
  22.  
  23.   // constants used by MapViewOfFileEx
  24.  
  25.   FILE_OFFSET_HIGH     =       0;      //   file offset to map from
  26.   FILE_OFFSET_LOW      =       0;
  27.  
  28.   BEGINNING_TO_END     =       0;
  29.  
  30. begin
  31.   result := nil;              // default return value
  32.  
  33.   if IsBadStringPtrA(Filename, STRING_MAX_LENGTH)                   then exit;
  34.  
  35.   for SCOPE := 1 to 1 do      // trick to create a scope one can break out of
  36.   begin
  37.     FileHandle := CreateFileA(Filename,
  38.                               GENERIC_READ,
  39.                               FILE_SHARE_READ,
  40.                               nil,
  41.                               OPEN_EXISTING,
  42.                               FILE_ATTRIBUTE_NORMAL,
  43.                               NO_TEMPLATE_FILE);
  44.  
  45.     if FileHandle = INVALID_HANDLE_VALUE then
  46.     begin
  47.       OutputToDeviceA(FUNCTION_ID,
  48.                       'unable to open ', Filename);
  49.       Newline();
  50.       break;
  51.     end;
  52.  
  53.     // with the file handle, create a mapping for it
  54.  
  55.     FileMapping := CreateFileMappingA(FileHandle,
  56.                                       nil,
  57.                                       PAGE_READONLY,
  58.                                       NO_MAXIMUM_SIZE_HIGH,  // use file size
  59.                                       NO_MAXIMUM_SIZE_LOW,
  60.                                       nil);
  61.     if (FileMapping = 0) then
  62.     begin
  63.       OutputToDeviceA(FUNCTION_ID, 'unable to map ', Filename);
  64.       Newline();
  65.       break;
  66.     end;
  67.  
  68.     p := MapViewOfFileEx(FileMapping,
  69.                          FILE_MAP_READ,
  70.                          FILE_OFFSET_HIGH,                   // from beginning
  71.                          FILE_OFFSET_LOW,
  72.                          BEGINNING_TO_END,                   // to end
  73.                          nil);                               // map anywhere
  74.  
  75.     if p = nil then
  76.     begin
  77.       OutputToDeviceA(FUNCTION_ID,
  78.                       'unable to map view of file', Filename);
  79.       Newline();
  80.       break;
  81.  
  82.       // the note below applies only when a non nil address is specified to
  83.       // map at a specific address
  84.  
  85.       // NOTE: MapViewOfFileEx fails if it cannot map at the specified address.
  86.       //       in other words, it will NOT attempt to map to a different address
  87.       //       to succeed.  Because of this there is no need to check if the
  88.       //       address returned is equal to the address specified.
  89.  
  90.       // The documentation states:
  91.  
  92.       // Reserved and committed pages are released when the view is unmapped and
  93.       // the file mapping object is closed. For details, see the UnmapViewOfFile
  94.       // and CloseHandle functions.
  95.  
  96.       // The above means that the file mapping is still valid even after we
  97.       // close the handles for the file and the file mapping.
  98.     end;
  99.   end;
  100.  
  101.   if (FileHandle  <> INVALID_HANDLE_VALUE) then CloseHandle(FileHandle);
  102.   if (FileMapping <>                    0) then CloseHandle(FileMapping);
  103.  
  104.   result := p;
  105. end;
  106.  

You used a for loop to create a scope so you can use break?
Yes, unfortunately.  There are no scopes in Pascal.   That is not as clean as I'd like it to be but, it's still way better than try/finally(ies.)

I never thought to use a 'fake' loop to create a scope. Interesting trick, I learn a new trick today. Thank you.
Title: Re: Exceptions
Post by: julkas on July 16, 2019, 08:14:28 pm
Assume you writing cotainer library. Library has function minmax(x) which returns largest value not greater than x. User makes call minmax(10) and container has only one element 12. What returns minmax?
Title: Re: Exceptions
Post by: SymbolicFrank on July 16, 2019, 08:29:00 pm
Assume you writing cotainer library. Library has function minmax(x) which returns largest value not greater than x. User makes call minmax(10) and container has only one element 12. What returns minmax?

That depends on the allowed ranges and types of objects in the container. If they're all positive integers, return a negative one. If they're objects, return nil. If they're just Integer[MININT..MAXINT], raise an exception. But I would try to use exceptions only for cases where other solutions are hard, so it would make more sense to change the library function to one that allows returning errors.
Title: Re: Exceptions
Post by: 440bx on July 16, 2019, 09:15:30 pm
Or using the program on a different CPU that lacks the "invalid opcode"?
I'm not sure what you're getting at with that.  If a program is functioning correctly and, it is not dealing with resources managed by code that is not its own, there should not be any exceptions.

  Also, these days, programmers are raising exceptions because they buried themselves in a pile of function/procedure calls and the only way they have left to inform the original caller something went wrong is by raising an exception (extremely poor programming!.)
Keeping all the code in one function and returning false or any value to indicate the exact error is not much different.
It is very different and, I believe you know that.  An exception is a goto across stack frames.  The worst kind of goto there is and, to make exceptions even worse, the final destination can very often only be determined at runtime.  When a function returns a value to indicate something went wrong, it unwinds the stack in an orderly fashion (no cross stack gotos there), the final destination can always be determined by simply reading a listing of the code.

And there is another "little" difference. I attached a drastically abbreviated PE dump of a DLL that uses exceptions instead of functions that return a value.   When using functions that return a value, the program doesn't get saddled with that pile of junk because it is not doing cross-frame gotos which is what makes the exception directory/table necessary.

The PE dump is of a DLL most people have on their computer, it's Chrome_Child.dll.


Taking either extreme is simply poor programming practice, and does not help readability nor prevents bugs.
I don't consider a function returning a value that indicates something went wrong as an extreme, I consider it clean programming, unlike cross stack/scope gotos currently baptized as "exceptions" to make their use sound "kewl".

Using enough functions that return error codes, or objects that raise exceptions carrying error codes and messages, both are simply two styles of programming. You can abuse either one.
Yes, spaghetti code is also a programming style, that doesn't make it good.   I don't recall seeing a program where I would say the programmer abused functions returning error codes to the caller.  I have seen plenty of OOP code that misuse and abuse the use of exceptions.


You should, it is just a style of programming.
just like spaghetti code is a "programming style" too.


It is hard to imagine an applications that does not deal with the internet, json, networks, databases, acquired data from wired/wireless sources like images or ... etc.
That's true but, I've dealt with the majority of the things in your list and I have not needed exceptions.   

otherwise the programmer should write clean code, which means do all the necessary checking before carrying out the function.
This applies when using OOP or not.
Yes but, OOP programmers seldom do it that way. What's seen more often than not is, code that doesn't check for anything and simply wraps statements in exception handlers.  When something goes wrong, the exception handler gets to have a look at what happened, if it is clueless as to how to fix the problem, it simply re-raises the exception passing the buck to who knows what code.



or depend on libraries that do use exceptions.
That's defines the typical case: a poorly written library spreads its poor design to the program that wants to use it.  Apparently, it's too much work these days to return an indication of failure or success as a function result.  Using exceptions makes a programmer look "kewl" and knowledgeable.  Function results ?... way too prosaic these days.
I hear you, you prefer to use a library and with each call to check the result: Initialize the library, check. A call, a check. Another call, another check, repeat... , finalize the library, then check.
Yes, I definitely prefer that because when something goes wrong, the calling function/procedure is the one that gets to know about the problem (that may not be the case when using exceptions) and the caller can figure out what the proper course of action to take is, not some exception handler who knows where which may not even have been programmed with the proper way of handling a problem from the code that raised the exception.

I will give you this, there are times when I have found all the checking to be tedious but, I'd much rather do the tedious work than writing a program that is just 21st century spaghetti code covered in exception cheese.

You don't like to include all of that in one block and leave the checking to the library itself? It would keep your code focused on its own part. But again, this sounds like your own style of programming, I have nothing against it. I can imagine how if thisVar=someErrorValue then grows in your code.
That's not quite it.  It's usually, "if functionreturn = false then <either handle the problem or exit if it cannot or should not>"

Not checking for function results is another one of those bad habits coming from C.   If it's a function, it's return value should be checked, that's why it's a function and not a procedure.

For the record, I don't have anything against anyone's programming style but, I do have something against OOP.  I find the results usually leave a great deal to be desired and, the really unfortunate part is that truly deficient programming practices have been coated in impressive sounding names to make them "attractive".  Cross scope goto (formerly known as longjmp) have been renamed "exceptions" and to make them even "nicer" the destination, unlike in the traditional goto, is dynamic.  Welcome back to COBOL's ALTER statement.  A statement programmer's got fired for using, now that it goes by the name of "exception", it is "kewl".



I want to let you know that JAVA does have exceptions and you have to catch them, and, unfortunately, FPC can produce JAVA classes, run on JVMs, and interact with other JAVA classes.
Fortunately, Java isn't and is rather unlikely to ever be one of my concerns.

Exceptions are just a tool. Like any tool, they need to be used right.
I agree with that.  These days they are grossly abused and misused.
Anything could be abused of misused, does not mean we need to avoid it, or consider it poor practice.
In generic terms, as you've stated them above, I fully agree with that.  That said, my point is that the entire exception mechanism is very poorly implemented, grossly misused and equally abused. 



Yes, until you are forced.
I can see being forced to have exception handlers (as you pointed out if the program is using a library that raises exceptions, that leaves no choice) but, I don't see how a programmer could be forced to use try/finally.


The debugger, when enough debugging data is available, is supposed to show you exactly where the exception had happened before it takes you anywhere else. But if you don't like try/finally for that reason, then you do not like else either because it jump somewhere far from the line the debugger was on.
True that the debugger will usually be able to show exactly where the exception took place.  No argument there.  That said, when resources need to be released, the code execution is going to jump around to each and every one of the try/finally(ies)  not so in the case of an if statement, it  will execute whatever is in the if, that's it, no jumping around.


It is not clean, and not supported by Pascal, but you used it. While Try/Finally is part of the language, the object oriented dialect, and you refuse to use it?
I openly admit that I'm not fond of having to "trick" the compiler in order to get a scope.  As far as supported by Pascal, I do believe that "for i := 1 to 1 do " is supported.   The one problem in that construct (which I wish I didn't have to use) is that if the optimizer ever gets really smart, it will remove the loop and then complain that there is a loose "break" statement. That is a potential problem.


"Scattered" or not, if deallocation happens in a logical and correct way, and always guaranteed there is no problem. The desire to have all the deallocations in one place came with extra price, at least, the chain of IF THEN. Again, a style of programming.
Scattered is a problem because the programmer has the burden of mentally assembling all the little "pieces".  Definitely less than desirable.

Yes, a call to member function of an object like AStringList.Add is actually a call to a function with the first parameter is the object itself: Add(AStringList,... see?
It is like adjectives are before/after the nouns in different languages:
English (Brown Horse)
Spanish (Horse Brown)
Which one do you prefer?
No, that example doesn't reflect the problems with OOP.  It isn't simply a matter of syntax.  Here is a question for you, why do you think it is customary/standard to prefix object fields with some letter, usually the letter F ?.  It isn't to make them look pretty.


I might give it a shot, not sure.
I offered only so we could compare specifics instead of staying in the realm of "theory".


When the level of nesting grows, it is time to consider another function/member function.
That's not a good solution either because it breaks the logical flow into a bunch of little pieces and the programmer has to go find them to read them, then mentally assemble all that stuff to see what the complete logic is.  A programmer should be designing algorithms and data structures not assembling binary jigsaw puzzles.

Now I believe that you have your own style of programming and refuse to change it, not because it protects you from bugs, but simply because you do not want to try something new. Nothing unusual here, but considering other ways as poor or more prone to bugs is not accurate.
That is not the case.  My programming style changed a lot over the years (Per Brinch Hansen is responsible for the greatest change).  Whenever I see a better way of doing something (which I define as something that simplifies the code or helps prevent potential bugs) I take it on the spot.    OOP is definitely not one of those things.


This whole thing started with "exceptions" and whether people use them or not.  My premise is that they can be, on rare occasions, extremely useful - usually when writing code that sticks its bytes where they probably don't belong - but, for normal programs that are not fiddling with resources they don't manage, the use of exceptions is not justified.


Just in case, I want to make sure the following is clear, this is a completely friendly discussion.  I know that I tend to support my viewpoints a little bit more "vigorously" than is thought of as friendly. I mention it because I don't want it to be misinterpreted.


ETA:

@Handoko

You're welcome!.



ETA 2:

Added missing attachment.
Title: Re: Exceptions
Post by: engkin on July 16, 2019, 10:26:29 pm
I might give it a shot, not sure.
I offered only so we could compare specifics instead of staying in the realm of "theory".
I have counter argument for every argument you made, but I think it is time to see a more practical example. Here is your function in one way:
Code: Pascal  [Select]
  1. unit uFileMapper;
  2.  
  3. {$mode objfpc}{$H+}
  4.  
  5. interface
  6.  
  7. uses
  8.   Classes, SysUtils;
  9.  
  10. Type
  11.   EFileMapperError = class(Exception);
  12.  
  13.   { TFileMapper }
  14.  
  15.   TFileMapper = class(TFileStream)
  16.   public
  17.     constructor Create(AFileName: String);
  18.     function MapFile(AOffset: QWord = 0): pointer;  {AOffset from beginning of file}
  19.   end;
  20.  
  21. implementation
  22. uses
  23.   Windows;
  24.  
  25.   { TFileMapper }
  26.  
  27.   constructor TFileMapper.Create(AFileName: String);
  28.   begin
  29.     Inherited Create(AFileName, fmOpenRead or fmShareDenyWrite);
  30.   end;
  31.  
  32.   function TFileMapper.MapFile(AOffset: QWord): pointer;
  33.   const
  34.     // constants used by CreateFileMapping
  35.  
  36.     NO_MAXIMUM_SIZE_HIGH =       0;      // 0 indicates to use the size of the
  37.     NO_MAXIMUM_SIZE_LOW  =       0;      //   file
  38.  
  39.     // constants used by MapViewOfFileEx
  40.  
  41.     BEGINNING_TO_END     =       0;
  42.   var
  43.     FileMapping: HANDLE;
  44.  
  45.   begin
  46.     Result := nil;
  47.  
  48.     // with the file handle, create a mapping for it
  49.     FileMapping := CreateFileMappingA(Handle,
  50.                                       nil,
  51.                                       PAGE_READONLY,
  52.                                       NO_MAXIMUM_SIZE_HIGH,  // use file size
  53.                                       NO_MAXIMUM_SIZE_LOW,
  54.                                       nil);
  55.     if FileMapping = 0 then
  56.       raise EFileMapperError.createfmt('File: %s, Line: %s, Unable to map file: %s', [{$I %File%},{$I %Line%},FileName]);
  57.  
  58.     Result := MapViewOfFileEx(FileMapping,
  59.                          FILE_MAP_READ,
  60.                          HI(AOffset),                        // from beginning
  61.                          LO(AOffset),
  62.                          BEGINNING_TO_END,                   // to end
  63.                          nil);                               // map anywhere
  64.  
  65.     CloseHandle(FileMapping);
  66.  
  67.     if Result = nil then
  68.       raise EFileMapperError.createfmt('File: %s, Line: %s, Unable to map view of file: %s', [{$I %File%},{$I %Line%},Filename]);
  69.   end;
  70.  
  71. end.

What do you think?
Title: Re: Exceptions
Post by: 440bx on July 16, 2019, 11:35:52 pm
I have counter argument for every argument you made, but I think it is time to see a more practical example. Here is your function in one way:
I understand and, I very much appreciate discussing specifics instead of theory.

Code: Pascal  [Select]
  1. unit uFileMapper;
  2.  
  3. {$mode objfpc}{$H+}
  4.  
  5. interface
  6.  
  7. uses
  8.   Classes, SysUtils;
  9.  
  10. Type
  11.   EFileMapperError = class(Exception);    // piece #1
  12.  
  13.   { TFileMapper }
  14.  
  15.   TFileMapper = class(TFileStream)       // piece #2 - I suppose this is where the file is opened.  
  16.                                          // there is likely memory allocated to implement it. That was not necessary before.
  17.   public
  18.     constructor Create(AFileName: String);  // piece #3 - didn't need that before.  Again, it probably allocates memory  
  19.                                             // which doesn't seem to be freed anywhere.
  20.  
  21.    // not comparing, just an observation.  The routine I posted didn't give the option to map at an offset.
  22.    // your implementation is going beyond the call of duty... which is perfectly fine.
  23.  
  24.     function MapFile(AOffset: QWord = 0): pointer;  {AOffset from beginning of file}  // piece #4
  25.   end;
  26.  
  27.   // I presume the program/caller will have to create an instance of TFileMapper.  I am under the impression that would
  28.   // be a global variable (that would not be desirable and it was not needed in the non-OOP function)
  29.  
  30. implementation
  31. uses
  32.   Windows;
  33.  
  34.   { TFileMapper }
  35.  
  36.   // vvvv  this wasn't necessary before.  There was no need to call an ancestor to do a CreateFile.
  37.  
  38.   constructor TFileMapper.Create(AFileName: String);
  39.   begin
  40.     Inherited Create(AFileName, fmOpenRead or fmShareDenyWrite);
  41.   end;
  42.  
  43.   function TFileMapper.MapFile(AOffset: QWord): pointer;
  44.   const
  45.     // constants used by CreateFileMapping
  46.  
  47.     NO_MAXIMUM_SIZE_HIGH =       0;      // 0 indicates to use the size of the
  48.     NO_MAXIMUM_SIZE_LOW  =       0;      //   file
  49.  
  50.     // constants used by MapViewOfFileEx
  51.  
  52.     BEGINNING_TO_END     =       0;
  53.   var
  54.     FileMapping: HANDLE;
  55.  
  56.   begin
  57.     Result := nil;
  58.  
  59.     // with the file handle, create a mapping for it
  60.     FileMapping := CreateFileMappingA(Handle,
  61.                                       nil,
  62.                                       PAGE_READONLY,
  63.                                       NO_MAXIMUM_SIZE_HIGH,  // use file size
  64.                                       NO_MAXIMUM_SIZE_LOW,
  65.                                       nil);
  66.     if FileMapping = 0 then
  67.       raise EFileMapperError.createfmt('File: %s, Line: %s, Unable to map file: %s', [{$I %File%},{$I %Line%},FileName]);
  68.  
  69.     Result := MapViewOfFileEx(FileMapping,
  70.                          FILE_MAP_READ,
  71.                          HI(AOffset),                        // from beginning
  72.                          LO(AOffset),
  73.                          BEGINNING_TO_END,                   // to end
  74.                          nil);                               // map anywhere
  75.  
  76.     CloseHandle(FileMapping);
  77.  
  78.     if Result = nil then
  79.       raise EFileMapperError.createfmt('File: %s, Line: %s, Unable to map view of file: %s', [{$I %File%},{$I %Line%},Filename]);
  80.   end;
  81.  
  82. end.

What do you think?
I added a few comments in the code but, in addition to those comments...

1. What used to be in one function is now broken into four (4) pieces.  I'll consider it three (3) because one of the pieces is to report error messages which I'll match to the function I had to do the same.

2. Now the opening of the file is no longer in the same function where the file is mapped.  It's not "atomic" anymore.

3. Instantiating the classes will result in memory being allocated and, there doesn't seem to be any code to free that memory.  Somewhere, the code to free that memory needs to be coded.  IOW, the equivalent implementation takes more code than what you presented.

4. if the MapFile method is successful, it doesn't close the file handle (likely because it's part of TFileStream) in spite of the fact that the file handle is no longer needed.  IOW, the program is carrying useless baggage for no good reason.

5. In summary, what used to be nicely packaged in one function is now scattered among 2 methods and, an ancestor.  Additionally, there are things that could be done if OOP had not be used (such as freeing the file handle) that are now "pending" (in this case, only 1.)  That leaves room for bugs, don't want to forget those pending things.  And, in addition to all that, there are unnecessary memory blocks allocated which will eventually need to be freed.   In the non-OOP solution, the entire function is in one "container", there are no memory blocks allocated since they are completely unnecessary, anything (handles) that are no longer needed are closed in the same function where they were obtained thus preserving parallelism.

Objectively, for all the reasons mentioned above, I consider the non-OOP version, vastly superior.














Title: Re: Exceptions
Post by: engkin on July 17, 2019, 12:32:12 am
In OOP if you create an object, you are responsible of freeing it. Similar to dealing with pair of functions Open/Close files. Or in our example: CreateFileMappingA then later CloseHandle.

The code I showed above does not leak memory, you can try it. It is dealing specifically with mapping the file, not with opening the file. Opening the file is left to the parent class: TFileStream. I don't need to repeat that code, and as an extra bonus, that part related to opening the file is going to work on any major platform FPC runs on: Windows, Linux, and Mac. If I were to port this class to, say, Linux, I just need to worry about MapFile function.

Is your code multi-platform?

Quote
  TFileMapper = class(TFileStream)       // piece #2 - I suppose this is where the file is opened.
No, this is to give our class all the abilities of TFileStream, and to build on top of it.

Quote
    constructor Create(AFileName: String);  // piece #3 - didn't need that before.  Again, it probably allocates memory
                                            // which doesn't seem to be freed anywhere.
This part is not necessary in our example, and it does not allocate any memory that needs to be freed.

Quote
   // not comparing, just an observation.  The routine I posted didn't give the option to map at an offset.
   // your implementation is going beyond the call of duty... which is perfectly fine.

    function MapFile(AOffset: QWord = 0): pointer;  {AOffset from beginning of file}  // piece #4
Because I can focus on mapping itself and different possibilities. Like you can pass the offset if you want, and if you do not it takes 0 by default.

Quote
  // I presume the program/caller will have to create an instance of TFileMapper.  I am under the impression that would
  // be a global variable (that would not be desirable and it was not needed in the non-OOP function)
It does not need to be global variable, it is up to you. In fact you can create , use, and free this class without a variable at all!

Quote
  // vvvv  this wasn't necessary before.  There was no need to call an ancestor to do a CreateFile.

  constructor TFileMapper.Create(AFileName: String);
  begin
    Inherited Create(AFileName, fmOpenRead or fmShareDenyWrite);
  end;
As you can see, it calls the parent class with fmOpenRead or fmShareDenyWrite as a second parameter so you do not have to pass them  when you use this class, as it is not relevant for the user of this class to bother with this specific detail.

To show how easy it is to maintain it, here is a reduced one based on your feedback:
Code: Pascal  [Select]
  1. unit uFileMapper_2;
  2.  
  3. {$mode objfpc}{$H+}
  4.  
  5. interface
  6.  
  7. uses
  8.   Classes, SysUtils;
  9.  
  10. Type
  11.   { TFileMapper }
  12.   TFileMapper = class(TFileStream)       // It does not allocate memory here.
  13.   public
  14.     function MapFile(AOffset: QWord = 0): pointer;  {AOffset from beginning of file}
  15.   end;
  16.  
  17. implementation
  18. uses
  19.   Windows;
  20.  
  21.   { TFileMapper }
  22.  
  23.   function TFileMapper.MapFile(AOffset: QWord): pointer;
  24.   const
  25.     // constants used by CreateFileMapping
  26.  
  27.     NO_MAXIMUM_SIZE_HIGH =       0;      // 0 indicates to use the size of the
  28.     NO_MAXIMUM_SIZE_LOW  =       0;      //   file
  29.  
  30.     // constants used by MapViewOfFileEx
  31.  
  32.     BEGINNING_TO_END     =       0;
  33.   var
  34.     FileMapping: HANDLE;
  35.  
  36.   begin
  37.     Result := nil;
  38.  
  39.     // with the file handle, create a mapping for it
  40.     FileMapping := CreateFileMappingA(Handle,
  41.                                       nil,
  42.                                       PAGE_READONLY,
  43.                                       NO_MAXIMUM_SIZE_HIGH,  // use file size
  44.                                       NO_MAXIMUM_SIZE_LOW,
  45.                                       nil);
  46.     if FileMapping = 0 then
  47.       raise Exception.createfmt('File: %s, Line: %s, Unable to map file: %s', [{$I %File%},{$I %Line%},FileName]);
  48.  
  49.     Result := MapViewOfFileEx(FileMapping,
  50.                          FILE_MAP_READ,
  51.                          HI(AOffset),                        // from beginning
  52.                          LO(AOffset),
  53.                          BEGINNING_TO_END,                   // to end
  54.                          nil);                               // map anywhere
  55.  
  56.     CloseHandle(FileMapping);
  57.  
  58.     if Result = nil then
  59.       raise Exception.createfmt('File: %s, Line: %s, Unable to map view of file: %s', [{$I %File%},{$I %Line%},Filename]);
  60.   end;
  61.  
  62. end.

Your comments are welcome.
Title: Re: Exceptions
Post by: 440bx on July 17, 2019, 01:36:22 am
Your comments are welcome.
Thank you. Here they are :)


In OOP if you create an object, you are responsible of freeing it. Similar to dealing with pair of functions Open/Close files. Or in our example: CreateFileMappingA then later CloseHandle.
The non-OOP function didn't need to allocate any memory (creating an object, allocates memory.)  Your usage of objects has imposed unnecessary overhead (create/use/track/free) on the program.

The code I showed above does not leak memory, you can try it.
It won't leak memory as long as the object is freed but, the freeing of the object is conspicuously missing as is its creation.  The non-OOP version didn't need to, directly or indirectly, allocate memory and didn't need code someplace else to free an unnecessary memory block (the object/class itself.)

It is dealing specifically with mapping the file, not with opening the file. Opening the file is left to the parent class: TFileStream.
And that is one of the problems.  In the non-OOP version, everything is right there in the function, makes it really easy to understand.  The programmer doesn't have to know/guess which of the ancestors opened the file nor wonder where the file handle is coming from.

I don't need to repeat that code,
It's not about repetition.  The call to CreateFile occurred in only one place in the non-OOP version.  The same place where the file mapping and the view are created.  All nice and neat in one function.   No need to go looking for stuff someplace else and wonder why the File Mapping is closed/freed but the File Handle isn't.  Unlike the OOP version, symmetrical and parallel as it should be.

and as an extra bonus, that part related to opening the file is going to work on any major platform FPC runs on: Windows, Linux, and Mac. If I were to port this class to, say, Linux, I just need to worry about MapFile function.
That's not a result of OOP programming, it's a result of the RTL.  You're giving OOP credit it did not earn.

Is your code multi-platform?
No and, it doesn't have to be.  The program (not just the function) only applies to Windows therefore, portability in this case is not applicable and, even if it were, it is no thanks to OOP but, to the RTL functions regardless of the paradigm used to implement them.

Quote
  TFileMapper = class(TFileStream)       // piece #2 - I suppose this is where the file is opened.
No, this is to give our class all the abilities of TFileStream, and to build on top of it.
The file doesn't get opened magically. Based on the classes and methods you've presented, it gives the impression that the inherited create method (from TFileStream) opens the file and, unnecessarily keeps the file handle opened.

Quote
    constructor Create(AFileName: String);  // piece #3 - didn't need that before.  Again, it probably allocates memory
                                            // which doesn't seem to be freed anywhere.
This part is not necessary in our example, and it does not allocate any memory that needs to be freed.
It's a fact that, somewhere memory is being allocated to hold the object and, that memory block has to be eventually freed.  That's another thing your program needs to keep track of that the non-OOP version doesn't have to do.

Quote
   // not comparing, just an observation.  The routine I posted didn't give the option to map at an offset.
   // your implementation is going beyond the call of duty... which is perfectly fine.

    function MapFile(AOffset: QWord = 0): pointer;  {AOffset from beginning of file}  // piece #4
Because I can focus on mapping itself and different possibilities. Like you can pass the offset if you want, and if you do not it takes 0 by default.
Nothing there that cannot be done just as simply without OOP.

Quote
  // I presume the program/caller will have to create an instance of TFileMapper.  I am under the impression that would
  // be a global variable (that would not be desirable and it was not needed in the non-OOP function)
It does not need to be global variable, it is up to you. In fact you can create , use, and free this class without a variable at all!
I'll give you that it doesn't have to be a global variable but, I find your claim that the class can be created, used and freed without using a variable, "dubious".  You got to have a way to refer to the class and, that's usually a variable (specifically, a pointer to the class - though the fact that it is a pointer is hidden by the compiler.)


Quote
  // vvvv  this wasn't necessary before.  There was no need to call an ancestor to do a CreateFile.

  constructor TFileMapper.Create(AFileName: String);
  begin
    Inherited Create(AFileName, fmOpenRead or fmShareDenyWrite);
  end;
As you can see, it calls the parent class with fmOpenRead or fmShareDenyWrite as a second parameter so you do not have to pass them  when you use this class, as it is not relevant for the user of this class to bother with this specific detail.
The non-OOP version didn't even need to pass those flags and, it didn't need to call a "parent" to execute one API call.  That's what OOP does, break things into tiny pieces, then the programmer has to reassemble all those pieces to figure out what the purpose of all of them put together is. 


To show how easy it is to maintain it, here is a reduced one based on your feedback:
Code: Pascal  [Select]
  1. unit uFileMapper_2;
  2.  
  3. {$mode objfpc}{$H+}
  4.  
  5. interface
  6.  
  7. uses
  8.   Classes, SysUtils;
  9.  
  10. Type
  11.   { TFileMapper }
  12.   TFileMapper = class(TFileStream)       // It does not allocate memory here.
  13.   public
  14.     function MapFile(AOffset: QWord = 0): pointer;  {AOffset from beginning of file}
  15.   end;
  16.  
  17. implementation
  18. uses
  19.   Windows;
  20.  
  21.   { TFileMapper }
  22.  
  23.   function TFileMapper.MapFile(AOffset: QWord): pointer;
  24.   const
  25.     // constants used by CreateFileMapping
  26.  
  27.     NO_MAXIMUM_SIZE_HIGH =       0;      // 0 indicates to use the size of the
  28.     NO_MAXIMUM_SIZE_LOW  =       0;      //   file
  29.  
  30.     // constants used by MapViewOfFileEx
  31.  
  32.     BEGINNING_TO_END     =       0;
  33.   var
  34.     FileMapping: HANDLE;
  35.  
  36.   begin
  37.     Result := nil;
  38.  
  39.     // with the file handle, create a mapping for it
  40.     FileMapping := CreateFileMappingA(Handle,
  41.                                       nil,
  42.                                       PAGE_READONLY,
  43.                                       NO_MAXIMUM_SIZE_HIGH,  // use file size
  44.                                       NO_MAXIMUM_SIZE_LOW,
  45.                                       nil);
  46.     if FileMapping = 0 then
  47.       raise Exception.createfmt('File: %s, Line: %s, Unable to map file: %s', [{$I %File%},{$I %Line%},FileName]);
  48.  
  49.     Result := MapViewOfFileEx(FileMapping,
  50.                          FILE_MAP_READ,
  51.                          HI(AOffset),                        // from beginning
  52.                          LO(AOffset),
  53.                          BEGINNING_TO_END,                   // to end
  54.                          nil);                               // map anywhere
  55.  
  56.     CloseHandle(FileMapping);
  57.  
  58.     if Result = nil then
  59.       raise Exception.createfmt('File: %s, Line: %s, Unable to map view of file: %s', [{$I %File%},{$I %Line%},Filename]);
  60.   end;
  61.  
  62. end.

Now that I've gotten to inspect it again, I have a question and a comment (or two, not sure yet)

1. The Handle being used in CreateFileMapping looks like a field inherited from TFileStream. Is this correct ?

2. The non-OOP version could be called without creating classes or anything else.  Just pass the filename string and that's it.  IOW, no need to create a class, no need to subsequently free the class.   It seems your code isn't complete, in order to use it, you have to create objects of that type.  The non-OOP version doesn't need to do that, call the function, you're done.

and your comments are welcome too :)


ETA:

Just info: it looks like I might not be able to reply for the next 10 to 12 hours.
Title: Re: Exceptions
Post by: engkin on July 17, 2019, 08:07:55 am
In OOP if you create an object, you are responsible of freeing it. Similar to dealing with pair of functions Open/Close files. Or in our example: CreateFileMappingA then later CloseHandle.
The non-OOP function didn't need to allocate any memory (creating an object, allocates memory.)  Your usage of objects has imposed unnecessary overhead (create/use/track/free) on the program.
That is true, but if it were for just one small function there would not be any need for OOP nor higher level languages. Assembly would be more than enough, that is one. Two, the concern about memory is not relevant any more with the gigantic amount of memory, unless you are working on some small limited embedded system or micro controller.

The code I showed above does not leak memory, you can try it.
It won't leak memory as long as the object is freed but, the freeing of the object is conspicuously missing as is its creation.  The non-OOP version didn't need to, directly or indirectly, allocate memory and didn't need code someplace else to free an unnecessary memory block (the object/class itself.)
If the class is supposed to get a pointer to a mapped file, and nothing else. It is possible to change MapFile into a class function which does not need an object. It is usage would be more like:
Code: Pascal  [Select]
  1.   ptr := TFileMapper.MapFile(SomeFileName);

But I am sure you need to do some more work with this pointer. And that is why I built this class on top of TFileStream.

It is dealing specifically with mapping the file, not with opening the file. Opening the file is left to the parent class: TFileStream.
And that is one of the problems.  In the non-OOP version, everything is right there in the function, makes it really easy to understand.  The programmer doesn't have to know/guess which of the ancestors opened the file nor wonder where the file handle is coming from.
Seeing everything that is not related to the function of the class (mapping files at this point ) is the problem. The programmer does not guess or wonder, the class and its ancestors are accessible. If you use Lazarus, click on Handle while holding CTRL key and it takes you to where it came from. You do not need to know where exactly the file is getting opened, you just need to know its handle. That is all you need to know to be able to map it. Otherwise, you still don't know how it is getting opened by the system when you call an API, in fact you do not have access to the source code, at best you have access to its assembly, which is, of course, not needed.

I don't need to repeat that code,
It's not about repetition.  The call to CreateFile occurred in only one place in the non-OOP version.  The same place where the file mapping and the view are created.  All nice and neat in one function.   No need to go looking for stuff someplace else and wonder why the File Mapping is closed/freed but the File Handle isn't.  Unlike the OOP version, symmetrical and parallel as it should be.
Reusing code is part of, you just need one class that opens files and allows you basic functions related to the open file. You do not need to call CreateFile every time you need to deal with some file.

and as an extra bonus, that part related to opening the file is going to work on any major platform FPC runs on: Windows, Linux, and Mac. If I were to port this class to, say, Linux, I just need to worry about MapFile function.
That's not a result of OOP programming, it's a result of the RTL.  You're giving OOP credit it did not earn.
I used the RTL class, and I mentioned what I know about the RTL (which applies to the class), not trying to credit it to OOP, but mentioned it as a fact.

Is your code multi-platform?
No and, it doesn't have to be.  The program (not just the function) only applies to Windows therefore, portability in this case is not applicable and, even if it were, it is no thanks to OOP but, to the RTL functions regardless of the paradigm used to implement them.
I used the RTL, your function did not use the RTL. I credit my class the benefit of using the RTL, see? Re-usability.

Quote
  TFileMapper = class(TFileStream)       // piece #2 - I suppose this is where the file is opened.
No, this is to give our class all the abilities of TFileStream, and to build on top of it.
The file doesn't get opened magically. Based on the classes and methods you've presented, it gives the impression that the inherited create method (from TFileStream) opens the file and, unnecessarily keeps the file handle opened.
I just answered this one when I mentioned changing the function into class function, of course with whatever modification needed.

Quote
    constructor Create(AFileName: String);  // piece #3 - didn't need that before.  Again, it probably allocates memory
                                            // which doesn't seem to be freed anywhere.
This part is not necessary in our example, and it does not allocate any memory that needs to be freed.
It's a fact that, somewhere memory is being allocated to hold the object and, that memory block has to be eventually freed.  That's another thing your program needs to keep track of that the non-OOP version doesn't have to do.
Again, no need for OOP to run one function. The result of your function is going to be used somehow. Also, the amount of memory is not relevant, leaks are.

Quote
   // not comparing, just an observation.  The routine I posted didn't give the option to map at an offset.
   // your implementation is going beyond the call of duty... which is perfectly fine.

    function MapFile(AOffset: QWord = 0): pointer;  {AOffset from beginning of file}  // piece #4
Because I can focus on mapping itself and different possibilities. Like you can pass the offset if you want, and if you do not it takes 0 by default.
Nothing there that cannot be done just as simply without OOP.
In this case of this example, this is true. But with objects you can keep related functions together. Maybe there is no reason to show the pointer returned by MapFile, and all the functionality could be contained in the class.

An object gives you more control over your variables. Some variables are local to one member function, while other variables are used by more than one member function and need to be valid along the life of the object. Some variables are common among all instances of this object.

It also gives you control over visibility, you could choose to make some fields or member functions not visible outside the object, or expand their visibility to descendant classes, or totally visible.

Then you have properties (and default property), inheritance, interfaces, RTTI,  ... etc. And don't forget generics. Generics is where you can't compare it with functions anymore, not even with generic functions. You need a list that deals with, say, doubles? Use TFPGList from unit fgl: specialize TFPGList<double>. Maybe you need a list of strings as well, simple: specialize TFPGList<string>. These lists are objects.

Quote
  // I presume the program/caller will have to create an instance of TFileMapper.  I am under the impression that would
  // be a global variable (that would not be desirable and it was not needed in the non-OOP function)
It does not need to be global variable, it is up to you. In fact you can create , use, and free this class without a variable at all!
I'll give you that it doesn't have to be a global variable but, I find your claim that the class can be created, used and freed without using a variable, "dubious".  You got to have a way to refer to the class and, that's usually a variable (specifically, a pointer to the class - though the fact that it is a pointer is hidden by the compiler.)
It is not unusual to use WITH:
Code: Pascal  [Select]
  1.     with TFileMapper.Create('test.txt', fmOpenRead or fmShareDenyWrite) do
  2.     begin
  3.       ptr := MapFile;
  4.       Free;
  5.     end;

But it is meant to be used more, or less like:
Code: Pascal  [Select]
  1.   try
  2.     FileMapper := TFileMapper.Create('test.txt', fmOpenRead or fmShareDenyWrite);
  3.     ptr := FileMapper.MapFile;
  4.   except
  5.     on e: Exception do WriteLn(e.Message); //<--- use your preferred way to report or react to the error.
  6.   end;
  7.   FileMapper.Free;

Full version:
Code: Pascal  [Select]
  1.   try
  2.     try
  3.       FileMapper := TFileMapper.Create('test.txt', fmOpenRead or fmShareDenyWrite);
  4.       p := FileMapper.MapFile(0);
  5.     except
  6.       on e: Exception do WriteLn(e.Message);
  7.     end;
  8.   finally
  9.     FileMapper.Free;
  10.   end;

Quote
  // vvvv  this wasn't necessary before.  There was no need to call an ancestor to do a CreateFile.

  constructor TFileMapper.Create(AFileName: String);
  begin
    Inherited Create(AFileName, fmOpenRead or fmShareDenyWrite);
  end;
As you can see, it calls the parent class with fmOpenRead or fmShareDenyWrite as a second parameter so you do not have to pass them  when you use this class, as it is not relevant for the user of this class to bother with this specific detail.
The non-OOP version didn't even need to pass those flags and, it didn't need to call a "parent" to execute one API call.  That's what OOP does, break things into tiny pieces, then the programmer has to reassemble all those pieces to figure out what the purpose of all of them put together is. 
If the only duty of your program is to map a file and get a pointer, then do not use OOP. I am sure it does more than mapping a file.

To show how easy it is to maintain it, here is a reduced one based on your feedback:
Code: Pascal  [Select]
  1. unit uFileMapper_2;
  2.  
  3. {$mode objfpc}{$H+}
  4.  
  5. interface
  6.  
  7. uses
  8.   Classes, SysUtils;
  9.  
  10. Type
  11.   { TFileMapper }
  12.   TFileMapper = class(TFileStream)       // It does not allocate memory here.
  13.   public
  14.     function MapFile(AOffset: QWord = 0): pointer;  {AOffset from beginning of file}
  15.   end;
  16.  
  17. implementation
  18. uses
  19.   Windows;
  20.  
  21.   { TFileMapper }
  22.  
  23.   function TFileMapper.MapFile(AOffset: QWord): pointer;
  24.   const
  25.     // constants used by CreateFileMapping
  26.  
  27.     NO_MAXIMUM_SIZE_HIGH =       0;      // 0 indicates to use the size of the
  28.     NO_MAXIMUM_SIZE_LOW  =       0;      //   file
  29.  
  30.     // constants used by MapViewOfFileEx
  31.  
  32.     BEGINNING_TO_END     =       0;
  33.   var
  34.     FileMapping: HANDLE;
  35.  
  36.   begin
  37.     Result := nil;
  38.  
  39.     // with the file handle, create a mapping for it
  40.     FileMapping := CreateFileMappingA(Handle,
  41.                                       nil,
  42.                                       PAGE_READONLY,
  43.                                       NO_MAXIMUM_SIZE_HIGH,  // use file size
  44.                                       NO_MAXIMUM_SIZE_LOW,
  45.                                       nil);
  46.     if FileMapping = 0 then
  47.       raise Exception.createfmt('File: %s, Line: %s, Unable to map file: %s', [{$I %File%},{$I %Line%},FileName]);
  48.  
  49.     Result := MapViewOfFileEx(FileMapping,
  50.                          FILE_MAP_READ,
  51.                          HI(AOffset),                        // from beginning
  52.                          LO(AOffset),
  53.                          BEGINNING_TO_END,                   // to end
  54.                          nil);                               // map anywhere
  55.  
  56.     CloseHandle(FileMapping);
  57.  
  58.     if Result = nil then
  59.       raise Exception.createfmt('File: %s, Line: %s, Unable to map view of file: %s', [{$I %File%},{$I %Line%},Filename]);
  60.   end;
  61.  
  62. end.

Now that I've gotten to inspect it again, I have a question and a comment (or two, not sure yet)

1. The Handle being used in CreateFileMapping looks like a field inherited from TFileStream. Is this correct ?
Yes. Actually TFileStream inherited it from THandleStream.

2. The non-OOP version could be called without creating classes or anything else.  Just pass the filename string and that's it.  IOW, no need to create a class, no need to subsequently free the class.   It seems your code isn't complete, in order to use it, you have to create objects of that type.  The non-OOP version doesn't need to do that, call the function, you're done.
You can have similar functions in classes. You can call them without creating a class as I showed above.

and your comments are welcome too :)
Thank you, and feel free to ask any question you might have.  :)
Title: Re: Exceptions
Post by: SymbolicFrank on July 17, 2019, 10:37:08 am
About the OOP / non-OOP:

The reason why many C libraries are such a mess isn't because header files exist (although that certainly isn't helping), but because the contents aren't grouped in a usable way. Like, they often start with multiple files simply containing all the constants used, followed with multiple files containing all the globals. Lots of clutter.

OOP is all about that logical grouping. The specifics for device A go into the object that handles that device. And because only that object is using them, we can encapsulate them: hide them from the rest of the program.

The result is, that each object has its own API / interface and contains everything you need to know about the device it encapsulates.  8-)

The code reuse is just a bonus.
Title: Re: Exceptions
Post by: SymbolicFrank on July 17, 2019, 11:24:49 am
I forgot to post an example of how many of my functions that handle exceptions look like:

Code: Pascal  [Select]
  1. function Exceptional: Boolean;
  2. begin
  3.   Result := False;
  4.  
  5.   try
  6.     DangerousProcedure1;
  7.     if DangerousFunction then
  8.       DangerousProcedure2;
  9.  
  10.     Result := True;
  11.   except
  12.   end;
  13. end;    

If there are resources to clean up, this becomes:

Code: Pascal  [Select]
  1. function Exceptional: Boolean;
  2. var
  3.   obj: TObject;
  4. begin
  5.   Result := False;
  6.  
  7.   try
  8.     obj := TObject.Create;
  9.  
  10.     DangerousProcedure1(obj);
  11.     if DangerousFunction then
  12.       DangerousProcedure2;
  13.  
  14.     Result := True;
  15.   finally
  16.     obj.Free;
  17.   end;
  18. end;      


Btw, while most people here seem to agree that you should check your inputs, most often the reasoning is that you should implement functions exactly as designed, and that returning errors is useless because they will be ignored. So you have to raise an exception if something is amiss. Or just not catch the previous one.
Title: Re: Exceptions
Post by: Thaddy on July 17, 2019, 12:23:43 pm
I hope you mean:
Code: Pascal  [Select]
  1. function Exceptional: Boolean;
  2. var
  3.   obj: TObject;
  4. begin
  5.   Result := False;
  6.   obj := TObject.Create; // <--------- should be here.
  7.    try
  8.     DangerousProcedure1(obj);
  9.     if DangerousFunction then
  10.       DangerousProcedure2;
  11.     Result := True;
  12.   finally
  13.     obj.Free;
  14.   end;
  15. end;
 
Title: Re: Exceptions
Post by: SymbolicFrank on July 17, 2019, 01:08:40 pm
Yes, you're right. If I was initializing an OS resource, it should go inside the try..finally, but in this example it should be outside.
Title: Re: Exceptions
Post by: 440bx on July 17, 2019, 02:21:28 pm
In OOP if you create an object, you are responsible of freeing it. Similar to dealing with pair of functions Open/Close files. Or in our example: CreateFileMappingA then later CloseHandle.
The non-OOP function didn't need to allocate any memory (creating an object, allocates memory.)  Your usage of objects has imposed unnecessary overhead (create/use/track/free) on the program.
That is true, but if it were for just one small function there would not be any need for OOP nor higher level languages. Assembly would be more than enough, that is one. Two, the concern about memory is not relevant any more with the gigantic amount of memory, unless you are working on some small limited embedded system or micro controller.
The amount of memory consumed is a secondary concern.  The real concern is that with OOP you have keep track of those objects so they can eventually be freed.  That subtracts from the program's simplicity and ease of maintenance.

Your OOP program, has to keep track of "entities" (memory blocks) in this case, which are unnecessary to implement the function.  The non-OOP version isn't burdened with having to keep track of things it doesn't need to get the job done.

My premise is and, has always been, that a non-OOP program is simpler, easier to understand and easier to maintain.  Your acknowledging that the OOP version needs to creates things the non-OOP version doesn't need, validates that.

The concern is very relevant because, once the non-OOP version calls the function, the only thing it has left to concern itself with is, unmapping the view of the file using the pointer returned by the function.   It doesn't have to track and free blocks of memory, it doesn't have to ensure that the file handle is eventually closed because it's already closed, it's not carrying baggage through the entire program  execution.  In addition to all that, in the non-OOP version, everything is right there, in one place, logically grouped, coherent and explicit instead of broken into multiple pieces the programmer has to mentally assemble.


The code I showed above does not leak memory, you can try it.
It won't leak memory as long as the object is freed but, the freeing of the object is conspicuously missing as is its creation.  The non-OOP version didn't need to, directly or indirectly, allocate memory and didn't need code someplace else to free an unnecessary memory block (the object/class itself.)
If the class is supposed to get a pointer to a mapped file, and nothing else. It is possible to change MapFile into a class function which does not need an object. It is usage would be more like:
Code: Pascal  [Select]
  1.   ptr := TFileMapper.MapFile(SomeFileName);
The code (class or otherwise) is supposed to map a file whose name is passed to it as a parameter and return the pointer to the location where the file is mapped.  That's all it's supposed to do.   Things it's _not_ supposed to do: implicitly or explicitly allocating unnecessary memory and, leaving open handles that should be closed.  Those are two things it's not supposed to do, yet your OOP implementation does both thus complicating the implementation.


But I am sure you need to do some more work with this pointer. And that is why I built this class on top of TFileStream.
You're right, it does plenty of work with that pointer, that's why there is a function to obtain it.  What there isn't are, 1. more than one function (class/object/whatever) which is unnecessary.  2. unnecessary memory blocks allocated that eventually have to be freed and, 3. handle(s) that are left open when they should have been closed because they are not needed.

The existence of those "deficiencies" in the OOP version already shows very clearly that the non-OOP version is cleaner, simpler and results in a program that is easier to understand and maintain.  And those deficiencies are not the entire list, there are others.

It is dealing specifically with mapping the file, not with opening the file. Opening the file is left to the parent class: TFileStream.
And that is one of the problems.  In the non-OOP version, everything is right there in the function, makes it really easy to understand.  The programmer doesn't have to know/guess which of the ancestors opened the file nor wonder where the file handle is coming from.
Seeing everything that is not related to the function of the class (mapping files at this point ) is the problem. The programmer does not guess or wonder, the class and its ancestors are accessible. If you use Lazarus, click on Handle while holding CTRL key and it takes you to where it came from. You do not need to know where exactly the file is getting opened, you just need to know its handle. That is all you need to know to be able to map it. Otherwise, you still don't know how it is getting opened by the system when you call an API, in fact you do not have access to the source code, at best you have access to its assembly, which is, of course, not needed.
In the non-OOP implementation, I don't need to go looking around for where the file handle is.  I don't need to depend on Lazarus (or any other tool) to show me where the file handle is (it's right there as a local variable.)  The programmer needs to know where the handle is because, he/she has to ensure that handle is freed.  In the non-OOP version ensuring that is trivial, in the OOP version, it isn't.  You are trying to obviate the fact that after the file is mapped you are left with unfinished tasks that must eventually be done, such as freeing objects and closing handles.  The non-OOP version does not burden the programmer with "pending" tasks that could be forgotten, thereby become a problem.

I don't need to repeat that code,
It's not about repetition.  The call to CreateFile occurred in only one place in the non-OOP version.  The same place where the file mapping and the view are created.  All nice and neat in one function.   No need to go looking for stuff someplace else and wonder why the File Mapping is closed/freed but the File Handle isn't.  Unlike the OOP version, symmetrical and parallel as it should be.
Reusing code is part of, you just need one class that opens files and allows you basic functions related to the open file. You do not need to call CreateFile every time you need to deal with some file.
If I need to map another file, I just call the FileMap function again. I don't need to code another call to CreateFile either and, my function doesn't leave unnecessary "baggage" laying around that I eventually have to deal with.


and as an extra bonus, that part related to opening the file is going to work on any major platform FPC runs on: Windows, Linux, and Mac. If I were to port this class to, say, Linux, I just need to worry about MapFile function.
That's not a result of OOP programming, it's a result of the RTL.  You're giving OOP credit it did not earn.
I used the RTL class, and I mentioned what I know about the RTL (which applies to the class), not trying to credit it to OOP, but mentioned it as a fact.
If I had wanted to make the function portable I could have made it portable but, since it doesn't have to be, there is no need to write unnecessary code.   Portability is nice to have when it is needed but, it's not always needed.  The real point is, OOP has nothing to do with portability.  Portability is usually obtained by implementing an additional interface layer which can be implemented using any programming paradigm.

Is your code multi-platform?
No and, it doesn't have to be.  The program (not just the function) only applies to Windows therefore, portability in this case is not applicable and, even if it were, it is no thanks to OOP but, to the RTL functions regardless of the paradigm used to implement them.
I used the RTL, your function did not use the RTL. I credit my class the benefit of using the RTL, see? Re-usability.
The RTL doesn't bring anything to the table that the program needs.  The Filemap function is very reusable as it is but, I will admit that I could have coded it in a way that would make it completely reusable instead of just "easily reusable".  I prefer code that is easy to customize than code that is fully reusable.  By fully re-usable I mean code that can simply be copied and pasted without any changes or placed in a unit and compiled for direct re-use.


Quote
  TFileMapper = class(TFileStream)       // piece #2 - I suppose this is where the file is opened.
No, this is to give our class all the abilities of TFileStream, and to build on top of it.
The file doesn't get opened magically. Based on the classes and methods you've presented, it gives the impression that the inherited create method (from TFileStream) opens the file and, unnecessarily keeps the file handle opened.
I just answered this one when I mentioned changing the function into class function, of course with whatever modification needed.
And what you've avoided is justifying leaving the file handle open.  Your OOP version burdens the program with unfinished tasks.

Quote
    constructor Create(AFileName: String);  // piece #3 - didn't need that before.  Again, it probably allocates memory
                                            // which doesn't seem to be freed anywhere.
This part is not necessary in our example, and it does not allocate any memory that needs to be freed.
It's a fact that, somewhere memory is being allocated to hold the object and, that memory block has to be eventually freed.  That's another thing your program needs to keep track of that the non-OOP version doesn't have to do.
Again, no need for OOP to run one function. The result of your function is going to be used somehow. Also, the amount of memory is not relevant, leaks are.
Are you saying that the function is better implemented without using OOP ?... I agree :)  As far as the amount of memory is concerned, as I've stated previously, the amount of memory isn't my primary concern in this case, my concern is that there are memory blocks left laying around that have to be freed.  The non-OOP version doesn't have to worry about that because no such memory blocks exist.


Quote
   // not comparing, just an observation.  The routine I posted didn't give the option to map at an offset.
   // your implementation is going beyond the call of duty... which is perfectly fine.

    function MapFile(AOffset: QWord = 0): pointer;  {AOffset from beginning of file}  // piece #4
Because I can focus on mapping itself and different possibilities. Like you can pass the offset if you want, and if you do not it takes 0 by default.
Nothing there that cannot be done just as simply without OOP.
In this case of this example, this is true. But with objects you can keep related functions together.
You can keep related functions together without using OOP.  OOP doesn't bring that to the table.

Maybe there is no reason to show the pointer returned by MapFile, and all the functionality could be contained in the class.
Maybe so but, that would create a lot of unnecessary coupling among the functions that implement the functionality.

An object gives you more control over your variables. Some variables are local to one member function, while other variables are used by more than one member function and need to be valid along the life of the object. Some variables are common among all instances of this object.
If an object gives more control over variables then why is it that a file handle which is no longer needed isn't closed ?  I don't call that more control.  I call that creating artificial dependencies in the implementation.


It also gives you control over visibility, you could choose to make some fields or member functions not visible outside the object, or expand their visibility to descendant classes, or totally visible.
The only thing that needs to be visible is the pointer where the file is mapped.  What is visible in the OOP version, among other things, is that there is a file handle somewhere that hasn't been closed and there are memory blocks (objects/classes/whatever) that need to be freed.  More things to keep track of.  More room for something that needs to be done to be forgotten or left out.

Then you have properties (and default property), inheritance, interfaces, RTTI,  ... etc. And don't forget generics. Generics is where you can't compare it with functions anymore, not even with generic functions. You need a list that deals with, say, doubles? Use TFPGList from unit fgl: specialize TFPGList<double>. Maybe you need a list of strings as well, simple: specialize TFPGList<string>. These lists are objects.
That sounds nice IF you need them and/or don't have better solutions.  Of everything you've listed there, the only thing I have occasionally wished for are generics and, generics are not an OOP construct, it's essentially a compiler macro.  They are a nice feature when needed but they have nothing to do with OOP.

The forum software just informed me that this posts exceeded the maximum allowed length (20000 characters).  Not only does OOP result in code bloat, it also results in post bloat. ;)  The post has been "abbreviated" to make it acceptable to the forum software.