Implement pattern match on FindUsersByName

Question

I would like some feedback on the code below. And how would I Implement pattern match on find users in role so that exact match not required?

// MembershipProvider.FindUsersByName       
public override MembershipUserCollection FindUsersByName(string usernameToMatch, int pageIndex, int pageSize, out int totalRecords)
{
    MembershipUserCollection users = new MembershipUserCollection();

    try
    {
        Profile.MembershipMapper memberMapper = new MembershipMapper();
        List<Profile.Membership> recs = (List<Profile.Membership>)memberMapper.GetMembershipsByUsername(_memberUtil.GetApplicationId(), usernameToMatch, pageIndex, pageSize, out totalRecords);

        foreach (Profile.Membership rec in recs)
        {
            users.Add(GetUserFromModel(rec, usernameToMatch));
        }
    }
    catch (Exception ex)
    {
        Exception e = CheckEventLog(ex, "FindUsersByName");
        throw e;
    }

    return users;
}

Answer 1

List<Profile.Membership> recs = (List<Profile.Membership>)memberMapper.GetMembershipsByUsername(_memberUtil.GetApplicationId(), usernameToMatch, pageIndex, pageSize, out totalRecords);

I don't understand why is the cast needed here, GetMembershipsByUsername() doesn't sound like a method for which a cast would be necessary.

Also, I think you should consider using var here, especially if you're going to keep the cast, so the type of the variable is clear. (I would probably use var even without the cast, but that's more debatable.)

Exception e = CheckEventLog(ex, "FindUsersByName");
throw e;

I don't see any reason for the (not very well named) variable e here, just throw the exception directly:

throw CheckEventLog(ex, "FindUsersByName");

Also, I think the method name CheckEventLog() could be improved. It's not clear at all that it returns an Exception, or what that exception means.

return users;

I would move this before the end of the try. That way, it's clearer that the method either returns a meaningful result or throws an exception. (And if you changed that later by removing or limiting the throw, you would get a compile error, which I think is a plus.)

Also, doing this means you can move the declaration of the users variable into the try too.