Kirill writes:
I've worked in this small company for a year, and on a daily basis I've come across things that make my eyes sink back into their sockets in fear, but mostly I've been too busy fixing them to post anything. It being my last day however, here's a classic
We'll take this one in parts. First, every element of the UI the user can navigate to is marked with an enum, defined thus:
enum UiItem
{ SectionA, SectionB, SectionC,...SectionG }
These names are not anonymized, so already I hate it. But it's the next enum that starts my skin crawling:
enum SecurityUiItem
{
SectionA = UiItem.SectionA,
SectionB = UiItem.SectionB,
...
SectionG = UiItem.SectionG
}
A SecurityUiItem
is a different type, but the values are identical to UiItem
.
These enums are used when trying to evaluate role-based permissions for access, and that code looks like this:
if ((currentAccess.ContainsKey(SecurityUiItem.SectionA) && currentAccess[SecurityUiItem.SectionA] != AccessLevel.NoAccess))
return UiItem.SectionA;
else if (!currentAccess.ContainsKey(SecurityUiItem.SectionB)
|| (currentAccess.ContainsKey(SecurityUiItem.SectionB) && currentAccess[SecurityUiItem.SectionB] != AccessLevel.NoAccess))
return UiItem.SectionB;
else if (!currentAccess.ContainsKey(SecurityUiItem.SectionC)
|| (currentAccess.ContainsKey(SecurityUiItem.SectionC) && currentAccess[SecurityUiItem.SectionC] != AccessLevel.NoAccess))
return UiItem.SectionC;
.....
else if (!currentAccess.ContainsKey(SecurityUiItem.SectionG)
|| (currentAccess.ContainsKey(SecurityUiItem.SectionG) && currentAccess[SecurityUiItem.SectionG] != AccessLevel.NoAccess))
return UiItem.SectionG;
else
return UiItem.Unknown;
Honestly, I don't hate the idea of having one data type representing the actual UI objects and a separate data type which represents permissions, and having a function which can map between these two things. But this is a perfect example of a good idea executed poorly.
I also have to wonder about the fall-through pattern. If I have access to SectionA
, I only seem to get SectionA
out of this function. Are these permissions hierarchical? I have no idea, but I suspect there's a WTF underpinning this whole thing.
Congratulations on Kirill's last day.