Which function is faster and efficient?

Question

I need to verify if a certain user exist on my asp.net site.I want to know if which of these two functions is more efficient, faster and better compared to each other and why.Thanks in advance!

 Public Function CheckIfFriendExist(ByVal arg As String) As Boolean
        Dim alluser As New MembershipUserCollection()
        alluser = Membership.GetAllUsers()
        For Each user As MembershipUser In alluser
            If user.UserName.ToLower() = arg.ToLower() Then
                Return True
                Exit For
            End If
        Next
        Return False
    End Function

or

  Public Function CheckIFFriendExist2(ByVal arg As String) As Boolean
        Dim x As Integer = 0
        Dim themember As MembershipUserCollection = Membership.FindUsersByName(arg, 0, 1, 1)
        For Each member As MembershipUser In themember
            x = x + 1
        Next
        If x > 0 Then
            Return True
        Else
            Return False
        End If

    End Function

Answer 1

First comment - get rid of the pointless instantiation when you declare allUser in your first block - change

Dim alluser As New MembershipUserCollection()

to

Dim alluser As MembershipUserCollection

It's impossible to answer the rest without knowing about the membership provider in user and the implementation of the method. Have you tried just timing it?

Answer 2

The performance of these functions are not going to make a noticeable difference until you have hundreds of thousands of users for the application.
My suggestion is to stop micro-optimizing unless you have identified this as a problem area through profiling.

Answer 3

Which function is better in terms of readability:

I don't remember visual basic too well, but in the second function, isn't there a way to check themember to see whether it's empty directly, instead of having to loop through it? Some kind of method name like "IsEmpty".

Assuming that there is, you can change the code in the second example to just 2 lines:

Dim themember As MembershipUserCollection = Membership.FindUsersByName(arg, 0, 1, 1)
' Check if themember is empty, return true or false appropriately

In that case, the second function will be much better in the sense that it's easier to read.

Which function is better in terms of efficiency:

It's impossible to even guess without knowing the details of FindUsersByName or GetAllUsers, but I would guess the second function is faster (since it offloads the work to a specific function designed for it).

However, this is a very crude guess. The only proper way to answer this question is to implement the two and run tests on both, to find out which is faster.

In any case, the difference is unlikely to be enough to matter, but if you really care, you definitely have to time each solution.

Answer 4

Usually the easiest way to answer questions of this type ("which is faster"), is to simply measure it.

.NET har a built-in class for this purpose, System.Diagnostics.Stopwatch:

Dim stopwatch As New System.Diagnostics.Stopwatch
stopwatch.Start()
For i = 0 To 10000
    MethodThatMightJustTakeTooLong()
Next i
stopwatch.Stop()
Console.Writeline(Elapsed)

Answer 5

I would go with the second function with a few modifications.

Instead of using FindUsersByName, try GetUser -- this is more correct as you are looking for one user.

Then, check the count property of the user.

Dim user = Membership.GetUser(arg)
If user = 1 Then
    Return True;
Else
    Return False;
End If

I don't really know Visual Basic, but I'm sure that if statement could be simpler, something like return (user === 1) ? true : false in other languages.

Answer 6

Generally speaking, the second option has the better potential for performance. The actual effect depends on which membership provider you are using, but in the second case any implementation can take advantage of any internal indexing mechanisms if they are present, and potentially less data needs to be retrieved and transferred because you're only getting at most one MembershipUser due to the paging.

Theoretically, the first option could be faster than the second, but that would mean the membership provider implementation really sucks :)

You appear to be counting the number of members in the collection, but you merely need to know if there is at least one member in the collection. Thus counting is not actually necessary. Also, what is wrong with using the Count property? I'm not saying this for optimization purposes since the impact will be minimal, but I think the intent of your code would be clearer if written that way.

Public Function CheckIFFriendExist2(ByVal arg As String) As Boolean
    Dim foundMembers As MembershipUserCollection = _
        Membership.FindUsersByName(arg, 0, 1, 1)

    Return foundMembers.Count > 0
End Function

Alternatively, use Membership.GetUser to retrieve a single user by name. After all, I wouldn't recommend implementing a membership provider that allows for multiple users with the same name.