Recent

Author Topic: CD Eject  (Read 10245 times)

J-G

  • Hero Member
  • *****
  • Posts: 953
Re: CD Eject
« Reply #15 on: February 22, 2019, 06:13:24 pm »
The change from UINT to byte is, I'll be nice and simply say that it's not a good idea.  The system can have more than 8 "devices" that have been assigned drive letters.  Using a byte means potentially throwing away 16 devices (I've seen a good  number of systems where the CDROM was assigned letter R or W).  Byte is definitely inadequate in this case, it borders on erroneous.  You should change that variable back to UINT.
???  Sorry I can't see your logic there, even if there were 255  Drive Types, Byte would still suffice - DrLet has a range of 26 characters, not DrType.

It used to be my practice to assign R & W to two Optical Drives when I built PCs which might be expected to copy CDs. I now tend to put single drives on 'Z'.

My current PC has only A, M & L free and the Optical is assigned 'R'.

Quote from: 440bx
here is another way of writing the for loop I used that you may like better and may not consider to be "over the top"
Code: Pascal  [Select][+][-]
  1. const
  2.   // drive letter bit indexes
  3.   A =  0;  
  4.   Z = 25;  
  5.  ...
  6. begin
  7.   for I in [A..Z] do
  8.   etc
  9. end;
Hmmm...  I still can't see any advantage in using that over :
Code: Pascal  [Select][+][-]
  1. begin
  2.   for I in [0..25] do
  3.   etc.
  4. end;

FPC 3.0.0 - Lazarus 1.6 &
FPC 3.2.2  - Lazarus 2.2.0 
Win 7 Ult 64

lucamar

  • Hero Member
  • *****
  • Posts: 4219
Re: CD Eject
« Reply #16 on: February 22, 2019, 07:56:00 pm »
[...] even if there were 255  Drive Types, Byte would still suffice [...]

Leaving apart the (somewhat logical) confussion between DrLet and DrType (and why not call them DriveLetter and DriveType? autocomplete is your friend here!), it's not really a good idea to use byte if the call to GetDriveType() returns a DWord. For one thing, it adds an unnecessary and loosy type-conversion; and, more important, you don't really know whether that result uses a simple progression (1, 2, 3, ...) or Microsoft uses (or will use it) as a bit-mask.

As a rule of thumb, it's always better to be safe than sorry: declare the variable of the same type the function returns and you'll never have to wonder why all at once the application doesn't work. ;)


@Bart: Nice function. Only thing I would do is remove the:
Code: [Select]
    DRIVE_UNKNOWN: Result := dtUnknown;line in the case and let it fall through to the
Code: [Select]
    else Result := dtUnknown;One less comparison! :)
« Last Edit: February 22, 2019, 08:06:10 pm by lucamar »
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus/FPC 2.0.8/3.0.4 & 2.0.12/3.2.0 - 32/64 bits on:
(K|L|X)Ubuntu 12..18, Windows XP, 7, 10 and various DOSes.

440bx

  • Hero Member
  • *****
  • Posts: 3946
Re: CD Eject
« Reply #17 on: February 22, 2019, 09:54:24 pm »
???  Sorry I can't see your logic there, even if there were 255  Drive Types, Byte would still suffice - DrLet has a range of 26 characters, not DrType.
I got the two (type and bitmap) mixed up therefore the range, as you pointed out, isn't an issue _but_, I still think it's a bad idea.  The reason is very simple, changing the size of the type to a different size than what is returned by Windows is an invitation to problems. It will work for you in this particular case but, it is still a bad idea and a very bad habit to acquire.

Quote from: 440bx
here is another way of writing the for loop I used that you may like better and may not consider to be "over the top"
Code: Pascal  [Select][+][-]
  1. const
  2.   // drive letter bit indexes
  3.   A =  0;  
  4.   Z = 25;  
  5.  ...
  6. begin
  7.   for I in [A..Z] do
  8.   etc
  9. end;
Hmmm...  I still can't see any advantage in using that over :
Code: Pascal  [Select][+][-]
  1. begin
  2.   for I in [0..25] do
  3.   etc.
  4. end;
The advantage should be very obvious... it makes it clear what the numbers mean and how they are used (particularly with the comment)... additionally, in a more complicated program that uses those values in more than one place, only the constant values needs to be changed.  (though in this particular program it is unlikely that such need would arise.)

you can't develop good programming habits and techniques if you constantly indulge in bad ones even when it would take next to no effort to do it right.  That is also part of personal programming style, therefore a personal choice. 


