tags:

views:

358

answers:

5

The method below called UserCanAccessThisPage is based on the following logic: each user and each page has a list of groups. If any of these match, the user has access to the page.

The code below does what I want, but my solution is very C# 1 (or C# 2, at least I didn't use ArrayList).

Can anyone refactor this so it is more straight-forward, e.g. using lambdas to do away with the two methods? I just can't get the syntax to do it.

using System;
using System.Collections.Generic;
using System.Linq;

namespace TestCompare2343
{
    class Program
    {
        static void Main(string[] args)
        {

            string anonymousUserAccessGroups = "loggedOutUsers";
            string normalUserAccessGroups = "loggedInUsers, members";
            string developerUserAccessGroups = "loggedInUsers, members, administrators, developers";

            string loginPageAccessGroups = "loggedOutUsers";
            string logoutPageAccessGroups = "loggedInUsers";
            string memberInfoPageAccessGroups = "members";
            string devPageAccessGroups = "developers";

            //test anonymousUser
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(anonymousUserAccessGroups, loginPageAccessGroups));
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(anonymousUserAccessGroups, logoutPageAccessGroups));
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(anonymousUserAccessGroups, memberInfoPageAccessGroups));
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(anonymousUserAccessGroups, devPageAccessGroups));
            Console.WriteLine("---");

            //test anonymousUser
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(normalUserAccessGroups, loginPageAccessGroups));
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(normalUserAccessGroups, logoutPageAccessGroups));
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(normalUserAccessGroups, memberInfoPageAccessGroups));
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(normalUserAccessGroups, devPageAccessGroups));
            Console.WriteLine("---");

            //test anonymousUser
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(developerUserAccessGroups, loginPageAccessGroups));
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(developerUserAccessGroups, logoutPageAccessGroups));
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(developerUserAccessGroups, memberInfoPageAccessGroups));
            Console.WriteLine(StringHelpers.UserCanAccessThisPage(developerUserAccessGroups, devPageAccessGroups));
            Console.WriteLine("---");

            Console.ReadLine();

        }
    }

    public class StringHelpers
    {
        public static bool UserCanAccessThisPage(string userAccessGroups, string pageItemAccessGroups)
        {
            List<string> userAccessGroupsList = StringHelpers.SplitAndTrimCommaDelimitedString(userAccessGroups);
            List<string> pageItemAccessGroupList = StringHelpers.SplitAndTrimCommaDelimitedString(pageItemAccessGroups);

            foreach (string userAccessGroup in userAccessGroupsList)
            {
                foreach (string pageItemAccessGroup in pageItemAccessGroupList)
                {
                    if (userAccessGroup == pageItemAccessGroup)
                        return true;
                }
            }

            return false;
        }

        public static List<string> SplitAndTrimCommaDelimitedString(string line)
        {
            List<string> piecesWithSpaces = line.Split(',').ToList<string>();
            List<string> piecesWithoutSpaces = new List<string>();
            foreach (string pieceWithSpace in piecesWithSpaces)
            {
                piecesWithoutSpaces.Add(pieceWithSpace.Trim());
            }
            return piecesWithoutSpaces;
        }
    }
}

Answer:

Fredrik had the most concise code that solved the original task above:

public static bool UserCanAccessThisPage(string userAccessGroups, string pageItemAccessGroups)
{
    return userAccessGroups
        .Split(',')
        .Select(s => s.Trim())
        .Contains(pageItemAccessGroups);
}

The code I used:

But Shaul was correct in assuming that the PageItems can also have multiple entries, e.g. "members,guests", and so I actually used Shaul's code:

public static bool UserCanAccessThisPage(string userAccessGroups, string pageItemAccessGroups) {
  List<string> userAccessGroupsList = StringHelpers.SplitAndTrimCommaDelimitedString(userAccessGroups);
  List<string> pageItemAccessGroupList = StringHelpers.SplitAndTrimCommaDelimitedString(pageItemAccessGroups);
  return userAccessGroupsList.Any(userAccessGroup => pageItemAccessGroupList.Any(pageItemAccessGroup => userAccessGroup == pageItemAccessGroup));
}

