Correct usage of MVVM and object for method

Question

I'm writing my first C# Windows Store app and am learning C# from scratch. I'm trying to implement MVVM as I understand it and object orientated patterns, obviously though I'm expecting to be doing it wrong so looking for pointers as to what I should improve.

Below is a method that gets called when a user either clicks a 'Search' XAML control, or hits return in the search textbox control.

The comments should explain the code. I feel like I probably have too much code in for a event method so please advise how I would restructure it.

  private void SubmitSearch(object sender, RoutedEventArgs e)
    {
        //New Validation object
        ViewModels.Validation validation = new ViewModels.Validation();

        // Capture user entered term and check is valid
        // submit appropriate message. 
        var searchValue = searchTerm.Text.ToString();
        dynamic validateInput = validation.inputNullCheck(searchValue);

        int? errorCode = validateInput.ErrorCode;

        if (errorCode == 0)
        {
            SubmitAction(searchValue);
        }
        else
        {
            DisplayTerms(validateInput);
        }

    }

My SubmitAction method:

public async Task<object> SubmitAction(string searchValue)
    {

        // Query webservice/database through Model and return response
        dynamic response = await new ViewModels.Search().QueryRequest(searchValue);
        DisplayTerms(response);

        //This is here only because I needed the method to be async, which in turn requires a
        // value to be returned. Ideally it would be async and end after DisplayTerms(response);
        return response;
    }

DisplayTerms method:

private void DisplayTerms(object value)
    {

        ListView termsList = termsListContainer;
        dynamic searchResponse = value;

        int count = searchResponse.Count;

        // This will eventually be a loop through the returned object
        // it is hardcoded at the moment because I am debugging something. 
        termsList.Items.Add(searchResponse[0].TermName);
    }

Validation class:

class Validation
{

    // Temporarily 0 = success, 1 = error
    public int? ErrorCode { get; set; }
    public string ErrorName { get; set; }
    public string ErrorMessage { get; set; }

    //Constructor    
    public Validation ()
    {

    }

    // Check if user input was empty
    public object inputNullCheck(string input)
    {
        if (input != "")
        {
            this.ErrorCode = 0;
            this.ErrorName = "Success";
            this.ErrorMessage = "Valid input received";
            return this;
        }
        else
        {
            this.ErrorCode = 1;
            this.ErrorName = "Input Empty";
            this.ErrorMessage = "You have not input anything.";
            return this; 
        }
    }
}

Answer 1

Class Validation

• you can omit the empty constructor. If a class contains no constructor the compiler will add a default parameterless one.
• the properties setters should be either private or protected
• nullable int as error code does not make sense, at least based on your implementation.

• public object inputNullCheck(string input)

  • this method implies by it's name that it is checking if input == null. But it checks if input != "". Also for readability always check against String.Empty.
  • passing null to this method leads to Success because null != ""
  • instead of returning object you should return Validation
  • instead of checking if ErrorCode == 0 for success you should provide a property Boolean HasError.
  • for input which is not null and not empty you shouldn't set the properties

• Refactored

  class ErrorConstants
  {
      public const int EmptyInput = 1;
      public const int NullInput = 2;
  }
  class Validation
  {
      public int? ErrorCode { get; private set; }
      public string ErrorName { get; private set; }
      public string ErrorMessage { get; private set; }
  
      public Boolean HasError { get { return ErrorCode.HasValue; } }
  
      public Validation ValidateInput(string input)
      {
          if (input == null)
          {
              this.ErrorCode = ErrorConstants.NullInput;
              this.ErrorName = "Input null";
              this.ErrorMessage = "Given input is null.";
          }
          else if (input.Length == 0)
          {
              this.ErrorCode = ErrorConstants.EmptyInput;
              this.ErrorName = "Input Empty";
              this.ErrorMessage = "You have not input anything.";
          }
          return this;
      }
  }
  

private void SubmitSearch(object sender, RoutedEventArgs e)

As searchTerm has a Text property this property will return a String, so calling ToString() is obsolete.

Implementing the changes above this will look like

private void SubmitSearch(object sender, RoutedEventArgs e)
{

    ViewModels.Validation validation = new ViewModels.Validation();

    var searchValue = searchTerm.Text;

    if (validation.inputNullCheck(searchValue).HasError)
    {
        DisplayTerms(validation);
    }
    else
    {
        SubmitAction(searchValue);            
    }
}

You shouldn't call DisplayTerms() here, as this will for sure throw an exception, beacuse a string does not have a property ´Count`.

Naming

Based on the naming guidlines

• classes, methods and properties should be named using PascalCase casing -> inputNullCheck()
• method names should be made out of verbs or verb phrases -> inputNullCheck()

General

• Comments should describe why something is done. Describing what is done should be done by the code itself.

Answer 2

Minor note:

Here in your validation class there's this comment.

// Temporarily 0 = success, 1 = error

Which is fine. You don't want to use a boolean because you'll be adding more status codes in the future. Cool! But you shouldn't be hard coding the error code numbers. This is what enums are for.

enum ValidationStatus {Success,GenericError}

// Check if user input was empty
public object inputNullCheck(string input)
{
    if (input != "")
    {
        this.ErrorCode = ValidationStatus.Success;

At this point, it would be better to remove this code

       this.ErrorName = "Success";
       this.ErrorMessage = "Valid input received";

And create a class that manages what Message to return based off of the value of the ValidationStatus passed into its constructor.

Reducing the above code to

    if (input != "")
    {
        return new ValidationResult(ValidationStatus.Success);