Interesting items, but...

Oct 19, 2010 at 4:42 PM

Interesting items, but you seem to be a bit shaky on your algorithms.   And, you seem never really sure if you want  KeyCharacter to be a char, int or string.  I'd suggest that you study up on the methods of the Char class, particular "IsDigit()".  Using it, you can convert you current 2-parameter AddMenuChoice method (which presently converts each char KeyCharacter into a string and then parse it into an int -- three time each!), to just:

	char keyCharVal = '1';
if (MenuChoices.Any(mc => Char.IsDigit(mc.KeyCharacter)))
{
var keyChar = MenuChoices.Where(mc=>Char.IsDigit(mc.KeyCharacter)).Max(mc=>mc.KeyCharacter);
keyCharVal = (char) ((keyChar - '0') + '1');
}
return AddMenuChoice(action, description, keyCharVal);

 I'll admit the math of Chars is a bit odd, but it does work.   To go a bit more extreme, that method can be reduced to just two lines:

	char keyCharVal = "123456789".Except(MenuChoices.Select(mc => mc.KeyCharacter)).First();
return AddMenuChoice(action, description, keyCharVal);

i.e., take the first item in that string (which here we can consider to be a IList<Char>) that is not among the KeyCharacters in MenuChoices.   Note: If all 9 digit choices are already used, they will throw an Exception.   The original would also, but it would be a different exception.

Similarly, the three parameter AddMenuChoice() can be reduced to :

	keyCharacter = Char.ToUpper(keyCharacter);
if (MenuChoices.Any(mc => mc.KeyCharacter == keyCharacter))
throw new ArgumentException("The keyCharacter specified has already been assigned: " + keyCharacter.ToString());

if (keyCharacter == this.ConsoleMenuOptions.EscapeCharacterMenuOption)
throw new ArgumentException("The keyCharacter specified is being used as the escape character menu option");

this.MenuChoices.Add(new MenuChoice(action, description, keyCharacter));
return this;
Here, if we always force KeyCharacter to upper case before saving it, we never have to worry about it being anything else later.
Coordinator
Oct 19, 2010 at 6:16 PM

Thanks for your feedback.  The way that menu choices are added and how their keyCharacters are generated certainly needs some improvement.  I had begun work to try to think through a new way to handle the menu choices list in changeset e14b64a14164, but I backed out those changes since I didn't finish implementing the change and wanted to release something that I knew would work, albeit in a limited way..

My plan is to refactor the code so that the AddMenuChoice() doesn't assign the key character value.  The numbering will be generated in a separate area of the code.  I mainly want to make this change to more easily accomodate paging; e.g. if you have a list of 29 items and your page size is 10, you'll first see a page of items labeled 0-9, then the next "page" will be labeled 0-9, etc.  This only works if I don't assign the character value when the menu choice is added.

I do appreciate your suggestions about enhancing my use of the Char class.  I'm so used to looking for extension methods off the object, I probably tried keyChar.ToUpper() and when it didn't work, I probably figured I'd just convert it to a string and move on.  Your way seems a lot more efficient and I'll definately think of that as I move forward in development.