User and group management access to main class

Question

UserGroup is not the main class. I don't really mind putting these classes in a class as they are related. I would still like to break them out. Problem I have is if I put internal List<Group> libraryGroups = new List<Group>(); in the main class I don't have access to it from UserGroup. I guess I could pass the main class to UserGroup but not sure if that is a proper approach.

Any other comments on style or other?

public class UserGroup
{
 internal List<User> libraryUsers = new List<User>();
 internal List<Group> libraryGroups = new List<Group>();
 public IEnumerable<User> LibraryUsers { get { return libraryUsers; } }
 public IEnumerable<Group> LibraryGroups { get { return libraryGroups; } }
 public class User
 {
 public override int GetHashCode()
 {
 return ID;
 }
 public override bool Equals(object obj)
 {
 if (obj == null || GetType() != obj.GetType())
 return false;
 User aa = (User)obj;
 return (ID == aa.ID);
 }
 public UInt16 ID { get; }
 public string Name { get; }
 public string Initials { get; }
 public DateTime LastLogOn { get; }
 public Role Role { get; }
 public bool Locked { get; }
 internal List<Group> groups { get; } = new List<Group>();
 public IEnumerable<Group> Groups { get { return groups; } }
 internal User(UInt16 id, string name, string initials, DateTime lastLogOn, Role role, bool locked)
 {
 ID = id;
 Name = name;
 Initials = initials;
 LastLogOn = lastLogOn;
 Locked = locked;
 }
 internal User(UInt16 id, string name, string initials, DateTime lastLogOn, Role role, bool locked, List<Group> _groups)
 {
 ID = id;
 Name = name;
 Initials = initials;
 LastLogOn = lastLogOn;
 Locked = locked;
 groups = _groups;
 }
 internal User(User user)
 {
 ID = user.ID;
 Name = user.Name;
 Initials = user.Initials;
 LastLogOn = user.LastLogOn;
 Locked = user.Locked;
 groups = user.groups;
 }
 }
 public class UserAuthority : User
 {
 //Authority is Read Only or Read Write 
 //It get used by by by properties and documents
 public Authority Authority { get; }
 public UserAuthority(User user, Authority authority)
 : base(user)
 {
 Authority = authority;
 }
 }
 public class Group
 {
 public override int GetHashCode()
 {
 return ID;
 }
 public override bool Equals(object obj)
 {
 if (obj == null || GetType() != obj.GetType())
 return false;
 Group aa = (Group)obj;
 return (ID == aa.ID);
 }
 public UInt16 ID { get; }
 public string Name { get; }
 internal List<User> users { get; } = new List<User>();
 public IEnumerable<User> Users { get { return users; } }
 internal Group(UInt16 id, string name)
 {
 ID = id;
 Name = name;
 }
 internal Group(UInt16 id, string name, List<User> _users)
 {
 ID = id;
 Name = name;
 users = _users;
 }
 internal Group(Group group)
 {
 ID = group.ID;
 Name = group.Name;
 users = group.users;
 }
 }
 public class GroupAuthority : Group
 {
 //Authority is Read Only or Read Write 
 //It get used by by by properties and documents
 public Authority Authority { get; }
 public GroupAuthority(Group group, Authority authority)
 : base(group)
 {
 Authority = authority;
 }
 }
 //User can have only one role but many groups 
 internal struct UserIDGroupID
 {
 public override int GetHashCode()
 {
 return UserID << 16 | GroupID;
 }
 public override bool Equals(object obj)
 {
 if (obj == null || GetType() != obj.GetType())
 return false;
 UserIDGroupID aa = (UserIDGroupID)obj;
 return (UserID == aa.UserID) && (GroupID == aa.GroupID);
 }
 public UInt16 UserID { get; }
 public UInt16 GroupID { get; }
 public UserIDGroupID(UInt16 userID, UInt16 groupID)
 {
 UserID = userID;
 GroupID = groupID;
 }
 }
 internal UserGroup(SDocsServer sDocsServer)
 {
 //no dynamic groups - need to log off and back on to get a fresh set of users and groups
 //same thing if properties are added
 foreach (Group group in sDocsServer.GetGroups())
 {
 Debug.WriteLine(group.Name);
 libraryGroups.Add(group);
 }
 foreach (User user in sDocsServer.GetUsers())
 {
 //Debug.WriteLine(user.Name);
 libraryUsers.Add(user);
 }
 List<UserIDGroupID> userIDsGroupIDs = new List<UserIDGroupID>();
 foreach (UserIDGroupID userIDGroupID in sDocsServer.GetUserIDGroupID())
 {
 userIDsGroupIDs.Add(userIDGroupID);
 //should not have duplicate - trusting server
 }
 foreach (Group group in libraryGroups)
 {
 foreach (UserIDGroupID userIDGroupID in userIDsGroupIDs.Where(x => x.GroupID == group.ID))
 { 
 group.users.Add(libraryUsers.FirstOrDefault(x => x.ID == userIDGroupID.UserID));
 }
 }
 foreach (User user in libraryUsers)
 {
 foreach (UserIDGroupID userIDGroupID in userIDsGroupIDs.Where(x => x.UserID == user.ID))
 {
 user.groups.Add(libraryGroups.FirstOrDefault(x => x.ID == userIDGroupID.GroupID));
 }
 }
 }
}

