A bad way to set up commands in your view model

tl;dr; Put code where it is defined. It makes maintenance much easier.

This has become a bit of a pet-peeve of mine lately. Please indulge me while I share.

I spend a lot of time working with code that was started by someone else.
As MVVM Light is used in most projects I work on, I see code that looks like this a lot:

    public class MyViewModel : ViewModelBase
    {
        public MyViewModel()
        {
            MyCommand = new RelayCommand(() =>
            {
               // some functionality here
            });
        }
        
        public RelayCommand MyCommand { get; private set; }
    }

This bugs me.
Seeing this just makes me think that the original developer was planning on allowing for the possibility that they may want to change what the command does at some point in the future.
But I haven't seen any code that actually does that, yet.

Remember Y.A.G.N.I.
I think this is much better:

    public class MyViewModel : ViewModelBase
    {
        public MViewyModel()
        {
        }
        
        public RelayCommand MyCommand 
        { 
            get
            {
                return new RelayCommand(() =>
                {
                    // some functionality here
                });
            }
        }
    }

 

And you probably don't really need the empty constructor. (I know in this instance you don't but depending on what else is being done in the VM you might.)
There is no difference in the actual number of lines of code or functionality but it's much better.

Why do I think this is better?

It puts the actual code where it is defined and so when I identify the command I'm after I can see what it does.
Putting the code where it is defined means it's easier to navigate the code when you aren't familiar with it.

Let's say I need to change (or fix) some behaviour that is triggered via a command. I'll start in the view (the UI) which uses/triggers that command. From there I can easily see where the command is defined (Ctrl + Click : with ReSharper). That's good, but if the code isn't there I have to search the code ("Find Usages" or "Find References") and then go to where I think it's most likely to be based on what I can see in the search results.
I've had to, unnecessarily, go hunting around the code for what I want.
The code I'm looking for has been hidden!
That's not a good way to treat the people who will be maintaining the code you wrote.

Not wanting to make any claims about my mental health and disposition but:

"Always code as if the person who ends up maintaining your code is a violent psychopath who knows where you live."

Am I missing something by doing this my way?
Does the maintainability benefit come at some other cost?

Obviously there are exceptions to the above and times when you may not want to have a readonly command. My point is to think about the code you're writing and the people who will be maintaining it and trying to work out what it does. The easier you can make this for them the less frustrated with you they'll get. :)