(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

J-G

  • Hero Member
  • *****
  • Posts: 953
Re: CD Eject
« Reply #18 on: February 22, 2019, 10:12:24 pm »
...if the call to GetDriveType() returns a DWord. For one thing, it adds an unnecessary and loosy type-conversion; and, more important, you don't really know whether that result uses a simple progression (1, 2, 3, ...) or Microsoft uses (or will use it) as a bit-mask.

As a rule of thumb, it's always better to be safe than sorry: declare the variable of the same type the function returns and you'll never have to wonder why all at once the application doesn't work. ;)
I've now looked at the GetDriveType() Function and see that it returns a UINT. I hadn't considered that aspect at all. I've also put it back to UINT and see that it doesn't make one iota of difference to the size of the compiled .EXE so, as you suggest Lucamar, better to be safe than sorry.

@bart
I haven't had time to study you posting properly - but I will :)   thanks for posting.

@440bx
Your post came in whilst I was typing. I take your point about bad habits  :-[  but the amount of programming I do is so small and none of it 'commercial' so as long as I can understand what's going on it is of little import   -  but thanks for taking the trouble to bring it to my attention, you could have simply cursed me in private and I would have learned nothing :)
FPC 3.0.0 - Lazarus 1.6 &
FPC 3.2.2  - Lazarus 2.2.0 
Win 7 Ult 64

Bart

  • Hero Member
  • *****
  • Posts: 5275
    • Bart en Mariska's Webstek
Re: CD Eject
« Reply #19 on: February 22, 2019, 10:44:30 pm »
@Bart: Nice function. Only thing I would do is remove the:
Code: [Select]
    DRIVE_UNKNOWN: Result := dtUnknown;line in the case and let it fall through to the
Code: [Select]
    else Result := dtUnknown;One less comparison! :)

Nice catch.
Thank you.

Bart

440bx

  • Hero Member
  • *****
  • Posts: 3946
Re: CD Eject
« Reply #20 on: February 22, 2019, 10:44:46 pm »
@440bx
Your post came in whilst I was typing. I take your point about bad habits  :-[  but the amount of programming I do is so small and none of it 'commercial' so as long as I can understand what's going on it is of little import   -  but thanks for taking the trouble to bring it to my attention, you could have simply cursed me in private and I would have learned nothing :)
I didn't mean to curse you but, I will admit to having been just a bit annoyed (partly for having confused one variable with another.. which is my mistake).  I understand your point about not programming much but, IMO, a programmer should, at least, consistently try to write the best code possible (unless it's a one time throw away kind of code.. and even then) 

The one exception I make to that is when I'm giving an example.  In those cases, I'll commonly use global variables (not a good thing at all) among other things I normally wouldn't but, I do it to emphasize the mechanics of the algorithm.  One thing that always helps make an example (or program) clearer and easier to understand is to choose good variable names and not abbreviate them (there are exceptions, such as loop indexes - commonly named "I" and such.)  I'm sure that in the example  I gave you, having variables named, DriveBitmap, DriveLetter, DriveType and DrivePresent made the example easier for you to understand.

The amount of effort it takes to have good variable names is negligible but, it often makes a significant difference in how easy (or hard) a program is to understand.  A few short and to the point comments put the final touch.  Those two things alone can make the difference between a program that is a pleasure to read from another one that is _literally_ a chore.

My suggestion is, go out of your way to develop good programming habits, bad ones are too easy to acquire (particularly if you read a lot of C code or program in C, though I don't think you are afflicted by that problem - programming in C, that is.)

ETA:

@Bart


the function
Code: Pascal  [Select][+][-]
  1. function GetDriveType(ADrive: Char): TDriveType;
can be implemented without a case statement.  The only comparison needed is to ensure that the DriveType returned by the API call is in the range of the enumerated type.


« Last Edit: February 22, 2019, 11:00:08 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.

Bart

  • Hero Member
  • *****
  • Posts: 5275
    • Bart en Mariska's Webstek
Re: CD Eject
« Reply #21 on: February 22, 2019, 11:20:10 pm »
@Bart

the function
Code: Pascal  [Select][+][-]
  1. function GetDriveType(ADrive: Char): TDriveType;
can be implemented without a case statement.  The only comparison needed is to ensure that the DriveType returned by the API call is in the range of the enumerated type.

Yes, I know and considered it.
I decided a case statement provided better readability.
Speed is not of concern to me here, since the API will eat more time than my code.

Bart

440bx

  • Hero Member
  • *****
  • Posts: 3946
Re: CD Eject
« Reply #22 on: February 22, 2019, 11:59:04 pm »
Speed is not of concern to me here, since the API will eat more time than my code.

Bart
I agree, speed can hardly be a concern in that case.  I should have mentioned what I'm going to mention now in my previous post.  I was thinking about two things, the first is code size.  The second is, with that implementation the function "hides" the possibility that GetDriveType may have been updated and returned a drive type which is not included in the enumerated type.  That implementation will lump that case as if it were the known case, DRIVE_UNKNOWN.  In other words, the function doesn't make it clear that it may need to be updated to account for new types.
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

lucamar

  • Hero Member
  • *****
  • Posts: 4219
Re: CD Eject
« Reply #23 on: February 23, 2019, 12:08:05 am »
[...] the function doesn't make it clear that it may need to be updated to account for new types.

But that would also happen with an enumeration if it were not updated, wouldn't it? Unless an "else" returned a known-bad value meaning "Type returned [XXX] which I don't know what it is!". Which, now I think about it, is (almost) the same as "DRIVE_UNKNOWN"...

Or am I missing something? (Very probable: I'm not, by far, a "Windows guru"...)
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus/FPC 2.0.8/3.0.4 & 2.0.12/3.2.0 - 32/64 bits on:
(K|L|X)Ubuntu 12..18, Windows XP, 7, 10 and various DOSes.

440bx

  • Hero Member
  • *****
  • Posts: 3946
Re: CD Eject
« Reply #24 on: February 23, 2019, 01:07:01 am »
Unless an "else" returned a known-bad value meaning "Type returned [XXX] which I don't know what it is!".
That's the idea.  There is a difference between a drive Windows doesn't know about and a drive the _function_ doesn't know about.  In the former, the function is operating as it should.  In the latter case, the function is missing a new type and needs to be updated to take it into account, yet, it doesn't give any indication that it needs to be updated.  It "hides" that when returning DRIVE_UNKNOWN for a drive Windows does not consider unknown.

It's much easier to maintain and keep a program up to date from one O/S version to another (and sometimes just a simple O/S update) when the code detects it is not able to process a return value as intended.  It can also save a lot of time because it spares the programmer from having to figure out why the program is no longer working quite as intended (particularly given that MS loves to "slip" updates into its user's computers.)

Which, now I think about it, is (almost) the same as "DRIVE_UNKNOWN"...
yes, almost but, not the same. :)
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

lucamar

  • Hero Member
  • *****
  • Posts: 4219
Re: CD Eject
« Reply #25 on: February 23, 2019, 01:20:52 am »
Yes, I see. You're right: "almost ... but not the same!" :)

So my correction to Bart's case statement was wrong too: the change should have been for the else part to set Result to something like "dtNewType".
« Last Edit: February 23, 2019, 01:25:27 am by lucamar »
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus/FPC 2.0.8/3.0.4 & 2.0.12/3.2.0 - 32/64 bits on:
(K|L|X)Ubuntu 12..18, Windows XP, 7, 10 and various DOSes.

440bx

  • Hero Member
  • *****
  • Posts: 3946
Re: CD Eject
« Reply #26 on: February 23, 2019, 01:46:56 am »
So my correction to Bart's case statement was wrong too: the change should have been for the else part to set Result to something like "dtNewType".
Yes, that could be one way of handling an unknown-to-the-function drive type.  Ideally, the code would turn that value into a visual cue in the interface, possibly something like "X: ???" in the list of drives or anything else that may be appropriate for the program.
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

Bart

  • Hero Member
  • *****
  • Posts: 5275
    • Bart en Mariska's Webstek
Re: CD Eject
« Reply #27 on: February 23, 2019, 03:11:58 pm »
The second is, with that implementation the function "hides" the possibility that GetDriveType may have been updated and returned a drive type which is not included in the enumerated type.  That implementation will lump that case as if it were the known case, DRIVE_UNKNOWN.  In other words, the function doesn't make it clear that it may need to be updated to account for new types.

In which case the function will crash if you just hardcast the windows returnvalue to the enum.
The predefined constants will most likely not change, but MS might add another one.
I rather have my program not recognize it then crash immediately.
And yes, I really did take this considerations into account when writing the code.

Bart

440bx

  • Hero Member
  • *****
  • Posts: 3946
Re: CD Eject
« Reply #28 on: February 23, 2019, 04:05:26 pm »
I rather have my program not recognize it then crash immediately.
And yes, I really did take this considerations into account when writing the code.

Bart
Maybe I missed something but, I don't think your function will crash, it will simply return "dtUnknown" for any drive type that your function is not aware of.
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

Bart

  • Hero Member
  • *****
  • Posts: 5275
    • Bart en Mariska's Webstek
Re: CD Eject
« Reply #29 on: February 23, 2019, 04:59:25 pm »
Maybe I missed something but, I don't think your function will crash, it will simply return "dtUnknown" for any drive type that your function is not aware of.

Which it does now.
If I rewrite the function so that Result := TDriveType(WinDriveType), it will crash if MS comes up with a new type.
The way around it is to check if WindriveType is in Ord(Low(TDriveType))..Ord(High(TDriveType)) and return dtUnknown if it is not.
And this looks ugly in my eyes, hence the case statement.

But, this is increasinlgy getting off-topic.

Bart

 

TinyPortal © 2005-2018