Improved Singleton

[erifash]

I got bored and decided that the code for the function _Singleton() (found in Misc.au3) was too long. This is the *much* shortened version:

EDIT: Code has been modified.

Func _Exist($handle, $flag = 0)
        Local $ERROR_ALREADY_EXISTS = 183, $dll = DllCall("kernel32.dll", "int", "CreateMutex", "int", 0, "long", 1, "str", StringReplace($handle, "\", "")), $err = DllCall("kernel32.dll", "int", "GetLastError")
        If $err[0] = $ERROR_ALREADY_EXISTS and $flag = 0 Then Exit
        SetError($err[0])
        Return $dll[0]
EndFunc

The syntax in this version is still the same.

I hope you like it!

Edited October 23, 2005 by erifash

[Valik]

It may be shorter but I highly doubt it's either (measurably) faster or improved.

First of all, when I read that code, I see "183" and as far as I know, it's a magic number. Magic numbers are bad because they are magic and nobody but you really knows what the number means. So you traded one line of code at the cost of readability/comprehensibility.

The next thing I see that you stripped out was the StringReplace() line. This line pretty much guaranteed the function would never attempt to create an invalid Mutex. Now your version without the line will allow the function to fail. So you traded one line of code at the cost of introducing potential failure.

Finally, you removed the If/Else structure. Your version does the same thing except its harder to read for some people. I prefer to see code written as:

If condition Then
  ; Do stuff
Else
  ; Do other stuff
EndIf

So you traded something like 5 lines at the cost of readability.

You also gave short, ambiguous names to all the variables. This, too, makes the code harder to read because the name of the variable doesn't convey what it actually holds.

So let's review what you did.

• Added a potential situation where the function will fail.
• Added a magic number which people will have to look up if they want to understand the function.
• Reduce the number of lines by removing block structure and also shortened the variable names. This results in making the time necessary to read and comprehend the function larger than it is in the longer version.

All this just to gain an imperceptible bit of speed. I'll take the readability over the speed in this case since I can't measure the speed but I can measure the readability. Factor in, too, that the overhead for this function is probably going to be a single call for the entire lifetime of the script and the speed is really not an issue at all.

[erifash]

Thanks for the reply! I didn't even notice that I was missing the StringReplace, it must have gotten lost in translation. I agree with you that magic numbers are not good. I also agree about the readibility of the function. However, it seems a little bit easier for me to not use If structures in short functions; take a look at my modified _Iif() function (found in Misc.au3)

Func _Test($b_Test, $v_True, $v_False)
    If $b_Test Then Return $v_True
    Return $v_False
EndFunc

If $b_Test was false then it wouldn't execute the first Return. If it was true then it would execute the first return and end the function. That's really all you need. I have always been focused on writing shorter code where it is possible. I agree that trading length for readibility is bad, but I have grown used to understanding that shortened code.

I will fix it according to your suggestions, thanks!