C
C#•3y ago
TotechsStrypper

Refactoring if else

Help ! 
partial void OnNameChanging(string value)
        {
            _validationService.ClearErrors(nameof(Name));
            ErrorsList.ToList()
                      .Where(error => error.PropId == 1)
                      .All(error => ErrorsList.Remove(error));

            if (string.IsNullOrEmpty(value) || string.IsNullOrWhiteSpace(value))
            {
                _validationService.AddError(nameof(Name), "You must provide your pet a name");

                var errors = GetErrors(nameof(Name)).OfType<string>().ToList();
                errors.ForEach(error => ErrorsList.Add(new ValidationProperty(1, error)));

            }
            else if (value.Length < 3)
            {
                _validationService.AddError(nameof(Name), "Your pet name is a little short");

                var errors = GetErrors(nameof(Name)).OfType<string>().ToList();
                errors.ForEach(error => ErrorsList.Add(new ValidationProperty(1, error)));
            }
            else if (value == "Snow")
            {
                _validationService.AddError(nameof(Name), "Wait! ain't snow is already in your profile ?");

                var errors = GetErrors(nameof(Name)).OfType<string>().ToList();
                errors.ForEach(error => ErrorsList.Add(new ValidationProperty(1, error)));
            }
        }
partial void OnNameChanging(string value)
        {
            _validationService.ClearErrors(nameof(Name));
            ErrorsList.ToList()
                      .Where(error => error.PropId == 1)
                      .All(error => ErrorsList.Remove(error));

            if (string.IsNullOrEmpty(value) || string.IsNullOrWhiteSpace(value))
            {
                _validationService.AddError(nameof(Name), "You must provide your pet a name");

                var errors = GetErrors(nameof(Name)).OfType<string>().ToList();
                errors.ForEach(error => ErrorsList.Add(new ValidationProperty(1, error)));

            }
            else if (value.Length < 3)
            {
                _validationService.AddError(nameof(Name), "Your pet name is a little short");

                var errors = GetErrors(nameof(Name)).OfType<string>().ToList();
                errors.ForEach(error => ErrorsList.Add(new ValidationProperty(1, error)));
            }
            else if (value == "Snow")
            {
                _validationService.AddError(nameof(Name), "Wait! ain't snow is already in your profile ?");

                var errors = GetErrors(nameof(Name)).OfType<string>().ToList();
                errors.ForEach(error => ErrorsList.Add(new ValidationProperty(1, error)));
            }
        }
6 Replies
Greenthy
Greenthy•3y ago
return on the first if and just keep the other if's (not nested in else if)
TotechsStrypper
TotechsStrypperOP•3y ago
can you provide me the modified code ?
Greenthy
Greenthy•3y ago
not behind a pc atm 😄
Klarth
Klarth•3y ago
Console.WriteLine(GetErrorMessage(""));
Console.WriteLine(GetErrorMessage("\t"));
Console.WriteLine(GetErrorMessage(null));
Console.WriteLine(GetErrorMessage("test"));
Console.WriteLine(GetErrorMessage("as"));
Console.WriteLine(GetErrorMessage("Snow"));

string? GetErrorMessage(string? value)
{
    //_validationService.ClearErrors(nameof(Name));
    //ErrorsList.ToList()
    //          .Where(error => error.PropId == 1)
    //          .All(error => ErrorsList.Remove(error));

    string? errorMessage = value switch
    {
        _ when string.IsNullOrWhiteSpace(value) => "You must provide your pet a name",
        _ when value.Length < 3 => "Your pet name is a little short",
        "Snow" => "Wait! ain't snow is already in your profile ?",
        _ => null
    };

    //if (errorMessage is not null)
    //{
    //    _validationService.AddError(nameof(Name), errorMessage);

    //    var errors = GetErrors(nameof(Name)).OfType<string>().ToList();
    //    errors.ForEach(error => ErrorsList.Add(new ValidationProperty(1, error)));
    //}

    return errorMessage;
}
Console.WriteLine(GetErrorMessage(""));
Console.WriteLine(GetErrorMessage("\t"));
Console.WriteLine(GetErrorMessage(null));
Console.WriteLine(GetErrorMessage("test"));
Console.WriteLine(GetErrorMessage("as"));
Console.WriteLine(GetErrorMessage("Snow"));

string? GetErrorMessage(string? value)
{
    //_validationService.ClearErrors(nameof(Name));
    //ErrorsList.ToList()
    //          .Where(error => error.PropId == 1)
    //          .All(error => ErrorsList.Remove(error));

    string? errorMessage = value switch
    {
        _ when string.IsNullOrWhiteSpace(value) => "You must provide your pet a name",
        _ when value.Length < 3 => "Your pet name is a little short",
        "Snow" => "Wait! ain't snow is already in your profile ?",
        _ => null
    };

    //if (errorMessage is not null)
    //{
    //    _validationService.AddError(nameof(Name), errorMessage);

    //    var errors = GetErrors(nameof(Name)).OfType<string>().ToList();
    //    errors.ForEach(error => ErrorsList.Add(new ValidationProperty(1, error)));
    //}

    return errorMessage;
}
You could try pattern matching, but I really suggest you find an entirely different approach for the top and bottom commented out sections. .ForEach is a code smell. Also, likely every time you type a letter (ie. the binding is changing), you're reallocating a new list for the filter on the ErrorList.ToList() line.
D.Mentia
D.Mentia•3y ago
Also it looks like you're already using some form of validation library, this is basically what it's made for... fluentvalidation, for example, you'd just define RuleFor(x => x.PetName).NotEmpty().Length(3,255).NotEqual("Snow")
Klarth
Klarth•3y ago
Yeah, this is Mvvm Toolkit's validation, so you want a different approach if there's nothing really specific to be handled in INotifyPropertyChanging. Something like https://paste.mod.gg/vsemuppbvcqv/0 is significantly better. You might be able to get away with only Data Annotations. Might be a neater way to write the switch expression too.

Did you find this page helpful?