开发者

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.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