Introduce constants for 'magic strings' in ManageController.

[dangle1]

Forgive me if this is an insignificant number of changes for a single pull-request. Lot of firsts for me here: contributing to an open source project, using git and github, so I just wanted to make sure I get the workflow down before I start making bigger changes.

Please let me know if I should come back with a different pull-request when there is more to review.
#340

[mheggeseth]

I ❤️ small PRs

[mheggeseth]

This is a great start, but I think you could take string abstraction a little further in ManageConrroller. There are a few more strings that could use constants.

[dangle1]

Okay, something like creating a constant for every string, similar to EMAIL_CONFIRMATION_SUBJECT ?

Or something more robust for things like Controller names? Keep in mind nameof() won't work with Controller classes as MVC expects the "Controller" part of the name to be removed when being passed around.

Otherwise, I can list all the strings throughout ManageController as consts fields.

[mheggeseth]

I was thinking a constant for each string

[dangle1]

Sure thing! Thanks for the feedback and I'll get back to implementing it now.

[dangle1]

@mheggeseth each string has been constant-fied. Please let me know what you think.

[tonysurma]

Once @mheggeseth does a review I can merge it in. thx

[mheggeseth]

You should wrap the href values in literal quotes (at least to be consistent with what was there before): <a href=\"{0}\">link</a>

[dangle1]

Sure, thanks for catching that.

[dangle1]

@mheggeseth literal quotes added as per your feedback.

[mheggeseth]

Looks good

[mgmccarthy]

@mheggeseth @tonysurma @BillWagner

Why are we changing strings to constants in files that only use the constant once?

IMO, it's easier on the eyes to "read a human readable string" then a BUNCH_OF_CONSTANT_NAMES in a file.. I can see where a constant comes in handy when it's used more than once in a given file, but if it's used ONLY once, what's the benefit?

I understood Issue #340 to be related to getting rid of magic strings for things like using the new nameof operator, that would give us compile time checks against items being renamed in the system instead of relying on runtime failures. To me, we get value out of a change like that.

Unless I'm missing something

[mheggeseth]

@mgmccarthy I don't feel strongly about it. Personally, I probably wouldn't have made constants for the single-use strings. Having to jump to another place in the file to read the string value is a little annoying but having a constant for every string serves as a good reminder to consider refactoring strings. It also makes it easier to do so when a single-use string becomes multi-use.