C# validate model by mapping values of multiple Lists

Question

I need to run validation on JSON input model by taking the value from one List and check if it exists in another. This value is a rating is from 1 - 5. If there is no matching number, then it should throw an error. Below is the code and this logic goes in the section commented with : //check if rating exist in score table

namespace Alpha.Model
{
    // INPUT
    public class AlphaCalcParamMethod
    {     
       public ICollection<PortfolioInputModel> portfolios { get; set; }
       public Setting settings { get; set; }

        public bool Validation(ref string errString)
        {
            // Check if portfolio exists
            if(portfolios == null || portfolios.Count < 1)
            {
                errString = "At least 1 Portfolio.";
                return false;
            }
            //check if weight adds upto 1
            foreach(var portfolio in portfolios)
            {
                // var holdings = new List<PortfolioHoldingInput>();
                var weightAggregator = 0.00;
                foreach(var holding in portfolio.portfolioHoldings)
                {
                    weightAggregator += holding.fundWeight;
                }
                if (weightAggregator != 1)
                {
                    errString = "Fund Weights should add upto 1";   
                }
                return false;
            }
            //check if rating exist in score table
            foreach(var portfolio in portfolios)
            {
                var holdings = new List<PortfolioHoldingInput>();
                var scores = new List<Setting>();

                foreach(var holding in holdings)
                {
                    //fetch the value of fundRating double
                    foreach(var score in scores)
                    {
                       //check if the value above exist in grossAlpha's List fundRating
                       // if it doesn't return false  
                    }
                }
                return false;
            }
            return true;
        }

    }

    // OUTPUT
    public class AlphaCalcResultMethod
    {
        public List<PortfolioOutputModel> portfolios { get; set; }
    }

    public class PortfolioInputModel
    {
        public string portfolioIdentifier { get; set; }
        public ICollection<PortfolioHoldingInput> portfolioHoldings { get; set; }
    }

    public class PortfolioOutputModel
    {
        public string portfolioIdentifier { get; set; }
        public double portfolioAlpha { get; set; }
        public ICollection<PortfolioHoldingOutput> portfolioHoldings { get; set; }
    }
    public class PortfolioHoldingInput
    {
        public string fundIdentifier { get; set; }
        public int fundRating { get; set; }
        public double fundExpenseRatio { get; set; }
        public double fundWeight { get; set; }
    }
    public class PortfolioHoldingOutput
    {
        public string fundIdentifier { get; set; }
        public int fundRating { get; set; }
        public double fundExpenseRatio { get; set; }
        public double fundWeight { get; set; }
        public double fundAlpha { get; set; }
    }
    public class Setting
    {
       public List<GrossAlpha> grossAlphas { get; set; }
    }

    public class GrossAlpha
    {
        public int fundRating { get; set; }
        public double grossAlpha { get; set; }
    }
}

Why error string is passed by `ref`? You are going to return it from validation method, so it should be `out` parameter — Sergey Berezovskiy, Feb 23 '17 at 20:56
the two nested `foreach` (for `holdings` and `scores`) won't be executed due to empty lists. This is what you meant? — elirandav, Feb 23 '17 at 21:06
It's not clear what you are trying to validate with scores. And where is grossAlpha's mentioned in comment — Sergey Berezovskiy, Feb 23 '17 at 21:08

Sergey Berezovskiy · Accepted Answer · 2017-02-23T21:22:59.757

2

If you are going to return additional values from method, you should use out parameters.
Don't specify type of variable in variable name. I.e. instead of errorString just use error. Hungarian notation and other tricks are not needed with modern IDEs.
Double type is not precise. You should avoid comparing it to integer values for equality. Prefer greater or less than comparisons.
Use LINQ to replace loops
Method name Validation is a noun. That is pretty confusing for method which is action and should a verb. Consider rename it to Validate or IsValid

Code

public bool IsValid(out string error)
{
    if (portfolios?.Any() == false)
    {
        error = "At least 1 Portfolio.";
        return false;
    }

    if (portfolios.Any(p => p.portfolioHoldings.Sum(h => h.fundWeight) < 1))
    {
        error = "Fund Weights should add upto 1";
        return false;
    }

    var holdings = portfolios.SelectMany(p => p.portfolioHoldings);
    var validRatings = new List<int> { 1, 2, 3, 4, 5 };

    if (holdings.Any(h => !validRatings.Contains(h.fundRating)))
    {
        error = "Fund ratings should be in " + String.Join(",", validRatings);
        return false;
    }

    error = "";
    return true;
}

Note: if valid ratings are sequential numbers, then you can just check range of fundRating value:

  if (holdings.Any(h => h.fundRating < 1 || 5 < h.fundRating))

edited Feb 23 '17 at 21:22

answered Feb 23 '17 at 21:12

Sergey Berezovskiy

232,247
41
429
459

Is there any reason to use `out` vs `ref`? edit - guess I should have just googled it. Here's an answer as to why you should use `out` http://stackoverflow.com/questions/1516876/when-to-use-ref-vs-out – Xander Luciano Feb 23 '17 at 21:15
Yes, `ref` requires error message to be initialized by caller, and it shows that value of argument is relevant to method which is called. Which is not true with error messages here. `out` does not require initialization on caller side and shows that method will do all the job related to assigning error message – Sergey Berezovskiy Feb 23 '17 at 21:20
var validRatings = new List { 1, 2, 3, 4, 5 }; Is there a way to dynamically check the values? I know it's 1 through 5 but don't want to assume that. – Mihir Patel Feb 23 '17 at 22:02
@MihirPatel as I asked you in comments - from question its not clear where valid ratings come from – Sergey Berezovskiy Feb 23 '17 at 22:05
public class GrossAlpha { public int fundRating { get; set; } public double grossAlpha { get; set; } } This has fundRating with a range of 1-5. – Mihir Patel Feb 23 '17 at 23:26
@MihirPatel that's just class definition. Where values? – Sergey Berezovskiy Feb 23 '17 at 23:26
\"settings\":{ \"grossAlphas\":[ { \"fundRating\": 0, \"grossAlpha\": 0 }, { \"fundRating\": 1, \"grossAlpha\": -0.005 }, { \"fundRating\": 2, \"grossAlpha\": 0 }, { \"fundRating\": 3, \"grossAlpha\": 0.0015 }, { \"fundRating\": 4, \"grossAlpha\": 0.003 }, { \"fundRating\": 5, \"grossAlpha\": 0.005 } ] } – Mihir Patel Feb 23 '17 at 23:27
@MihirPatel that's a json. How you get data in validation method? – Sergey Berezovskiy Feb 23 '17 at 23:29
I don't at this moment. I am assuming I would have to define a path with JSON reader? – Mihir Patel Feb 23 '17 at 23:30
1

@MihirPatel something like that. Then you should pass `settings` instance to validator class. And use it like `var validRatings = settings.grossAlphas.Select(ga => ga.fundRating).ToList();` – Sergey Berezovskiy Feb 23 '17 at 23:35

score 1 · Answer 2 · answered Feb 23 '17 at 21:11

Look this code:

if(!score.grossAlphas.Exists(x => x.fundRating == holding.fundRating))
 {
   return false;
 }

it check if exists grossAlphas with fundRating equals holding.fundRating.

put it in the loop where you want to check, let me know if it is what you want and if it works.

C# validate model by mapping values of multiple Lists

2 Answers2