0
\$\begingroup\$

I started to learn C# recently by following the official Microsoft docs. I originally created the GradeBook project from Pluralsight which I am now adding my own features to.

This section lets the user input the name of the student and the subject, which creates a new gradebook file. The code that follows this logs the students' grades for each subject which can be used to calculate statistics.

The code currently works as intended but I am new to this. Does this code follow common best practices? Are there more optimized methods or flaws that I might not recognize?

public class AddStudentDetails : AddGradeBookSubjects
{

    public static void StudentDetails()
    {

        Clear();

        //WriteLineColor just uses Writeline and ConsoleColor to change the text.
        //Used to Add the users name to the GradeBook.
        WriteLineColor("####Please enter your students name.####\n", ConsoleColor.Red);
        var studentName = ReadLine();
        // add if(Regex.IsMatch(studentName, @"^[a-zA-Z]+$")) to declare valid name input?
        WriteLineColor($"{studentName} has been added to the gradebook.\n", ConsoleColor.Red);
        Clear();

        //Adds course subject to the Gradebook, SubjectName = subjectName; validates that the user input is in the string array of subjects and is the correct format. 
        WriteLineColor("Please enter the course subject.\n", ConsoleColor.Red);
        SubjectName = subjectName;
        WriteLineColor($"You have set the subject as: {SubjectName}. \n", ConsoleColor.Red);
        Clear();

        //Creates a .txt file within a folder with the name and subject of the created gradebook.
        WriteLineColor($"Gradebook selected: {studentName}: {SubjectName}. \n", ConsoleColor.Red);
        var fileName = $"{studentName} {SubjectName} GradeBook.txt";
        var path = Combine(@"H:\Repository\Gradebook Version One\Student Gradebooks", fileName);
        {
            using (var sw = CreateText(path))
            {
            }

        }
        Program.WriteLineColor($"Grade book {studentName}, {subjectName} has been created in {path}", ConsoleColor.Red);

        var book = new InMemoryBookStorage($"This book belongs to {studentName} and its subject is {SubjectName}");
        EnterStudentGrades.EnterGrades(book);

    }
}


public class AddGradeBookSubjects 
{
    public static string subjectName;

    public static string SubjectName
    {
        get { return subjectName; }
        set
        {
            List<string> addSubject = new List<string>() {"Maths", "Science", "English"};
            while (true)
            {
                Console.WriteLine("Select from the list below.");
                foreach (string item in addSubject) Console.WriteLine(item.ToString());
                var input = Console.ReadLine();
                if (addSubject.Contains(input, StringComparer.OrdinalIgnoreCase) &&
                    Regex.IsMatch(input, @"^[a-zA-Z]+$"))
                {
                    subjectName = input;
                    return;
                }
                else
                {
                    Console.Clear();
                    Console.WriteLine("Please enter a valid subject.");
                    
                }

                
            }
        }




    }
}

In the future, I plan on adding features that will not allow duplicate books of the same name and subject and also storing it in a database, rather than a text file.

New contributor
James is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
4
  • 2
    \$\begingroup\$ Probably you may add more code for review e.g. methods Clear, CreateText and WriteLineColor, class InMemoryBookStorage. The best code example is reproducible example. \$\endgroup\$ – aepot Feb 23 at 1:53
  • 1
    \$\begingroup\$ 1) The main problem here is Property setter design. Property getters and setters are mot allowed to work for a long time. Property setter can make some conversions to received value or reject the changes but it must do it fast. 2) A tip: to create file simply call File.WriteAllText(path, string.Empty) 3) Another tip: as an option to write array or list to console you can do Console.WriteLine(string.Join(Environment.NewLine, array)) with no loop at all. Where Environment.NewLine is delimiter string. ", " is valid here too. \$\endgroup\$ – aepot Feb 23 at 2:04
  • 1
    \$\begingroup\$ StudentDetails() is a bad method name (doesn't say what it does), WriteLineColor is a bad method name (the parameter shouldn't be part of the method name), subjectName shouldn't be public, why are your properties static, class names shouldn't be plural (AddGradeBookSubjects) except in very specific cases, your code mixes presentation and back-end logic (all those Console calls in the middle of setters etc.),... \$\endgroup\$ – BCdotWEB Feb 23 at 8:42
  • 2
    \$\begingroup\$ You are getting answers in comments rather than a whole answer because the question is off-topic since there is not enough code in the question, among other things the AddStudentDetails class inherits from AddGradeBookSubjects which is not define in the question. Please read How do I ask a good question?. Please edit the question, obviously people want to help you but they can't at the moment. \$\endgroup\$ – pacmaninbw Feb 23 at 12:32

Your Answer

James is a new contributor. Be nice, and check out our Code of Conduct.

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.