-2
\$\begingroup\$

I have written the following Repository in C#, using NetCore 2.2 and EF Core.

I was thinking how to write properly the Search method according to the SOLID principles.

This code gives a null reference exception when "Title" or "Genre" property is null. It means I will need to put an if to check for null values, and in that case, it does not respect the single responsibility principle: as this method is returning a set of elements from the DB and also making some operations with strings.

How can I refactor this to avoid null exception and respect the SRP?

public class MovieRepository: IMovieRepository
{
    private readonly MovieDbContext _moviesDbContext;
    public MovieRepository(MovieDbContext moviesDbContext)
    {
        _moviesDbContext = moviesDbContext;
    }

    public IEnumerable<Movie> GetAll()
    {
        return _moviesDbContext.Movies;
    }

    public IEnumerable<Movie> Search(MovieFilters filters)
    {
        var title = filters.Title.ToLower();
        var genre = filters.Genre.ToLower();
        return _moviesDbContext.Movies.Where( 
             p => (p.Title.Trim().ToLower().Contains(title) | string.IsNullOrWhiteSpace(p.Title))
             & (p.Genre.Trim().ToLower().Contains(genre) | string.IsNullOrWhiteSpace(p.Genre))
             & (p.YearOfRelease == filters.YearOfRelease | filters.YearOfRelease == null)
           );
    }
}

And this is the code that works in case of null values but I don't know how to refactor this according to with SOLID, I will appreciate some ideas to move those IFs sentences:

public IEnumerable<Movie> Search(MovieFilters filters)
{
    string title = "";
    if (string.IsNullOrWhiteSpace(filters.Title) == false)
    {
        title = filters.Title.ToLower();
    }

    string genre = "";
    if (string.IsNullOrWhiteSpace(filters.Genre) == false)
    {
        genre = filters.Genre.ToLower();
    }

    return _moviesDbContext.Movies.Where( p => (p.Title.Trim().ToLower().Contains(title) | string.IsNullOrWhiteSpace(p.Title))
                                               & (p.Genre.Trim().ToLower().Contains(genre) | string.IsNullOrWhiteSpace(p.Genre))
                                               & (p.YearOfRelease == filters.YearOfRelease | filters.YearOfRelease == null)
                                        );
}
\$\endgroup\$

closed as off-topic by πάντα ῥεῖ, t3chb0t, Graipher, Mast, Heslacher Jan 21 at 5:27

This question appears to be off-topic. The users who voted to close gave these specific reasons:

  • "Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review." – Graipher, Mast, Heslacher
  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – πάντα ῥεῖ, t3chb0t
If this question can be reworded to fit the rules in the help center, please edit the question.

0
\$\begingroup\$

There is a difference between & and && as well as | and ||. There is a null coalescing operator ?. and ?? that you can use and string equals method has a second parameter where you can pass StringComparison.OrdinalIgnoreCase.

The lambda could look like:

o => p.Title?.Trim().Equals(title ?? "", StringComparison.OrdinalIgnoreCase) 
  || p.Genre?.Trim().Equals(genre ?? "", StringComparison.OrdinalIgnoreCase)

What the SOLID means is that you pass that in a an argument to a method like

public IEnumerable<T> Search<T>(System.Linq.Expressions.Expression<Func<T, bool>> predicate)
{
    return _dbContext.Set<T>()
           .Where(predicate)
           .AsEnumerable();
}

and it would not contain your validation checks or business logic.

\$\endgroup\$
  • \$\begingroup\$ Regarding the || in the lambda expressions, is not what I want, I would like all the conditions to be checked: so if the movie title has something and also the genre I want to apply the filter for both conditions. Not sure if I understood well you code. \$\endgroup\$ – MarcosF8 Jan 20 at 7:53

Not the answer you're looking for? Browse other questions tagged or ask your own question.