Pierwsze co się rzuca w oczy to foldery bin i obj oraz .vs. Ich nie powinno być. Dodaj te foldery do gitignore
Jeszcze drobnostka, oczywiście to nie jest błąd, ale masz jakiś powód dla którego korzystasz akurat z .NETFramework v4.7.2? Najnowszy to 4.8, więc warto z niego korzystać. A jeszcze lepiej z .NET Core, ponieważ on powoli przejmuje .NET Framework + .net core 3.0/3.1 posiada obsługę c# 8.0.
Jeszcze ogólnie co do samego kodu.
Widzę, że nazywasz argumenty metod zaczynając od _. Raczej konwencja jest żeby argumenty zapisywać normalnie, z małej litery, a pola prywatne z _ lub bez, tak samo jak argumenty. Oczywiście możesz sobie przyjąć to jak chcesz, jeśli konsekwentnie się będziesz tego trzymać.
#region powinno się raczej rzadko używać, ponieważ klasy i metody powinny być na tyle krótkie i mieć tylko jedną odpowiedzialność, że używanie region staje się niepotrzebne. Osobiście w ogóle nie używam region, albo bardzo rzadko.
Public properties przeważnie zaczyna się z dużej litery, ale to też kwestia indywidualna/teamu.
Co do szczegółów kodu, to jutro przejrzę dokładniej i napiszę tu w komentarzu resztę.
Update:
Na pewno klasy są zbyt długie.
SelectQuiz wydzieliłbym do inne klasy wraz z ShowQuizes.
while (!Int32.TryParse(input, out intInput)
|| --intInput < 0 // Decrement here, right now [1] is first index
|| intInput > Quizes.Count)
{
Console.Clear();
Console.WriteLine("Incorrect input! Please choose quiz again!");
ShowQuizes();
input = Console.ReadLine();
}
Ten warunek nie podoba mi się, jest stanowczo nieczytelny. Trzeba chwilę czasu poświęcić na zrozumienie kiedy będzie się wykonywać. Wydziel to do metody zwracającej bool np. isCorrectInput i potem użyj tak:
while ( !isCorrectInput(input))
Widzę że masz aż 3 metody CreateNewQuiz (jedna to CreateQuizTest). Wydziel je do osobnej klasy i również w CreateNewQuiz uprość ten warunek while'a wydzielając do nowej metody. Ogólnie możesz porobić więcej metod, tym samym zmniejszając skomplikowanie kodu.
Z tymi properties myślę, czy nie lepiej jako klasę/strukturę zrobić.
public static int numberOfAnswers { get; } = 4;
public static int minQuestions { get; } = 2;
public static int maxQuestions { get; } = 10;
public static int minTitleLength { get; } = 4;
public static int minDescriptionLength { get; } = 15;
public void LoadGame()
{
if (!File.Exists(saveFileName)) // Go back if questions doesn't exist
return;
BinaryFormatter binFormat = new BinaryFormatter();
using (Stream fStream = File.OpenRead(saveFileName))
{
try
{
List<Quiz> tempQuzies = (List<Quiz>)binFormat.Deserialize(fStream);
try
{
CheckQuiz(tempQuzies);
Quizes = tempQuzies;
}
catch (ArgumentException)
{
Console.ForegroundColor = ConsoleColor.Red;
Console.WriteLine("Plik \"Quizes.dat\" jest uszkodzony!");
Console.ReadLine();
}
}
catch (System.Runtime.Serialization.SerializationException)
{
Console.ForegroundColor = ConsoleColor.Red;
Console.WriteLine("Plik \"Quizes.dat\" jest uszkodzony!");
Console.ResetColor();
}
}
}
Dlaczego block try catch w bloku try catch?
Najlepiej to postaraj się w ogóle oddzielić kontrolę wyjątków od głównej logiki programu.
Jeszcze drobna uwaga, to raczej używaj var tam gdzie to możliwe zamiast pełnego typu zmiennej. Tak jest przejrzyściej.
private void CheckQuiz(List<Quiz> _quizes)
{
if (_quizes == null
|| ((from q in _quizes where q.Title == String.Empty
|| q.Description == String.Empty select q).Count() > 0))
throw new ArgumentException("Title or description of question cannot be empty!");
if ((from q in _quizes where q.Questions == null select q).Count() != 0)
throw new ArgumentException("Question cannot be null");
// If every answers
if ((from q in _quizes where q.ExistEmptyAnswer() == true select q).Count() != 0)
throw new ArgumentException("Answers cannot be empty");
}
trochę dziwnie wyglądają wyrażenia LinQ w tej formie. Lepiej używać normalnych łańcuchów z metod -
_quizes.Where(x => x.ExistEmptyAnswer()).Count()
albo jeszcze lepiej:
_quizes.Any(x => x.ExistEmptyAnswer())
W klasie Quiz:
public void ShowAllQuestionsAndAnsers()
{
Console.Clear();
foreach (Question q in Questions)
{
if (q.questionHasAnswer() != 1)
{
Console.WriteLine("Musisz odpowiedzieć na wszystkie pytania!");
return;
}
}
ConsoleColor prevBGColor = Console.BackgroundColor;
ConsoleColor wrongAnswerBGColor = ConsoleColor.Red;
ConsoleColor correctAnswerBGColor = ConsoleColor.Green;
ConsoleColor prevFGColor = Console.ForegroundColor;
ConsoleColor wrongAnswerFGColor = ConsoleColor.Black;
ConsoleColor correctAnswerFGColor = ConsoleColor.Black;
// If any answer has no answer, or any answer has more than one answer
foreach (Question q in Questions)
{
Console.ForegroundColor = prevFGColor;
Console.BackgroundColor = prevBGColor;
Console.WriteLine($"\t\t**********\t{q.Title.ToUpper()}\t**********\n");
for (int i = 0; i < q.Answers.Length; i++)
{
// If answer is selected and incorrect
if (q.Answers[i].IsSelected // Check only selected answers
&& !q.Answers[i].SelectedIsCorrect())
{
Console.BackgroundColor = wrongAnswerBGColor;
Console.ForegroundColor = wrongAnswerFGColor;
Console.Write($"[{i + 1}]. {q.Answers[i].Title}\n");
continue;
}
// Correct answer
if (q.Answers[i].IsSelected // Check only selected answers
&& q.Answers[i].SelectedIsCorrect())
{
Console.ForegroundColor = correctAnswerFGColor;
Console.BackgroundColor = correctAnswerBGColor;
Console.Write($"[{i}]. {q.Answers[i].Title}\n");
continue;
}
Console.ForegroundColor = prevFGColor;
Console.BackgroundColor = prevBGColor;
Console.Write($"[{i}]. {q.Answers[i].Title}\n");
}
// Not selected answer, and not correct
Console.BackgroundColor = prevFGColor;
Console.BackgroundColor = prevBGColor;
Console.WriteLine();
}
Console.ResetColor();
Console.ReadLine();
}
Źle wygląda ten foreach. Głównie przez te zmiany kolorów. Wydzieliłbym do osobnej klasy całą metodę i podzielił na mniejsze metody każdą odpowiedź.
Na razie tyle.
Jeśli jesteś studentem to polecam zainstalować Resharper od JetBrains, sporo może pomóc przy tworzeniu kodu w c#, a będąc studentem ma się licencję na rok za darmo.
