3

I'm currently trying to learn C# and want to enhance my understanding of Object Oriented Programming (OOP). I'm hoping to accomplish this by experimenting with a small program that keeps track of my school assignments/information. I've constructed classes relating to my Institutions, Semesters(Terms), Courses, and Assignments. My question is whether or not I have created and implemented my classes correctly in regards to the information they represent. My thinking suggests that my classes should not inherit from one another in a parent/child like fashion because they are physically unrelated. However accessing objects through multilevel lists seems impractical (I think).. Is there a better way of doing this that doesn't force me to access object by iterating through collections, but still implements OOP best practices?


Program Source Code

Entry Point / Main

static void Main(string[] args)
{
    List<Institution> Institutions = new List<Institution>();
    Institutions.Add(new Institution("My college"));
    Institutions[0].AddNewTerm("2016", "Spring");
    Institutions[0].Terms[0].AddNewCourse("Math 210");
    Institutions[0].Terms[0].Courses[0].AddNewAssignment("Chapter 1");
    MessageBox.Show(Institutions[0].Terms[0].Courses[0].Assignments[0].Name);
}

Class List

Institution

class Institution
{
    public string Name { get; set; }
    public List<Term> Terms { get; set; }

    public Institution(string UP_Name)
    {
        this.Name = UP_Name;
        this.Terms = new List<Term>();
    }

    public void AddNewTerm(string NewTermYear, string NewTermSeason)
    {
        Terms.Add(new Term(NewTermYear, NewTermSeason));
    }
}

Term

class Term
{
    public string Name { get; set; }
    public string Year { get; set; }
    public string Season { get; set; }
    public List<Course> Courses { get; set; }

    public Term(string NewSeason, string NewYear)
    {
        this.Season = NewSeason;
        this.Year = NewYear;
        this.Courses = new List<Course>();
        this.Name = (this.Season + " " + this.Year);

    }

    public void AddNewCourse(string NewCourseName)
    {
        this.Courses.Add(new Course(NewCourseName));
    }
}

Course

class Course
{
    public string Name { get; set; }

    public List<Assignment> Assignments { get; set; }

    public Course(string UP_Name)
    {
        this.Name = UP_Name;
        this.Assignments = new List<Assignment>();
    }

    public void AddNewAssignment(string NewAssignmentName)
    {
        Assignments.Add(new Assignment(NewAssignmentName));
    }

}

Assignment

class Assignment
{
    public string Name { get; set; }

    public Assignment(string UP_Name)
    {
        this.Name = UP_Name;
    }
}
  • It would be a bit cleaner if you got rid of the AddNew... methods. Since you aggregate yours lists you can just use the Add methods of the aggregated lists instead. E.g.: Institutions.Terms.Add(new Term("spring", "2016")). You should not start a variable name (Institutions) with a capital letter, those are for class names by broadly embraced convention. As an alternative approach, check out encapsulation vs aggregation and indexed properties. – Martin Maat Jan 21 '16 at 19:54
  • @MartinMaat Thank you for the insight. I'll be sure to try an follow the lower case variable name casing rule going forward. Don't want to develop any bad habits :) Would happen to know of a good post or article that expands on what you mentioned with "encapsulation vs aggregation and indexed properties" I'm not sure I know enough about it to find a good source to learn it from. Thank you! – kylepdavis Jan 21 '16 at 20:02
  • 1
    Please look at this line and know it is not a good practice. MessageBox.Show(Institutions[0].Terms[0].Courses[0].Assignments[0].Name); Read up on the Law of Demeter en.wikipedia.org/wiki/Law_of_Demeter – David Basarab Jan 21 '16 at 20:56
  • @DavidBasarab wow thanks! I will work on this and update my code! – kylepdavis Jan 21 '16 at 21:09
  • I used the wrong term, it is aggregation vs containment. Google that and you will find the sources.That wiki David points at is about the difference too. With aggregation you get a hold of the child object, with containment you don't. And one is not necessarily better than the other. It depends what is more important, often it is about simplicity against flexibility. You can use indexed properties to get some of the direct syntax of aggregation while applying containment. msdn.microsoft.com/en-us/library/aa288464(v=vs.71).aspx – Martin Maat Jan 21 '16 at 21:16
4

