Shortening boolean with if statement for document version comparison

Question

Can this be shortened to one line?

 bool areVersionsEqual = documentComparer.Equals(version, imported);

if (areVersionsEqual == false)
  {
    changedDocument.Add(imported); 
   }

Answer 1

Sure. Use the Not operator.

if (!documentComparer.Equals(version, imported))
{
    changedDocument.Add(imported); 
}

---

To explain a little, it's bad practice to do things like this.

bool boolean = true;
if (boolean == false)
{
//...

because it's completely equivalent to saying

if (true == false)

and can be simplified down to

if (false)

Answer 2

I believe it could be a one line, but I don't think you should.

One liner's are kind of "cool" to write, but they are a pain to debug, and a pain to read.

I think you should keep your variable, it is great for readability.

What I would change though is the indentation of your if. Bring those brackets to be on the same line as the if.

And, as @RubberDuck pointed, you should use the ! operator instead of == false.

bool areVersionsEqual = documentComparer.Equals(version, imported);

if (!areVersionsEqual)
{
    changedDocument.Add(imported); 
}

I think this looks better then if you do it on one line.

Answer 3

You can make this one line by moving the boolean statement into the conditional and removing the unnecessary brackets (You don't need brackets with a single line conditional). This code is perfectly legal syntax:

if (!documentComparer.Equals(version, imported)) changedDocument.Add(imported);

I consider it good style to remove brackets when they're not necessary. It saves vertical space. Note that the length of this line is 79 characters. When you use a shell, the default line width is 80, any more than that and your lines start wrapping which makes it difficult to read. Nominally I'm okay with anything 80 characters or less. Some people prefer 76 characters because when you're showing line numbers in an editor they take up the first 4 characters, leaving you with only 76 characters until the line wraps. If you want to make everyone happy, I would write it like this:

if (!documentComparer.Equals(version, imported)) 
    changedDocument.Add(imported);

You could also include your brackets in a single line conditional like this:

if (!documentComparer.Equals(version, imported)) { changedDocument.Add(imported); }

Of course this brings you up to 83 characters so I would curse you to programming hell!

Do not use a ternary operator. It's not necessary and it only serves to confuse the code, as there is no else statement.

Also do not use a short circuited logical operator to turn this into a one liner. It's confusing and it wouldn't even be syntactically correct in C#.

Answer 4

Technically, yes you can make this a single expression:

documentComparer.Equals(version, imported) || (changedDocument.Add(imported), true);

Don't do that, though. :) Instead, I would take RubberDuck's solution.

Answer 5

You could try a ternary operator

documentComparer.Equals(version, imported) == false ?  changedDocument.Add(imported) : break; 

crappy? Yes