Pass IEnumerable<T> as an argument of method and repository pattern

Question

I've seen tutorials of Unit Testing and I've never seen that IEnumerable<T> used as an argument of method. All authors use Repository pattern and Service layers to interact with data, that is, authors of tutorials get data in Service layer by Repository and there is no need to pass collections between the methods of Service layer.

However, I've written a simple quiz game which imitates Repository pattern and when I started writing unit test methods, then I realized that many of my methods has arguments type of IEnumerable<T>.

This is a simple quiz game where user can give simple answers to simple questions . For example, quiz asks question, then program will remember the answer of player and in the end program will calculate the overall score of the player answers. e.g. "How many continents are there on the Earth?", then quiz shows 4 possible answers, and then quiz remembers answers.

The whole code of my quiz game looks like this:

Model classes:

public class Answer
{
    public Answer(int idAnswer, int idQuestion, string content)
    {
        IdAnswer = idAnswer;            
        IdQuestion = idQuestion;
        Content = content;
    }

    public int IdAnswer { get; }
    public string Content { get; }
    public int IdQuestion { get; }
}

public class Question
{
    public Question(int idQuestion, string content)
    {
        IdQuestion = idQuestion;
        Content = content;
    }

    public int IdQuestion { get; }
    public string Content { get; }
}

public class QuestionAnswer
{
    public QuestionAnswer(int idQuestion, int idAnswer)
    {
        IdQuestion = idQuestion;
        IdAnswer = idAnswer;
    }
    public int IdQuestion { get; set; }
    public int IdAnswer { get; set; }
}

Program class:

static void Main(string[] args)
{
   IQuestionRepository questionService = Factory.CreateInstance<QuestionRepository>();
   var questions = questionService.GetQuestions();

   IAnswerRepository answerService = Factory.CreateInstance<AnswerRepository>();
   var possibleAnswers = answerService.GetPossibleAnswers(questions);
   var playerAnswers = GetPlayerAnswers(questions, possibleAnswers);
   IQuestionAnswerRepository questionAnswerRepository = 
       Factory.CreateInstance<QuestionAnswerRepository>();
   var correctAnswers = questionAnswerRepository.GetCorrectAnswers(questions);

   ICountPlayerScoreBySum playerScores = 
       Factory.CreateInstance<CountPlayerScoreBySumService>();
   var playerScore = playerScores.CountPlayerScoreBySum(playerAnswers, correctAnswers);

   var winScoreString = ConfigurationManager.AppSettings.Get("WinScore");
   int winScore = 0;
   int.TryParse(winScoreString, out winScore);

   Console.WriteLine( playerScore == winScore ? 
        $"Wow! You are a winner! Your score is {playerScore}" 
        : $"Try again! It is just the lesson to win! Your score is {playerScore}");
}

The method GetPlayerAnswers of Program class:

private static IEnumerable<Answer> GetPlayerAnswers(IEnumerable<Question> questions, 
    IEnumerable<Answer> possibleAnswers)
    {
        List<string> allowedAnswers = new List<string>()
            {
                Constants.Constants.Answers.A,
                Constants.Constants.Answers.B,
                Constants.Constants.Answers.C,
                Constants.Constants.Answers.D,
            };

        var playerAnswers = new List<Answer>();

        foreach (var question in questions)
        {
            var possibleAnswersViewModel = possibleAnswers
                .Where(a => a.IdQuestion == question.IdQuestion)
                .OrderBy(a=>a.IdAnswer)
                .Select((a, i) => new PlayerAnswerViewModel { 
                    Content = $"{ IntToLetters(i)}. {a.Content}", 
                    IdAnswer = a.IdAnswer, 
                    IdQuestion = a.IdQuestion, 
                    PlayerKey = IntToLetters(i) 
            });                

            AskQuestion(question, possibleAnswersViewModel);

            while (true)
            {
                var playerKey = Console.ReadKey().KeyChar.ToString().ToUpper();
                Console.WriteLine();
                if (!allowedAnswers.Contains(playerKey))
                {
                    AskQuestion(question, possibleAnswersViewModel, true);
                }
                else
                {
                    var answer = possibleAnswersViewModel
                        .Where(a => a.PlayerKey == playerKey)
                        .FirstOrDefault();
                    if(answer != null)
                        playerAnswers.Add(new Answer(
                            answer.IdAnswer, 
                            question.IdQuestion, 
                            playerKey));
                    break;
                }
            }
        }

        return playerAnswers;
    }

The methods AskQuestion and IntToLetters of Program class:

