DriveGetDrive API error for proper type - why?

[mrider]

Sorry for the terse topic title, I'll happy to edit it so it sounds less confrontational if anyone has any suggestions.

I'm wondering if it might be better if DriveGetDrive behaved a bit differently.  On my computer, the code below fails at "REMOVABLE", since I don't have any removable drives.  My question is not, how do I keep it from failing.  My question is why does asking for something legitimate - that I just happen to not have - considered an error?  Wouldn't it make more sense to simply return the equivalent of

$array[1] = [0] - and reserve the error for something truly wrong?  Like DriveGetDrive("A_Totally_BOGUS_VALUE")?

PrintDrive("ALL")
PrintDrive("CDROM")
PrintDrive("REMOVABLE")
PrintDrive("FIXED")
PrintDrive("NETWORK")
PrintDrive("RAMDISK")
PrintDrive("UNKNOWN")

Func PrintDrivesForName($drive)
    ConsoleWrite($drive & " : ")
    Local $drives = DriveGetDrive($drive)
    For $i = 1 To $drives[0]
        ConsoleWrite($drives[$i] & @TAB)
    Next
    ConsoleWrite(@LF)
EndFunc

The problem is that it greatly complicates handling the case of "you don't have one of those" - which is distinct from "you asked for something totally incorrect".

 

Please note once again, my question is not how do I keep the code from failing - I can figure out how to add an error check. 

My quesiton is: what use-case makes the current behavior desirable over simply returning an array with zero as the first and only element?  (Except in the case where the code genuinely asks for something bogus of course)

[Melba23]

mrider,

Good question. I will move the thread to the more suitable "Developer Chat" section.

M23

[mrider]

Thanks!

[JohnOne]

 I would imagine it's because it's trivial to implement.

[BrewManNH]

It's not failing, you don't check for an error, your code is what is failing.

If you use DriveGetDrive with "Removable" and you have no removable drives, the function returns @error set to 1, checking to see if @error is anything other than zero would prevent the array error message.

[jpm]

I understand you like a return array with the first value as 0 but checking error whatever it is return as, is a fundamental habit.

I don't think Jon will change the error return for this function

[mrider]

 

 I would imagine it's because it's trivial to implement.

I suppose. 

However, it seems to me like it would be trivial to alter the API once, and then let everyone get the benefit of that change.  The problem is that as it stands, there are probably hundreds of scripts out there that ignore the error message that they know can't happen, but really can.  How often do you check @error for e.g. CDROM?  The computer always has a CDROM after all.  But what if it doesn't?

One of the beauties of AutoIt is that one can quickly write code that has a relatively low bug count due to the fact that AutoIt tolerates inexactness well.  For example:

Local $l = "5"

Local $r = "10"

ConsoleWrite(($l * $r) & @LF)

 

Most programming languages would fall on their face immediately on that code.  AutoIt would be considerably less pleasant to use if that threw an error because $l and $r started life as Strings (cough Python cough).

 

 

 

It's not failing, you don't check for an error, your code is what is failing.

That's not what I asked.  My question is - why does it set an error at all?  There's nothing wrong with asking about "REMOVABLE" other than the fact that I don't have any.  It seems to me that the error would be better served replying to a request for something outside the allowed types.

Edited September 23, 2013 by mrider

[mrider]

 

I understand you like a return array with the first value as 0 but checking error whatever it is return as, is a fundamental habit.

I don't think Jon will change the error return for this function

 

Fair enough.

[mrider]

Knowing that this will never come to pass, I'd still like to leave this parting shot.  Compare the two versions of the same subroutine - one where the API is altered versus as it is now (and thank you to all for reading and replying to my post):

; Returning a zero array
Func PrintDrivesForName($drive)
    Local $drives = DriveGetDrive($drive)
    ConsoleWrite($drive & " : ")
    If @error Then
        ConsoleWrite($drive & " is not a valid designation")
    Else
        For $i = 1 To $drives[0]
            ConsoleWrite($drives[$i] & @TAB)
        Next
    EndIf
    ConsoleWrite(@LF)
EndFunc

; As it is now
Func PrintDrivesForName($drive)
    Local $drive_types[7] = [ _
        "ALL", "CDROM", "REMOVABLE", "FIXED", "NETWORK", "RAMDISK", "UNKNOWN"]
    Local $drives = DriveGetDrive($drive)
    ConsoleWrite($drive & " : ")
    If @error Then
        Local $found = False
        For $i = 0 To UBound($drive_types) - 1
            If $drive = $drive_types[$i] Then
                $found = True
                ExitLoop
            EndIf
        Next
        If Not $found Then
            ConsoleWrite($drive & " is not a valid designation")
        EndIf
    Else
        For $i = 1 To $drives[0]
            ConsoleWrite($drives[$i] & @TAB)
        Next
    EndIf
    ConsoleWrite(@LF)
EndFunc

Edited September 23, 2013 by mrider

[BrewManNH]

 

That's not what I asked.  My question is - why does it set an error at all?  There's nothing wrong with asking about "REMOVABLE" other than the fact that I don't have any.  It seems to me that the error would be better served replying to a request for something outside the allowed types.

It sets an error because you created an error in looking for something that doesn't exist. What would you like it to return? Searching for something that doesn't exist is an error condition in every language I can think of.

 

