Recent

Author Topic: Exceptions  (Read 5223 times)

julkas

  • Guest
Re: Exceptions
« Reply #30 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?

SymbolicFrank

  • Hero Member
  • *****
  • Posts: 1313
Re: Exceptions
« Reply #31 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.

440bx

  • Hero Member
  • *****
  • Posts: 3921
Re: Exceptions
« Reply #32 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.
« Last Edit: July 16, 2019, 10:26:34 pm by 440bx »
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: Exceptions
« Reply #33 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?
« Last Edit: July 16, 2019, 10:28:23 pm by engkin »

440bx

  • Hero Member
  • *****
  • Posts: 3921
Re: Exceptions
« Reply #34 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.














« Last Edit: July 16, 2019, 11:38:12 pm by 440bx »
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: Exceptions
« Reply #35 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.
« Last Edit: July 17, 2019, 12:37:57 am by engkin »

440bx

  • Hero Member
  • *****
  • Posts: 3921
Re: Exceptions
« Reply #36 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.
« Last Edit: July 17, 2019, 01:41:26 am by 440bx »
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: Exceptions
« Reply #37 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.  :)

SymbolicFrank

  • Hero Member
  • *****
  • Posts: 1313
Re: Exceptions
« Reply #38 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.

SymbolicFrank

  • Hero Member
  • *****
  • Posts: 1313
Re: Exceptions
« Reply #39 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.
« Last Edit: July 17, 2019, 11:49:35 am by SymbolicFrank »

Thaddy

  • Hero Member
  • *****
  • Posts: 14161
  • Probably until I exterminate Putin.
Re: Exceptions
« Reply #40 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;
 
Specialize a type, not a var.

SymbolicFrank

  • Hero Member
  • *****
  • Posts: 1313
Re: Exceptions
« Reply #41 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.

440bx

  • Hero Member
  • *****
  • Posts: 3921
Re: Exceptions
« Reply #42 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.
« Last Edit: July 17, 2019, 02:26:32 pm by 440bx »
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

 

TinyPortal © 2005-2018