Coding Conventions for if statements and variables

Hey,

I just got ReSharper for Visual Studio and it was recommending me some changes to my code.

First of all it recommended me to use

var cosDeg = Mathf.Cos((transform.eulerAngles.y * Mathf.PI) / 180);

instead of

float cosDeg = Mathf.Cos((transform.eulerAngles.y * Mathf.PI) / 180);

The reason is that it is ‘clear’ that cosDeg is a float, but I don’t really think that’s really the case here. I’m inclined to only use var for things like thisvar test = new Vector3();

Secondly it recommended to use this

void PrintName(Person p)
{
  if (p == null) return
  if (p.Name == null) return;
  Console.WriteLine(p.Name);
}

instead of

void PrintName(Person p)
{
  if (p != null)
  {
    if (p.Name != null)
    {
      Console.WriteLine(p.Name);
    }
  }
}

I’d very much like the thoughts of other programmers on this. I’m sure there are some people here that have been coding for quite some time that have an opinion on this.

  • Alex

I like to keep my do nothing code short and early, so I’d have written this one to use short circuiting:

void PrintName(Person p)
{
  if (p == null || p.Name == null) return;
  Console.WriteLine(p.Name);
}

I don’t tend to use var and would have put float too. Maybe it is obvious what var is from the context, but I have to read a lot more to know that than glancing at ‘float’ in the obvious place.

1 Like

Alright that’s of course even better! I just never thought of inversing the if-statement that way to avoid code nesting, but that piece of code is very readable, so I think I’m going to let ReSharper do what it wants :slight_smile:

As to the use of ‘var’ keyword, I think it’s been a trend these days, with more statically typed languages like Kotlin or Scala adopting such a concept. Even Java - the quintessential verbose and ugly static typed OOP language that everyone loves to criticize :stuck_out_tongue: - is considering to follow suit with its JEP-286.

I guess it has something to do with popularity of IDEs these days. Almost everyone uses one when they write code in such statically typed languages, so they can easily find out the exact type of such variables without having to explicitly write them in their code.

And probably, they might have gotten tired of those dynamic language hipsters keep telling them how awfully cumbersome to having to write down type names every time they declare a variable.

I’m also using Rider (which is based on ReSharper) as my main IDE, and basically learned to code in C# by reading its inspection warning messages. As things like how to write a return statement is determined by a convention, I just assumed those who make a living by writing a development tool would know of what majority of developers who use that language think, far better than myself.

So, I also try to do what my IDE recommends most of the times.

1 Like

Both recommendations are reasonable. THere’s no reason to enter type manually when it can be guessed from the expression. Likewise, there’s no reason to use {} in “if” statement, when “if” only controls one line of code.

I would put “return” on its own line, but otherwise this modification is how I’d write this kind of function myself. Rather than nesting blocks, you can just return early when required conditiosn are not met.

2 Likes

Yes, it really is the case with that example. For starters, the “f” in Mathf literally stands for “float” and every method of that class returns one. Additionally keep in mind that C# won’t implicitly cast a float to anything other than a double as to do so would lose data.

https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/implicit-numeric-conversions-table

1 Like

Alright thanks!!

Yes, agree! I was wondering why I should var if I could just write float. I guess ultimately it’s the same but I’ll let my IDE handle things automatically. ReSharper gives a warning thought for the Start() and Update() functions since they ‘are not used anywhere’. I’ll have to fix that.

Ah. Whoops. I read your post backwards for some reason. :sweat_smile:

I did discover a Stack Overflow discussion about it though. Some of them just turn that “feature” off.

https://stackoverflow.com/questions/1873873/why-does-resharper-want-to-use-var-for-everything

Thanks! I guess in the example with the dict, using var is indeed the way to go but for basic things like bools and floats, maybe I’ll just stick to writing those few characters more :stuck_out_tongue: I think it’s more readable since I don’t have to look at the rest of the line to see what type I’m working with.

Me too… I missed it when doing a quick copy paste job :slight_smile:

@Ryiah the var thing is obvious in hindsight, but at the beginning of the line when I know I need a float but haven’t yet typed that = sign and thought of what comes next, I’d type my current thought of float, rather than postponing that decision by typing var. It’d also help if later I forgot to type the f at the end of Math.

1 Like

I’d use this instead:

var cosRad = Mathf.Cos(transform.eulerAngles.y * Mathf.Deg2Rad);

I’d use Mathf.Deg2Rad rather than (angle*Mathf.PI)/180 to make the intend of that conversion more obvious/explicit. Mathf.Cos returns the cosine of the specified angle is radians, therefore I would drop the “Deg(ree)” in the variable name “cosDeg” or replace it with “Rad(ians)”.

3 Likes

The main advantage of Var over Type is because var allows for fixed width variable declarations.

var x = 1;
var y = new Thing();

the variable names are visually aligned.

float x = 1;
int y = 2;

variable names are staggered, which gives the impression of messy code. This is the area where learning a bit of basic typography dovetails with coding convention.

I tend to prefer column aligned assignment and declaration though when reasonable:

float x = 1;
int   y = 2;

In general, unless dealing with really obscene generic declarations I prefer writing out types in most cases despite the loss in clarity - but I totally respect that most others don’t share this preference.

In terms of conditionals, I agree with resharpers preference. It’s far, far easier to read statements with less indenting

I also prefer error/empty conditions to be presented at the top of a function - again for clarity.

function x(){
 // error checking / validation / empty return

 // gather whatever data needed outside of parameters and members

 // perform transformations

 // massage for return
}

The above is my basic template for every function/method. If I need to do more error checking/validation after gathering additional data, it usually means the function needs to be refactored.

5 Likes

Very useful, thanks! And using var for readability is something I didn’t think of before, thanks!

Its worth noting that resharper has an option to change warning/suggestion severity. So you can turn stuff on and off. I deactivate the ‘var’ suggestion myself since I prefer types written out. Every suggestion can be configured from ignore to error.

You can also configure naming conventions (prefix, suffix, etc), and even put together templates for reformatting classes. Resharper is pretty amazing.

1 Like

Knowing how the dev community works there is probably an extension to reshaper out there to specifically work with Unity and its magic methods.

1 Like

Unless there is some clear statement of type in the assignment, I find var to be stupid and ends up requiring more work to understand the code.

3 Likes

I’m in this boat.

I tend to use var only when im indicating that I really don’t give two hoots about the type of the variable. Which is pretty useful with some of the more exotic plugins I’ve used.

Resharper has a set of tags that can be used to document code (for IDE).

[UsedImplicitly] is a tag that tells resharper that the method is involved without explicit use, you can also define custom tags for this. I made a [UnityMethod] since I hated the look of “UsedImplicitly” and wanted something more explicit.

There’s other stuff like [CanBeNull] or full on contract annotations, that allow for smarter resharper feedback.

CanBeNull (and similar) is very useful since it can provide reminders to do null checks and the like where needed.

[Pure] is also nice to use.

Worth noting that Unity actually distributes a version of the resharper annotations dll you can use without custom imports, although it’s an older version iirc.

1 Like

Not a fan of aligning anything horizontally with spaces. This kind of formatting tends to break horribly when the code is displayed in non-monospace font.

The ‘var’ keyword can be especially useful if you use generics a lot.

For example, just compare this:

IDictionary<string, IList<string>> itemMap = new Dictionary<string, IList<string>>();

with this:

var itemMap = new Dictionary<string, IList<string>>();
4 Likes