Is there any way to improve this kind of code? I have converted this from production code for privacy reasons, so there might be some mistakes.public class AddNewUserDto{    public bool IsAdmin { get; set; }    public bool IsRead { get; set; }    public bool IsWrite { get; set; }    public string[] GetRolesByDto()    {        List<string> roles = new List<string>();        if (this.IsAdmin)        {            roles.Add(USER_ROLES.ADMIN);        }        if (this.IsRead)        {            roles.Add(USER_ROLES.IsRead);        }        if (this.IsWrite)        {            roles.Add(USER_ROLES.IsWrite);        }        if (this.IsEntityUser)        {            roles.Add(USER_ROLES.IsEntityUser);        }        return roles.ToArray();    }}public static class USER_ROLES{    public const string IsAdmin= Admin;    public const string IsRead= Read;    public const string IsWrite= Write;    public const string IsEntityUser = EntityUser;}
Add string to array based on flag
c#
strings are probably the wrong choice to represent roles. You lose the power of the type system -- the compiler won't pick up on typos, e.g. Amdin. Consider using an enum instead.Be consistent with your naming. Why is it USER_ROLES.IsRead and USER_ROLES.Write (not IsWrite)? Stick to the standard naming conventions, e.g. write UserRole.IsRead instead of USER_ROLES.IsRead.Does the calling code really need an array returned? It would probably be fine with an IEnumerable.If we change the return type to IEnumerable<UserRole>, we can writepublic IEnumerable<UserRole> GetRoles(){    if (this.IsAdmin)    {        yield return UserRole.Admin;    }    if (this.IsRead)    {        yield return UserRole.Read;    }    if (this.IsWrite)    {        yield return UserRole.Write;    }    if (this.IsEntityUser)    {        yield return UserRole.EntityUser;    }}But I'm guessing you're asking more about the repetition. Here's one way to get around it -- create an association between the booleans and the values. Here I've used a dictionary; it's possibly a bit of overkill but it does give us the nice initialiser syntax:public IEnumerable<UserRole> GetRoles(){    var roles = new Dictionary<UserRole, bool>    {        { UserRole.Admin, this.IsAdmin },        { UserRole.Read, this.IsRead },        { UserRole.Write, this.IsWrite },        { UserRole.EntityUser, this.IsEntityUser }    };    return roles.Where(role => role.Value)                .Select(role => role.Key);}Other options would be an array of KeyValuePairs, an array of Tuples, etc.