public static List<string> SplitAndTrimCommaDelimitedString(string line) {
  return line.Split(',').Select(s => s.Trim()).ToList();
}
+5  A: 

OK, you said you wanted compact...! :)

public static bool UserCanAccessThisPage(string userAccessGroups, string pageItemAccessGroups) {
  List<string> userAccessGroupsList = StringHelpers.SplitAndTrimCommaDelimitedString(userAccessGroups);
  List<string> pageItemAccessGroupList = StringHelpers.SplitAndTrimCommaDelimitedString(pageItemAccessGroups);
  return userAccessGroupsList.Any(userAccessGroup => pageItemAccessGroupList.Any(pageItemAccessGroup => userAccessGroup == pageItemAccessGroup));
  // or:
  // return userAccessGroupsList.Any(userAccessGroup => pageItemAccessGroupList.Contains(userAccessGroup));
}

public static List<string> SplitAndTrimCommaDelimitedString(string line) {
  return line.Split(',').Select(s => s.Trim()).ToList();
}
Shaul
I think your second return statement is far more legible than the first and the use of Any (which I completely forgot about) makes it faster than mine. Good answer.
Martin Harris
Thanks, bro. :)
Shaul
A: 

Something like (sorry no IDE to hand)

public static bool UserCanAccessThisPage(string userAccessGroups, string pageItemAccessGroups)
{
    List<string> userAccessGroupsList = StringHelpers.SplitAndTrimCommaDelimitedString(userAccessGroups);
    List<string> pageItemAccessGroupList = StringHelpers.SplitAndTrimCommaDelimitedString(pageItemAccessGroups);

    return userAccessGroupsList.Where(
                          uag => pageItemAccessGroupList.Contains(uag)).Count > 0;
}

public static List<string> SplitAndTrimCommaDelimitedString(string line)
{
    return line.Split(',').Select(s => s.Trim()).ToList<string>();
}

I really don't see a big problem with it having two methods, since the functionality is nicely separated, but I guess you could move the code from the second method inline to the first if you really want to.

Martin Harris
+2  A: 

StringHelpers could look like this:

public class StringHelpers
{
    private static readonly char[] separator = ",".ToCharArray();
    public static bool UserCanAccessThisPage(
        string userAccessGroups, 
        string pageItemAccessGroups)
    {
        return userAccessGroups
            .Split(separator) // split on comma
            .Select(s => s.Trim()) // trim elements
            .Contains(pageItemAccessGroups); // match
    }
}
Fredrik Mörk
I'm not sure that'll work since pageItemAccessGroups is also a comma delimited string, but like I say in my answer I don't have an IDE to hand right now to check.
Martin Harris
I noticed that the original implementation splits pageItemAccessGroups as well, but no sample data contains a comma, and it is never tested for. So I employed a test driven approach; the code passes the outlined test ;o)
Fredrik Mörk
Good catch, I didn't notice that.
Martin Harris
+1  A: 

Once you have the two IEnumerable<string>s, you can use the Intersect function:

return userGroups.Intersect(pageGroups).Count > 0;

This is in case you need the full list of permissions for the user on the page.
However, I'd go with Shaul's example: the Any function is faster, it should stop at the first match.

Kobi
Hey, I didn't know about the Intersect() function - +1 for teching me a new trick! תודה רבה!
Shaul
+1  A: 
public static bool UserCanAccessThisPage(
    string userAccessGroups, string pageItemAccessGroups)
{
    HashSet<string> u = new HashSet<string>(
        userAccessGroups.Split(',').Select(x => x.Trim()));
    return u.Overlaps(pageItemAccessGroups.Split(',').Select(x => x.Trim()));
}
LukeH