Improve readability of a short snippet while keeping StyleCop happy
The code below looked ok to me when I wrote it, but when I came back to it again, it was pretty hard to grasp what is going on. There used to be parenthesis around value == ...
, but I had to remove them after StyleCop became mandato开发者_如何转开发ry (I cannot really control this). So, how can I improve this section of code? I was thinking: x = value == y ? true : false;
, but that probably is even more confusing, plus silly, although compiler will optimize that.
set
{
Debug.Assert(value == ConfigType.DATABASE || value == ConfigType.FILE,
"Configuration type must be either 'File-based' or 'Database-based'; it was: "
+ value.ToString());
// HG TODO: The following is concise but confusing.
this.fileBasedRadioButton.Checked = value == ConfigType.FILE;
this.databaseBasedRadioButton.Checked = value == ConfigType.DATABASE;
}
bool isFile = value == ConfigType.FILE;
bool isDatabase = value == ConfigType.DATABASE; // or isDatabase = !isFile
Debug.Assert(isFile || isDatabase,
"Configuration type must be either 'File-based' or 'Database-based'; it was: "
+ value.ToString());
this.fileBasedRadioButton.Checked = isFile;
this.databaseBasedRadioButton.Checked = isDatabase;
This makes it a little more readable (explicitly declaring the bool), you know it has to be true or false.
And this way, if you need to (maybe in the future) change settings based on file/database in the same method, you already have the bool handy, instead of checking each time
If you don't want to use the ?:
operator use if..else
. Sure it is a little more verbose, but you wont spend more than a few seconds figuring it out.
A few months from now when you revisit this code you will be glad you took an extra 5 lines.
Making code easy to maintain should be your #1 priority.
if (value == ConfigType.FILE)
this.fileBasedRadioButton.Checked = true;
else
this.fileBasedRadioButton.Checked = false;
if (value == ConfigType.DATABASE)
this.databaseBasedRadioButton.Checked = true;
else
this.databaseBasedRadioButton.Checked = false;
Indent the second and third line of the Debug.Assert()
method. It should then look like this:
Debug.Assert(value == ConfigType.DATABASE || value == ConfigType.FILE,
"Configuration type must be either 'File-based' or 'Database-based'; it was: "
+ value.ToString());
I know this is really a minor stylistic alteration, but I've always found when I have to pass a lot of arguments or have some really long statement, when I carry over to a newline I should indent before the ;
.
It prevents the Debug.Assert()
from looking like 3 lines.
As for the value==
, I agree with the previous poster. You should make a bool
isDatabase
and isFile
to prevent calling a field from ConfigType
twice in your first arg.
精彩评论