I always use Result classes for returning state from a service class (wouldn't implement that in Repository as it shouldn't contain business logic):
public class QueryResult
{
private static readonly QueryResult success = new QueryResult { Succeeded = true };
private readonly List<QueryError> errors = new List<QueryError>();
public static QueryResult Success { get { return success; } }
public bool Succeeded { get; protected set; }
public IEnumerable<QueryError> Errors { get { return errors; } }
public static QueryResult Failed(params QueryError[] errors)
{
var result = new QueryResult { Succeeded = false };
if (errors != null)
{
result.errors.AddRange(errors);
}
return result;
}
}
public class QueryResult<T> : QueryResult where T : class
{
public T Result { get; protected set; }
public static QueryResult<T> Suceeded(T result)
{
if (result == null)
throw new ArgumentNullException(nameof(result));
var queryResult = new QueryResult<T>
{
Succeeded = true,
Result = result
};
return queryResult;
}
}
public class QueryError
{
public string ErrorId { get; set; }
public string ErrorMessage { get; set; }
}
And use it like
var schoolclassCode = await context.Schoolclasses
.SingleOrDefaultAsync(s => s.Id == schoolclassCodeId && s.UserId == userId);
if (schoolclassCode == null)
return QueryResult.Failed(new QueryError
{
ErrorId = 1,
ErrorMessage = "Invalid User Id"
});
Edit:
Just as an addition and rule of thumb
- Services which operate on one or multiple entities and perform user input validation should return
Result classes
- Domain Models (which you don't seem to use, since you use a repository and Repository + Rich Domains doesn't work out well in real life applications) should throw exception (i.e.
InvalidOperationException or ArgumentException, ArgumentNullException). Doing Result-types her will pollute the model and mix the separation of responsibility (Domain Model will suddenly also do validation instead only guarding against invalid state)
Using XxxResult type classes gives you an easy way to transport one or multiple errors back to the user, where an exception should act as an guard against your domain model getting into invalid state.
Edit 2
In response to the comments:
public async Task<IActionResult> Post([FromBody]CreatePupilRequestDto dto)
{
var userId = User.GetUserId();
var pupil = dto.ToPupil();
var result = await repository.CreatePupil(pupil, dto.SchoolclassId, userId);
// If you want to suppress the error messages, just call return BadRequest() instead
if(!result.Succeeded)
return BadRequest(result.Errors);
return Ok(pupil.Id);
}
Edit 3
Example with 3 parameters for let's say /api/schoolclasses/1/students/2/lessons/2 (Update an existing lesson to the student with the id 2 for the school class with id 1).
// on SchoolClasses Controller
[HttpPost("{schoolClassId:int}/students/{studentId:int}/lessons/{lessonId:int}")]
public async Task<IActionResult> Post([FromBody]Lessons lessonDto)
{
// rough input validation, do first to avoid db hits
if(!ModelState.IsValid)
return BadRequest(ModelState);
// best put logic into service classes i.e. SchoolClassService
var result = schoolClassService.UpdateLessonFor(schoolClassId, studentId, lessonDto)
// If you want to suppress the error messages, just call return BadRequest() instead
if(!result.Succeeded)
return BadRequest(result.Errors);
return Ok();
}
Content of UpdateLessonsFor
List<ErrorMessage> errors = new List<ErrorMessage>();
// with .Include to include both student and all of his lessons
// does student exist?
// Hits db once and gets both, student and all lessons in a single query
var student = _context.SchoolClasses
.Include(sc => sc.Students)
.ThenInclude(s => s.Lessons)
.Where(sc => sc.SchoolClassId == schoolClassId)
.SelectMany(sc => sc.Students)
FirstOrDefault(s => s.StudentId == studentId);
if(student==null)
return QueryResult.Failed( new ErrorMessage { ErrorId = 1, ErrorMessage = "Student or School Class not found" } );
// Doesn't hit the database, since lessons have been loaded with the above call
var lesson = student.Lessons.Any(l => l.LessonId = lessonId))
if(lesson == null)
return QueryResult.Failed( new ErrorMessage { ErrorId = 2, ErrorMessage = "Lesson not found. " } );
// modify it
lesson.SomeValue = dto.SomeValue;
try
{
} catch(Exception ex) {
return QueryResult.Failed(new ErrorMessage { ErrorId = 3, ErrorMessage = "Couldn't update the lesson. Try again and if the error appears again, contact the administrator." } );
} finally {
return QueryResult.Suceeded;
// or if you also want to return a result
return QueryResult.Suceeded(lesson);
}
Also from the comments of the other answer: Don't put logic into your repository, that's what services are for when you use anemic domain (models have no logic, all in services) or have thin service layer and put most logic into domain service. But that's out of the scope.