private static void AskQuestion(Question question, 
     IEnumerable<PlayerAnswerViewModel> possibleAnswers, 
     bool showPossibleKeys = false)
{
        if (showPossibleKeys)
        {
            Console.WriteLine();
            Console.WriteLine("Possible keys are A, B, C or D");
        }

        Console.WriteLine(question.Content);
        possibleAnswers
            .ToList()
            .ForEach(a => Console.WriteLine(a.Content));
}

public static string IntToLetters(int value)
{
    string result = string.Empty;
    result = (char)('A' + value % 26) + result;
    return result;
}

Repositories:

public interface IAnswerRepository
{
    IEnumerable<Answer> GetPossibleAnswers(IEnumerable<Question> questions);
}

interface IQuestionAnswerRepository
{
    IEnumerable<QuestionAnswer> GetCorrectAnswers(IEnumerable<Question> questions);
}

interface IQuestionRepository
{
    IEnumerable<Question> GetQuestions();
}

And implementation of repositories. AnswerRepository:

class AnswerRepository : IAnswerRepository
{
    public IEnumerable<Answer> GetPossibleAnswers(IEnumerable<Question> questions)
    {
        return new List<Answer>() {
            new Answer(11, 3, "Sequoia"), new Answer(12, 3, "Berch"), new Answer(13, 3, "Lindens"), new Answer(14, 3, "Alder"),
            new Answer(1, 1, "1"), new Answer(2, 1, "2"), new Answer(3, 1, "5"), new Answer(4, 1, "6"),
            new Answer(7, 2, "More than 1"), new Answer(8, 2, "More than 2"), new Answer(9, 2, "More than 5"), new Answer(10, 2, "More than 6"),
            new Answer(15, 4, "yes, I do!"), new Answer(16, 4, "Sure!"), new Answer(17, 4, "Exactly"), new Answer(18, 4, "Yeap!"),
            new Answer(19, 5, "yes, I do!"), new Answer(20, 5, "Sure!"), new Answer(21, 5, "Exactly"), new Answer(22, 5, "Yeap!"),
            new Answer(23, 6, "yes, I do!"), new Answer(24, 6, "Sure!"), new Answer(25, 6, "Exactly"), new Answer(26, 6, "Yeap!"),
            new Answer(27, 7, "yes, I do!"), new Answer(28, 7, "Sure!"), new Answer(29, 7, "Exactly"), new Answer(30, 7, "Yeap!")
        }.Where(qa => questions
            .Select(q => q.IdQuestion)
            .Contains(qa.IdQuestion)
        );
    }
}

QuestionAnswerRepository. Imitation of getting correct answers of questions from Database:

public class QuestionAnswerRepository : IQuestionAnswerRepository
{
    public IEnumerable<QuestionAnswer> GetCorrectAnswers(IEnumerable<Question> questions)
    {
        return new List<QuestionAnswer>() {
            new QuestionAnswer(1, 1),
            new QuestionAnswer(2, 2),
            new QuestionAnswer(3, 3),
            new QuestionAnswer(4, 4),
            new QuestionAnswer(5, 1),
            new QuestionAnswer(6, 1),
            new QuestionAnswer(7, 1),
            new QuestionAnswer(8, 1),
            new QuestionAnswer(9, 1),
            new QuestionAnswer(10, 1)
        }            
        .Where(qa => questions
            .Select(q=>q.IdQuestion)
            .Contains(qa.IdQuestion)
        );
    }
}

QuestionRepository:

public class QuestionRepository : IQuestionRepository
{
    public IEnumerable<Question> GetQuestions()
    {
        return new List<Question>() {
            new Question(1, "How many are there contintents?"),
            new Question(2, "How many are there colours?"),
            new Question(3, "What is the tallest tree?"),
            new Question(4, "Do you like dolphins?"),
        };
    }
}

and CountPlayerScoreBySumService:

public class CountPlayerScoreBySumService : ICountPlayerScoreBySum
{
    public int CountPlayerScoreBySum(IEnumerable<Answer> playerAnswers, 
        IEnumerable<QuestionAnswer> correctAnswers)
    {
        var sum = 0;
        foreach (var userAnswer in playerAnswers)
        {
            var correctAnswer = correctAnswers
                .Where(a => a.IdQuestion == userAnswer.IdQuestion)
                .FirstOrDefault();

            if (correctAnswer != null) {
                if (userAnswer.IdAnswer == correctAnswer.IdAnswer)
                    sum += 1;
            }
        }

        return sum;
    }
}

and Factory service:

public class Factory
{
    public static T CreateInstance<T>() where T : new()
    {
        return new T();
    }
}

