Often, the interviewers write to me that in the test tasks I do not do validation.

Previously for me, the concept of юнит-тестирование , валидация , try...catch somehow was the same. And only in the last year I have “mastered” unit testing and now I can’t present my code without tests and encode it via TDD (without fanaticism).

try ... catch for me is also a little dark forest, namely: I did not understand exactly when to use it.

But today I want you to help me deal with validation.

Right now I am writing code, implementing the factory method pattern, where a TResultDTO object is created to transfer its AngularJS to View. The TResultDTO object is formed based on the EF-entity TestResult. And I'm trying to organize a validation:

 public TResultDTO FactoryMethod(TestResult entity) { //Валидация if(entity != null) { var dto = new TResultDTO { ExamCode = entity.TestPlan.Test.NumberCode, ExamName = entity.TestPlan.Test.Name, ExamDate = entity.TestDate.ToShortDateString(), ParticipCode = entity.ParticipCode }; return dto; } }

Question

  1. Well first VS now writes:

enter image description here

and what should I do about it? I see no way out, except that at the end of the method specify return null . Well then, in general, something incomprehensible comes out.

  1. Is it enough to check the entity for null? Or, ideally, you should check its properties ( entity.TestPlan.Test.NumberCode , entity.TestPlan.Test.Name ...), directly to the values ​​that the method refers to?

I also understand that there are no ready-made formulas for the fish and a lot (maybe) will depend on the subject area. The only thing that I could find on this account in the network: a colleague on the forum writes that the incoming parameters of public methods must necessarily be subjected to the validation process.

Update

Having listened to the answers and comments, I do validation as follows:

 namespace Monit95App.Models { public class TResultDTOcreator : ITResultDTOcreator { public TResultDTO FactoryMethod(TestResult entity) { if (String.IsNullOrEmpty(entity.TestPlan.Test.NumberCode)) throw new ArgumentException("Property cannot be null or empty", nameof(entity.TestPlan.Test.NumberCode)); if (String.IsNullOrEmpty(entity.TestPlan.Test.Name)) throw new ArgumentException("Property cannot be null or empty", nameof(entity.TestPlan.Test.Name)); if (entity.TestPlan.TestDate == null || entity.TestPlan.TestDate == DateTime.MinValue) throw new ArgumentException("Property cannot be null or empty", nameof(entity.TestPlan.TestDate)); if (String.IsNullOrEmpty(entity.ParticipCode)) throw new ArgumentException("Property cannot be null or empty", nameof(entity.ParticipCode)); var dto = new TResultDTO { ExamCode = entity.TestPlan.Test.NumberCode, ExamName = entity.TestPlan.Test.Name, ExamDate = entity.TestDate.ToShortDateString(), ParticipCode = entity.ParticipCode }; return dto; } } }

What can colleagues say now?

Previously forgot to note that the project uses the database first approach .

    3 answers 3

    Everything in order:

    1. Visual Studio is absolutely correct

    Your method is not void , but returns a value only if entity != null . And for the case of entity == null ? Of course, you can return null , but you can discover exceptions or the previously proposed NullObject pattern.

    1. It all depends on the task, as well as how and by whom the object class is implemented.

    It is possible that all the necessary checks are already implemented inside this class (if the code first approach was used) or at the database level. But, if they are not there, or this class or database was not created by you, it is better to err. Unfortunately, the second part of the question is very difficult to answer unambiguously (it is too “common”).

    • The TestResult class is a database object. This example uses the database first approach. I am interested in what you said: ... все необходимые проверки уже реализованы внутри этого класса . What exactly does this mean? - Adam
    • @adamshakhabov, unfortunately, for some reason there is not a word in the source code of the question about database first. But even in this case, it is possible to implement validation on the C # side. It is not necessary to edit the code generated by VS. Enough to attach to the original entity, for example, "decorator". If, of course, it is justified. - Streletz

    Data validation is a very simple thing. This is just a test for matching the input data with the expected range of values.

    For example, if the person’s age is given to you, then he obviously cannot be negative. Well, the age of 4 billion years also somehow does not look right (although, it still depends on the task). If you are given an entry start date and an end date, then the end cannot be earlier than the beginning.

    A simpler case is that if an object containing a data set arrives at the input, null should not come instead.

    More precisely, the set of restrictions on the input data (that is, the validity criterion) must be issued by the person who wrote the function. This is usually part of the documentation.

    Data validation is the verification that the data conforms to the constraints and the appropriate response to the inconsistencies. In C #, it is customary to throw an exception (usually ArgumentException or its derivatives), other languages ​​report errors differently (for example, an object of type error returned to Go).

    For your case, your code refers to the properties of the entity , entity.TestPlan and entity.TestPlan.Test . Therefore, for starters, your code should check the entity :

     if (entity == null) throw new ArgumentNullException(nameof(entity));

    Next, does entity.TestPlan have the right entity.TestPlan be null ? If not, you should consider this in validation:

     if (entity.TestPlan == null) throw new ArgumentNullException(nameof(entity.TestPlan));

    If so, your code will entity.TestPlan == null in the case where entity.TestPlan == null , and it needs to be fixed.

    The same applies to the other properties that you use in the code.

    Simple principle: the code has no right to fall with shameful exceptions like NullReferenceException or there IndexOutOfBoundException . If you validate your input data, you must catch all possible sources of problems, and either process them correctly in your code, or reject the data during validation, throwing an exception.

      It all depends on the context in which your factory method is used.

      except that at the end of the method specify return null

      If you do not want your factory method to return null (and, as a result, avoid a subsequent check for null ) then consider using the NullObject pattern

      Is it enough to check the entity for null ?

      It all depends on your task. If the statement that the object is valid is enough to check for null , then yes. Another question is whether the factory method should validate the object and does it not contradict the SRP ?