6
\$\begingroup\$

Can this be shortened to one line?

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

if (areVersionsEqual == false)
  {
    changedDocument.Add(imported); 
   }
\$\endgroup\$
14
\$\begingroup\$

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)
\$\endgroup\$
  • 1
    \$\begingroup\$ This is not technically "one line" though. \$\endgroup\$ – Simon Forsberg Dec 18 '14 at 14:19
  • 6
    \$\begingroup\$ @SimonAndréForsberg no. It's not. You're right, but we're not here to obfuscate code. \$\endgroup\$ – RubberDuck Dec 18 '14 at 14:21
  • 5
    \$\begingroup\$ @user3437721 Why are you asking about regular expressions now suddenly? Something tells me that you haven't provided anywhere near enough context here. What is it that you really want to do? \$\endgroup\$ – Simon Forsberg Dec 18 '14 at 14:29
  • 1
    \$\begingroup\$ @user3437721 You could write one line without an IF statement (at least in JavaScript): documentComparer.Equals(version, imported) && changedDocument.Add(imported);. It depends on the mechanism of short-circuit evaluation. While that being a topic you should know about, it doesn't justify producing a kind of bizarre one-liner. \$\endgroup\$ – ComFreek Dec 18 '14 at 18:42
  • 1
    \$\begingroup\$ @user3437721 since you have two operations your trying to accomplish (determining if the versions are equal & adding the imported document if they are) with the second operation being conditional, a one line solution is impossible or ugly. You could achieve a one-liner in this file by adding a method to changedDocument's class like: changedDocument.AddIfNotEqual(version, imported, documentComparer); \$\endgroup\$ – Sam Berry Dec 18 '14 at 21:59
9
\$\begingroup\$

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.

\$\endgroup\$
  • 1
    \$\begingroup\$ Pedantic comment: in a language which uses true/false (cf yes/no), versionsAreEqual would probably be a better name \$\endgroup\$ – sapi Dec 19 '14 at 8:06
  • \$\begingroup\$ You're right! I didn't really checked the variable name as it was okay. \$\endgroup\$ – IEatBagels Dec 19 '14 at 13:06
1
\$\begingroup\$

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#.

\$\endgroup\$
  • 2
    \$\begingroup\$ Okay. Better. I truly believe you should always use brackets. Big bad bugs happen when you omit them. \$\endgroup\$ – RubberDuck Dec 18 '14 at 16:40
  • \$\begingroup\$ Bracket standards are a never ending argument among developers. If you're working in a professional environment you should always adhere to the standards set forth by your development team. That said, the question was about turning this into a one liner and not about bracketing philosophy. \$\endgroup\$ – ktbiz Dec 19 '14 at 3:15
  • \$\begingroup\$ any and all facets of the code are up for review. We often mention things that don't directly relate to OP's question here. If you don't believe that it's best practice to just always use the brackets, I highly recommend that you read the link I posted. \$\endgroup\$ – RubberDuck Dec 19 '14 at 3:26
-1
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ This works in C#?? I tried real quick but I can't get it to work. Is there doc about this... thing? :p \$\endgroup\$ – IEatBagels Dec 18 '14 at 15:54
  • \$\begingroup\$ That's invalid syntax. The result of the second part after || would have to return a boolean value, and it won't in this case. \$\endgroup\$ – user24822 Dec 18 '14 at 16:42
  • \$\begingroup\$ In Javascript, there is a synthax identical to this one that specifies : If left part == false, execute operation on the right. This is why I asked if it existed in C# \$\endgroup\$ – IEatBagels Dec 18 '14 at 18:18
  • \$\begingroup\$ Oh, drat. My mistake. I thought C# inherited the comma operator from C/C++, but now I see that it doesn't. (I didn't realize I was on MSDN's C++ pages when I looked it up.) \$\endgroup\$ – munificent Dec 22 '14 at 22:01
-5
\$\begingroup\$

You could try a ternary operator

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

crappy? Yes

\$\endgroup\$
  • 2
    \$\begingroup\$ How is this supposed to compile? 1. You can't use the ternary operator as a statement in C#. 2. How is that break supposed to work? What are you breaking out of? \$\endgroup\$ – svick Dec 18 '14 at 15:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.