However, my signatures of methods looks like this. Many methods have arguments type of array:

private IEnumerable<Answer> GetPlayerAnswers(IEnumerable<Question> questions, 
    IEnumerable<Answer> possibleAnswers)
{
    ...
}

Is it okay? Is it a code smell to pass collections as arguments of a method? Or my quiz game is not properly designed? If yes, please be very kind, to advice me how design of an application can be improved.

In addition, I've pushed all my code of Quiz game code into GitHub.

Answer 1

There is no general rule for using IEnumerable or not but there is one that says that you should use the most abstract representation of something because it gives you the most felxibility in what you can pass to such method because there would be fewer restrictions.

This means if you are iterating a collection and you do this only once then IEnumerable<T> is perfect because there is virtually nothing more general than this. However, if you plan to use Add or Count or iterate a collection multiple times then something meterialized would be more appropriate like IList<T> or ICollection<T>.

public int CountPlayerScoreBySum(IEnumerable<Answer> playerAnswers, 
    IEnumerable<QuestionAnswer> correctAnswers)
{
    var sum = 0;
    foreach (var userAnswer in playerAnswers)
    {
        var correctAnswer = correctAnswers
            .Where(a => a.IdQuestion == userAnswer.IdQuestion)
            .FirstOrDefault();

        if (correctAnswer != null) {
            if (userAnswer.IdAnswer == correctAnswer.IdAnswer)
                sum += 1;
        }
    }

    return sum;
}

Here, e.g. the first argument is ok, it's iterated only once but correctAnswers is used multiple times inside the loop so it probably should be something like IList<T> to make it more predictible and to inform the caller that you are going to use it more then once because it might otherwise execute some lengthly query.

You could also materialize it inside the method but this is not always a good idea and to make such decisions requires to take a look at the big picture and the entire design. Sometimes it's acceptable, another time it might not be the case.

So the bottom line is, don't stupidly use any type because some book says so but rather look carefully which type is the simplest one you can use and gives you the required felxibility at the same time. You have to decide in on case by case basis. You'll mostly end up with IEnumerable<T> anyway but make it a sensible decision and not I've heard that....

Answer 2

There is nothing inherently wrong with using IEnumerable<T>, just as there would be nothing wrong with using List<T>. Whatever collection type you choose to accept or return is far more a matter of your application's needs than any style guide.

For example, if you were writing a public-facing API, you would likely want to be most permissible in what you accept in order to make it easy for clients to use that API. In that case, using IEnumerable<T> would allow any collection type (including arrays) to be passed to your methods.

On the other hand, if you were writing high-performance code that is internal to your application, you'd probably use List<T> and pass it between your (presumably private) methods (using concrete types is slightly faster than interfaces).

Remember that style guides are just that - guides. It's up to you the programmer to apply those guidelines intelligently and logically.

Answer 3

Changing comments to answer on OP request. I am not answering the direct question asked by OP but few comments about the overall architecture and design of there solution which may indirectly solve the issue OP is worried about, passing collections as parameters.

IQuestionRepository questionService = Factory.CreateInstance<QuestionRepository>(); 

Light heartly, never ever create an object from repository and name it a service, it makes code confusing for people me like who following onion architecture as show below,

Also in C# usually objects are initialled together WHEN you are only initiating them for calling a single function or maybe 2 and there is no risk of memory leak, it also makes code more readable. Did you thought about using DI ?

   IQuestionRepository questionService = Factory.CreateInstance<QuestionRepository>();
   IAnswerRepository answerService = Factory.CreateInstance<AnswerRepository>();
   IQuestionAnswerRepository questionAnswerRepository = Factory.CreateInstance<QuestionAnswerRepository>();
   ICountPlayerScoreBySum playerScores = Factory.CreateInstance<CountPlayerScoreBySumService>();

   var possibleAnswers = answerService.GetPossibleAnswers(questions);
   var questions = questionService.GetQuestions();
   var playerAnswers = GetPlayerAnswers(questions, possibleAnswers);
   var correctAnswers = questionAnswerRepository.GetCorrectAnswers(questions);
   var playerScore = playerScores.CountPlayerScoreBySum(playerAnswers, correctAnswers);

   var winScoreString = ConfigurationManager.AppSettings.Get("WinScore");
   int winScore = 0;
   int.TryParse(winScoreString, out winScore);

Also sometimes abstraction is not a good thing, especially if there is no scope of scalling program in future. Do you have any ERD for your this app ? Your quiz app doesn't seems relational when it could be a good candidate - here's an example ERD I picked up from internet,