HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Correct usage of MVVM and object for method

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
methodmvvmusagecorrectforandobject

Problem

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 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 deb

Solution

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);

Code Snippets

// Temporarily 0 = success, 1 = error
enum ValidationStatus {Success,GenericError}

// Check if user input was empty
public object inputNullCheck(string input)
{
    if (input != "")
    {
        this.ErrorCode = ValidationStatus.Success;
this.ErrorName = "Success";
       this.ErrorMessage = "Valid input received";
if (input != "")
    {
        return new ValidationResult(ValidationStatus.Success);

Context

StackExchange Code Review Q#73354, answer score: 4

Revisions (0)

No revisions yet.