Use _FileListToArray as an example, search for a folder that doesn't exist, you'll get an error condition, search for a file that doesn't exist, you get an error condition. This is consistent with practically every function.

[BrewManNH]

 

Knowing that this will never come to pass, I'd still like to leave this parting shot.  Compare the two versions of the same subroutine - one where the API is altered versus as it is now (and thank you to all for reading and replying to my post):

; Returning a zero array
Func PrintDrivesForName($drive)
    Local $drives = DriveGetDrive($drive)
    ConsoleWrite($drive & " : ")
    If @error Then
        ConsoleWrite($drive & " is not a valid designation")
    Else
        For $i = 1 To $drives[0]
            ConsoleWrite($drives[$i] & @TAB)
        Next
    EndIf
    ConsoleWrite(@LF)
EndFunc

; As it is now
Func PrintDrivesForName($drive)
    Local $drives = DriveGetDrive($drive)
    ConsoleWrite($drive & " : ")
    If @error Then
        Local $found = False
        For $i = 0 To UBound($drive_types) - 1
            If $drive = $drive_types[$i] Then
                $found = True
                ExitLoop
            EndIf
        Next
        If Not $found Then
            ConsoleWrite($drive & " is not a valid designation")
        EndIf
    Else
        For $i = 1 To $drives[0]
            ConsoleWrite($drives[$i] & @TAB)
        Next
    EndIf
    ConsoleWrite(@LF)
EndFunc

Your first example doesn't return anything,and it's the proper way to use the function as written. The second one also doesn't return anything, but isn't how the function works as is now either.

[wraithdu]

I think a better compromise might be simply an additional @error return value: 1 = no drives found, 2 = invalid drive specification. It's non-script breaking and adds your functionality. I say non-script breaking, because it's just as valid in the current function to test if the return is an array or not with IsArray().

PS, neither of your scripts will work correctly as written, as you're testing @error after ConsoleWrite() which will reset the @error value.

Edited September 23, 2013 by wraithdu

[mrider]

Oh well.  Just something I noticed and thought could be improved.  Never mind.

 

 

PS, neither of your scripts will work correctly as written, as you're testing @error after ConsoleWrite() which will reset the @error value.

 

You're right of course.  I quickly typed that without testing and didn't notice that problem.

Edited September 23, 2013 by mrider

[JohnOne]

There is nothing stopping you officially requesting this.

There's some sort of feature request place somewhere, sorry I don't know where to find it.

EDIT:

Think it's the bugtracker up the top there.

Edited September 23, 2013 by JohnOne

[mrider]

The reason I came up with this (admittedly dumb) idea in the first place is that I have a ton of scripts where I have a simple text field where I ask for a "starting directory" before performing some simple operation.  Typically I'll have a button titled something along the lines of "Select Directory" next to it that brings up a folder selection dialog.  I typically write the code to cleanly handle typed in directories as well as e.g. "ALL" or "NETWORK". 

It seemed like having an easy and clean way to deal with the difference between e.g. "NETWORK" versus "NETOWRK" would be nice (e.g. "you don't have any of those" as opposed to "hey dummy, check your spelling") would be helpful.

 

Wraithdu brought up an excellent point that never occurred to me - namely that people might be using IsArray() on the return of DriveGetDrive to switch on the results.  So the alteration I thought of would break scripts and would be bad.

I kind of like the idea of returning a different error for "you asked for a bogus name" as opposed to "you don't have any of those" though.  It doesn't break anything, and greatly simplifies error handling.  So I might try that as a suggestion.  The only scripts it would break would be ones where coders have already written "if error = 1" as opposed to "if error"

Edited September 23, 2013 by mrider

[JohnOne]

I don't think what wraithdu suggested would break any scripts.

Also, are you aware of FileSelectFolder() function?

[mrider]

 

I don't think what wraithdu suggested would break any scripts.

Agreed.  The thought I had would break scripts.  wraithdu's suggestion wouldn't. 

 

In fact, I turned in a feature request asking if the devs could alter the @error value from DriveGetDrive function to differentiate between using a term that is incorrect versus asking for something that's correct but happens to have no results.  http://www.autoitscript.com/trac/autoit/ticket/2483

 

 

Also, are you aware of FileSelectFolder() function?

That's precisely what I use.  Generally speaking, I tend to have a moderately large text box where one can type the path, and next to it I have a button that brings up the folder selector from FileSelectFolder function.  I don't like having nothing but a GUI, I can type "D:Data" faster than I can click the button and navigate to the folder in question.  On the other hand, I like being able to search for "D:DataFooBarBaz" when required.

As I say, I try to handle having the user type in e.g. "ALL" elegantly.

[wraithdu]

The additional @error return wouldn't patently be script breaking, however there are circumstances where it would be. Since the current docs specify an @error return of '1', versus a more general 'non-zero' return, it is acceptable for a dev to test 'If @error = 1 Then'. If a dev expects that to test any error condition, then yes, the addition of another @error return value would be script breaking for them.

But I think it is much more common for devs to test the more generic 'If @error Then'. So the impact would be minor, and documented of course.

[jpm]

Small remark,

I did not change in all place set @error to 1 to set @error to non-zero but should have do it.