Sign in to follow this  
Followers 0
erifash

Improved Singleton

3 posts in this topic

#1 ·  Posted (edited)

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! B)

Edited by erifash

Share this post


Link to post
Share on other sites



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.

Share this post


Link to post
Share on other sites

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! B)

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0