It's not bad for a newbie. You're right to be concerned about accessing objects through multilevel lists. Also, hard coding the indexes will hurt you eventually (but it's OK for now).

You might try changing AddNewxxx() to return what was added. For example, if you change Term's AddNewClass to

public Course AddNewCourse(string NewCourseName)
{
    Course result = new Course(NewCourseName);
    this.Courses.Add(result);
    return result;
}

in your Main() function you'll be able to do something like

var mathCourse = Institutions[0].Terms[0].AddNewCourse("Math 210");
mathCourse.AddNewAssignment("Chapter 1");
mathCourse.AddNewAssignment("Chapter 2");

and so on.

Be patient; there is a lot to learn.

2

One thing I would like to point out is that all your Add methods are not necessary. If you expose the List<T> like you do, you can use them for adding. And you can use the collection initializer syntax for that:

var institutions = new List<Institution>
{
    new Institution("My college")
    {
        Terms =
        {
            new Term("2016", "Spring")
            {
                Courses =
                {
                    new Course("Math 210")
                    {
                        Assignments =
                        {
                            new Assignment("Chapter 1")
                        }
                    }
                }
            }
        }
    }
};

On first sight, this does not look any better than your original, but I believe it reflects the intention of the code better. (And it probably won't look as bad in more realistic code.)

  • I really like this. Another option would be for the Institution to implement IEnumerable<Term> and have Add(Term) so you can use collection initializers directly. That would also allow you to hide the Lists (it's a bad idea to have them exposed) and keep the neat syntax. – Zdeněk Jelínek Jan 21 '16 at 21:33
  • @ZdeněkJelínek I hate to ask this of you since you were kind enough to provide a recommendation in the first place but most of what you just said is escaping me. Is there a code snippet you might could share if you have the time? – kylepdavis Jan 21 '16 at 21:39
  • @ZdeněkJelínek Logically, an institution is not a collection of terms, so I don't think what you're proposing should be done, even if the result would be more succinct code. – svick Jan 21 '16 at 21:42
  • @kylepdavis This article has an explanation about how collection initializers work and how can you make them work with your own classes, should you wish to do that. – svick Jan 21 '16 at 21:47
  • @svick I don't agree since IEnumerable is not a collection. However, it is true that having a collection initializer could break on future additions into the class so I respect that point. – Zdeněk Jelínek Jan 21 '16 at 21:47
2

I do have some further insight that I can provide. First, about your classes, since the name of an institution should probably remian the same and not change, upon instantiation is the only time that you should allow external implementation to modify that string for the Name -- make it immutable from the class perspective. This would be proper encapsulation; a getter which is publicly accessible and any setter functionality as private. Same principle applies to other fields within other classes. Additionally, overriding the ToString() method for user-defined objects is usually always fairly useful in cases where the object has a string representation.

Keep in mind, you can also create your own collection types by implementing a class that implements IList or ICollection for instance, among other interfaces. If you want your object to be enumerable via a foreach loop then consider IEnumerable as well. Unless you have specific guidelines as to how your class should be used, the idea is generally to keep the implementation as general as possible for scalability reasons.

As for the naming conventions, there are .NET naming conventions that you might want to follow and make part of your habit from MSDN (https://msdn.microsoft.com/en-us/library/ms229002(v=vs.110).aspx). Typically locals use camelCase rather than PascalCase.

If your intention is to gear the implementation towards strings, then indexed properties as mentioned before may be the best case for this. As most examples show, the paradigm looks a lot similar to any of the .NET provided TryParse() methods to avoid an exception from being thrown but allow indication of whether the action succeeded or not too.

I don't see a reason for Aggregation here because the heirchy to me looks like it should be as follows: Institution -> Semester -> Terms -> Courses -> Assignments. None of the lower level objects should exist without a parent in my opinion. A semester from my past experience was the equivalent of 2 terms. What you probably want in this case in terms of object relationship is Composition.

1

You're right not to use inheritance, and in fact I think your design is fine. Let's take a look at one of your uses of your code.

MessageBox.Show(Institutions[0].Terms[0].Courses[0].Assignments[0].Name);

While the [0] uses here are standard for testing, in most uses of a class-based design, you would not be accessing the lists directly by some index, eg writing the number 0 into your code. Instead, you would have lots of "for-each" loops that are performing some important function on all Terms, and then all Courses in that term. Those operations will also look cleaner when the looping is separated into multiple tabbed {} blocks, rather than all in one line.

One-to-Many object mapping is a very important and common concept in programming in general. Your operating system has many Users (usually just one). Each User can run many Programs simultaneously. Those Programs can run multiple Threads...etc

You won't ever get away from the need to use collections, although programming has evolved to make them easier to use. C# in particular has a mechanism called "LINQ" that lets you quickly create "pretend-collections" that represent all parts of another collection, but changed in some way. That's worth reading up on its own, though.

Response to comment...

That's already how all objects (in C# anyway) work; only by references. Let's look at this example.

Course course101 = Institutions[0].Terms[0].Courses[5];
Console.Writeline(Institutions[0].Terms[0].Courses[5].Name); // "C# 101"
course101.Name = "Boring Class";
Console.Writeline(Institutions[0].Terms[0].Courses[5].Name); // "Boring Class"

The only time you actually have a new, separate object is when you call new Course(). Otherwise, the only things you are passing around as variables are references. So in case you were worried you'd need to pass around "institutionIndex, termIndex, courseIndex" for each function, definitely don't worry about that.

  • I'm not sure if this will make sense but what are your thoughts on making the List collections somehow hold something like references to objects instead of the objects themselves? That way I wouldn't have to get inside the the collections to edit them but still have them make sense in a relational type of way? Is this a thing? – kylepdavis Jan 21 '16 at 19:40
  • @kylepdavis Understood; added more to my answer that might hopefully help make sense of how object references work. – Katana314 Jan 21 '16 at 19:53

Your Answer

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

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