1

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();
}
Edward Tanguay
  • 189,012
  • 314
  • 712
  • 1,047

5 Answers5

5

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 Behr
  • 36,951
  • 69
  • 249
  • 387
  • 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 Jul 14 '09 at 09:06
2

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
  • 155,851
  • 29
  • 291
  • 343
  • 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 Jul 14 '09 at 09:02
  • 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 Jul 14 '09 at 09:05
  • Good catch, I didn't notice that. – Martin Harris Jul 14 '09 at 09:07
2
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
  • 263,068
  • 57
  • 365
  • 409
1

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
  • 135,331
  • 41
  • 252
  • 292
0

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
  • 28,277
  • 7
  • 90
  • 101