3
\$\begingroup\$

I'm trying to apply Uncle Bob's Clean Code practices onto this exercise I'm helping someone with. (The exercise assignment is in a "docstring" comment above the Main.)

I'm unsure what the best naming convention would be for ReturnAllCombinationsOfInputtedList, although I suspect it doesn't involve the return word in it's name for starters. Same for CreateArray1to30(). Any suggestions for name choices/proper conventions here?

Do you guys see anything else that I should've done differently in your opinion?

using System;
using System.Collections.Generic;
using System.Linq;

namespace ConsoleAppHelpingAlexWithExercices
{
    class Program
    {
        //**************************************************************************
        // How many different exam tickets with two different questions can be made, 
        // from a syllabus with 30 questions
        //**************************************************************************
        static void Main(string[] args)
        {
            int[] questionsList = CreateArray1to30();
            List<string> combinations = ReturnAllCombinationsOfInputtedList(questionsList);
            int amountOfUniqueCombinations = combinations.Distinct<string>().Count();
            Console.WriteLine("Total unique combinations: {0}", amountOfUniqueCombinations);
        }

        static List<string> ReturnAllCombinationsOfInputtedList(int[] questionsList)
        {
            List<string> combinations = new List<string>();
            int[] aTest = new int[2];
            foreach (int firstQuestion in questionsList)
            {
                foreach (int secondQuestion in questionsList)
                {
                    if (firstQuestion != secondQuestion)
                    {
                        aTest[0] = firstQuestion;
                        aTest[1] = secondQuestion;
                        //Makes AB and BA tests both AB, so we can de-duplicate later.
                        Array.Sort(aTest);
                        combinations.Add(String.Format("{0}&{1}", aTest[0], aTest[1]));
                        Console.WriteLine("{0} and {1}", aTest[0], aTest[1]);
                    }
                }
            }
            return combinations;
        }

        static int[] CreateArray1to30()
        {
            return Enumerable.Range(1, 30).ToArray();
        }
    }
}
\$\endgroup\$
2
  • 3
    \$\begingroup\$ Welcome to Code Review! We need to know what the code is intended to achieve. To help reviewers give you better answers, please add sufficient context to your question, including a title that summarises the purpose of the code. We want to know why much more than how. The more you tell us about what your code is for, the easier it will be for reviewers to help you. The title needs an edit to simply state the task, rather than your concerns about the code. \$\endgroup\$ Commented Jan 18, 2021 at 19:15
  • \$\begingroup\$ I'm not a c# programmer so won't go into any detail, but surely you don't need to make all permutations and de-duplicate them? If you have 30 questions, you have a choice of 29 to combine with the first, but only 28 to combine with the second and so on... \$\endgroup\$ Commented Jan 20, 2021 at 13:56

1 Answer 1

3
\$\begingroup\$
  • CreateArray1to30() could be renamed to CreateQuestions()
  • ReturnAllCombinationsOfInputtedList() could be renamed to Combinations() or QuestionCombinations()
  • ReturnAllCombinationsOfInputtedList() does too much. The call to .Distinct() is unecessary, and can be replaced with correct indices in the first place. Furthermore, the aTest array is entirely useless and could be replaced by using firstQuestion and secondQuestion directly.
  • Strings are a non ideal data structures for the exams, as you use a different format when printing them out
using System;
using System.Collections.Generic;
using System.Linq;


namespace ConsoleAppHelpingAlexWithExercices
{
    struct ExamTicket {
        public int Question1, Question2;
        public ExamTicket(int Question1, int Question2) {
            this.Question1 = Question1;
            this.Question2 = Question2;
        }
    }
    class Program
    {
        //**************************************************************************
        // How many different exam tickets with two different questions can be made, 
        // from a syllabus with 30 questions
        //**************************************************************************
        static void Main(string[] args)
        {
            int[] questions = CreateQuestions();
            var combinations = new List<ExamTicket>(Combinations(questions));
            foreach (var combination in combinations) {
                Console.WriteLine($"{combination.Question1} and {combination.Question2}");
            }
            Console.WriteLine("Total unique combinations: {0}", combinations.Count);
        }

        static IEnumerable<ExamTicket> Combinations(int[] questions)
        {
            for (int i=0;i<questions.Length;++i) {
                for (int j=i+1;j<questions.Length;++j) {
                    yield return new ExamTicket(questions[i],questions[j]);
                }
            }
        }

        static int[] CreateQuestions()
        {
            return Enumerable.Range(1, 30).ToArray();
        }
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ static IEnumerable<ExamTicket> ToExam(this int[] questions) :) \$\endgroup\$ Commented Jan 20, 2021 at 18:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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