• Pieter Witvoet: You talk about access to (from?) a main class, but you haven't included it in your post...
• paparazzo: @PieterWitvoet I don't get why you would need the main class. You can assume it is nothing but internal List<Group> libraryGroups = new List<Group>()
• Pieter Witvoet: You're asking about how two classes should interact, but then only show one of them. If we don't know the purpose of that other class, then how can those questions be answered?
• Pieter Witvoet: I get all that. What I don't get is why you want to move a list from UserGroup to the main class, and why you'd then want to pass the main class to UserGroup. Showing how these two classes currently interact provides some context and could make it easier to tell what exactly you want to achieve. And that's very useful to know when reviewing code.
• Pieter Witvoet: You've said that multiple times now, even in the post itself, but it's still not clear why you want to do that (what's the purpose of that list?) and what exactly the problem is (why would UserGroup need to access the list in your startup class?).

Answer 1

You've been already told in your previous post that you should use ushort instead of UInt16. Although it is completely up to you using of ushort is common practice.

---

If purpose of this stuff

internal List<User> libraryUsers = new List<User>();
internal List<Group> libraryGroups = new List<Group>();
public IEnumerable<User> LibraryUsers { get { return libraryUsers; } }
public IEnumerable<Group> LibraryGroups { get { return libraryGroups; } }

is to prevent changing of libraryUsers and libraryGroups outside of your assembly then you should use something like this:

public IEnumerable<User> LibraryUsers => libraryUsers.AsReadOnly();

because LibraryUsers can be downcasted to List<User> and be changed.

---

I'm still insisting you should use empty lines between class members.

---

This

internal List<Group> groups { get; } = new List<Group>();

is a property so name should be PascalCased.

---

List<T> has a great method named AddRange. Use it everytime your hands are trying to write code like this:

foreach (User user in sDocsServer.GetUsers())
{
 libraryUsers.Add(user);
}

With the method mentioned above it will be

libraryUsers.AddRange(sDocsServer.GetUsers());

---

I have feeling that the architecture of your code is not good...

1. I don't understand how UserAuthority can be a subclass of User. Authority is an attribute of user and not user itself. The same applies to GroupAuthority.
2. In my opinion it is bad to keep users related to specific group inside the group. You should take a look at databases developing where there is the first normal form according to which you need separate objects that will connect User and Group. And I see you have the class for this – UserIDGroupID. So proceed completely with it removing list of users from the Group.
3. The previous point in fact is even more important since groups contains related users and users contain related groups... In my opinion it not just can be reorganized, it have to be.

• Pieter Witvoet: Regarding #2 and #3: a reference (or list of references) does not necessarily imply a 'contains-a' or 'owns-a' relationship. In this case, it models a 'belongs-to' relationship. Removing those lists in favor of directly exposing a mapping struct would make this code much more cumbersome to use.
• paparazzo: Well if you i n s i s t s.

Answer 2

I am just passing users and groups back to main
I like not having users and groups depend on main
They could very well be used with other main for example a web page

public class User
{
 public override int GetHashCode()
 {
 return ID;
 }
 public override bool Equals(object obj)
 {
 if (obj == null || GetType() != obj.GetType())
 return false;
 User aa = (User)obj;
 return (ID == aa.ID);
 }
 public UInt16 ID { get; }
 public string Name { get; }
 public string Initials { get; }
 public DateTime LastLogOn { get; }
 public Role Role { get; }
 public bool Locked { get; }
 internal List<Group> groups { get; } = new List<Group>();
 public IEnumerable<Group> Groups => groups.AsReadOnly().OrderBy(x => x.Name);
 internal User(UInt16 id, string name, string initials, DateTime lastLogOn, Role role, bool locked)
 {
 ID = id;
 Name = name;
 Initials = initials;
 LastLogOn = lastLogOn;
 Role = role;
 Locked = locked;
 }
 internal User(UInt16 id, string name, string initials, DateTime lastLogOn, Role role, bool locked, List<Group> _groups)
 {
 ID = id;
 Name = name;
 Initials = initials;
 LastLogOn = lastLogOn;
 Locked = locked;
 groups = _groups;
 }
 internal User(User user)
 {
 ID = user.ID;
 Name = user.Name;
 Initials = user.Initials;
 LastLogOn = user.LastLogOn;
 Locked = user.Locked;
 groups = user.groups;
 }
}
public class UserAuthority : User
{
 //Authority is Read Only or Read Write 
 //It is used by by properties and documents
 public Authority Authority { get; }
 public UserAuthority(User user, Authority authority)
 : base(user)
 {
 Authority = authority;
 //this may come from one of their groups
 }
}
public class Group
{
 public override int GetHashCode()
 {
 return ID;
 }
 public override bool Equals(object obj)
 {
 if (obj == null || GetType() != obj.GetType())
 return false;
 Group aa = (Group)obj;
 return (ID == aa.ID);
 }
 public UInt16 ID { get; }
 public string Name { get; }
 internal List<User> users { get; } = new List<User>();
 public IEnumerable<User> Users => users.AsReadOnly().OrderBy(x => x.Name);
 internal Group(UInt16 id, string name)
 {
 ID = id;
 Name = name;
 }
 internal Group(UInt16 id, string name, List<User> _users)
 {
 ID = id;
 Name = name;
 users = _users;
 }
 internal Group(Group group)
 {
 ID = group.ID;
 Name = group.Name;
 users = group.users;
 }
}
public class GroupAuthority : Group
{
 //Authority is Read Only or Read Write 
 public Authority Authority { get; }
 public GroupAuthority(Group group, Authority authority)
 : base(group)
 {
 Authority = authority;
 }
}
//User can have only one role but many groups 
internal struct UserIDGroupID
{
 public override int GetHashCode()
 {
 return UserID << 16 | GroupID;
 }
 public override bool Equals(object obj)
 {
 if (obj == null || GetType() != obj.GetType())
 return false;
 UserIDGroupID aa = (UserIDGroupID)obj;
 return (UserID == aa.UserID) && (GroupID == aa.GroupID);
 }
 public UInt16 UserID { get; }
 public UInt16 GroupID { get; }
 public UserIDGroupID(UInt16 userID, UInt16 groupID)
 {
 UserID = userID;
 GroupID = groupID;
 }
}
internal class UserGroup
{
 internal void GetUsersGroups(out List<User> users, out List<Group> groups, SDocsServer sDocsServer)
 {
 users = new List<User>();
 groups = new List<Group>();
 groups.AddRange(sDocsServer.GetGroups());
 users.AddRange(sDocsServer.GetUsers());
 List<UserIDGroupID> userIDsGroupIDs = new List<UserIDGroupID>();
 userIDsGroupIDs.AddRange(sDocsServer.GetUserIDGroupID());
 foreach (Group group in groups)
 {
 //Debug.WriteLine($"{group.ID} {group.Name}");
 foreach (UserIDGroupID userIDGroupID in userIDsGroupIDs.Where(x => x.GroupID == group.ID))
 {
 group.users.Add(users.FirstOrDefault(x => x.ID == userIDGroupID.UserID));
 }
 }
 foreach (User user in users)
 {
 //Debug.WriteLine($"{user.ID} {user.Name}");
 foreach (UserIDGroupID userIDGroupID in userIDsGroupIDs.Where(x => x.UserID == user.ID))
 {
 user.groups.Add(groups.FirstOrDefault(x => x.ID == userIDGroupID.GroupID));
 }
 }
 }
 internal UserGroup()
 { }
}
internal class UserGroupTest
{
 internal UserGroupTest(SDocsServer sDocsServer)
 {
 UserGroup userGroup = new UserGroup();
 List<User> users = new List<User>();
 List<Group> groups = new List<Group>();
 userGroup.GetUsersGroups(out users, out groups, sDocsServer);
 foreach (User user in users)
 {
 Debug.WriteLine($"{user.Name} {user.Role}");
 foreach (Group group in user.Groups)
 {
 Debug.WriteLine($" {group.Name}");
 }
 }
 foreach (Group group in groups)
 {
 Debug.WriteLine($"{group.Name}");
 foreach (User user in group.Users)
 {
 Debug.WriteLine($" {user.Name}");
 }
 }
 Debug.WriteLine($"done");
 }
}

• Pieter Witvoet: I get the impression that you're overlooking some basic programming techniques, which causes you to see a problem where in reality there isn't a problem at all. The UserGroup class in your opening post already had two properties: LibraryUsers and LibraryGroups. Your main or test code could simply access those properties: foreach (Group group in userGroup.LibraryGroups) { ... }.
• paparazzo: @PieterWitvoet I know I could access them userGroup.LibraryGroups. I don't want to access it that way. I want List<User> Users in my main. I don't want UserGroup in my main as to me that is an implementation detail I want to hide.
• Pieter Witvoet: Then why even have a separate UserGroup class at all? Oh well, if you want to make things more complicated than they need to be, go ahead...
• paparazzo: @PieterWitvoet It is a struct. Because that is the fastest database load with minimum volume. That is exactly one table int the database.