C# Intermediate course Exercise-2 : Complete Solution: Feedback Request

Hello, it’s frustrating that Josh no longer provides solutions, I cannot compare mine to the ideal solution. Therefore, would people mind giving feedback on my solution and also highlighting areas of improvemnt:

namespace ConsoleApp1
{
    public class Post
    {
        private string? _title;
        private string? _description;
        private readonly DateTime? _created;
        private int _vote;

        public Post(string postTitle, string postDescription)
        {
            _created = DateTime.Now;
            CreatePost(postTitle, postDescription);
            _vote = 0;
        }

        private void CreatePost(string postTitle, string postDescription)
        {
            if (string.IsNullOrWhiteSpace(postTitle) || string.IsNullOrWhiteSpace(postDescription))
            {
                throw new Exception(postTitle);
                throw new Exception(postDescription);
            }
            if (postTitle != null && postDescription != null)
            {
                _title = postTitle;
                _description = postDescription;
            }

        }

        public void PostUpVote()
        {
            _vote += 1;
        }

        public void PostDownVote()
        {
            _vote -= 1;
        }

        public string GetPostTitle()
        {
            if (!string.IsNullOrWhiteSpace(_title))
            {
                return _title;
            }
            else
            {
                throw new Exception("Error: No Title Set");
            }
        }

        public string GetPostDescription()
        {
            if (!string.IsNullOrWhiteSpace(_description)) {
                return _description;
            } else
            {
                throw new Exception("Error: No Description Set");
            }
        }

        public int GetPostVotes()
        {
            return _vote;
        }

    }
}



namespace ConsoleApp1
{
    internal class Program
    {
        static void Main(string[] args)
        {
            var newPost = new Post("This is a title","Hello, this is a new post");
            newPost.PostUpVote();
            newPost.PostUpVote();
            newPost.PostUpVote();
            newPost.PostUpVote();
            newPost.PostUpVote();
            Console.WriteLine("Title: " + newPost.GetPostTitle());
            Console.WriteLine("Description: " + newPost.GetPostDescription());
            Console.WriteLine("Votes: " + newPost.GetPostVotes());
            newPost.PostDownVote();
            Console.WriteLine("Title: " + newPost.GetPostTitle());
            Console.WriteLine("Description: " + newPost.GetPostDescription());
            Console.WriteLine("Votes: " + newPost.GetPostVotes());
        }
    }
}

A few comments:

You check for null and empty title and description in the constructor already so I would remove those checks from the getters.

        public string GetPostTitle()
        {
            return _title;
        }

        public string GetPostDescription()
        {
            return _description;
        }

Also, I would move the content of CreatePost into the constructor itself (maybe except for the validation):

        public Post(string postTitle, string postDescription)
        {
            CheckTitle(post title);
            CheckDescription(postDescription);
            _created = DateTime.Now;
            _title = postTitle;
            _description = postDescription;
            _vote = 0;
        }

Validation should ideally throw better exceptions than Exception and use better error messages. I think ArgumentException is correct here:

private CheckTitle(string postTitle) {
  if (string.IsNullOrWhiteSpace(postTitle)
  {
    throw new ArgumentException("title must be a non-empty string");
  }
}

private CheckDescription(string postDescription) {
  if (string.IsNullOrWhiteSpace(postDescription)
  {
    throw new ArgumentException("description must be a non-empty string");
  }
}

There are probably more things that could be commented on by a more experienced C# developer, but there is no such thing as an “ideal” solution. Mosh could have provided a canonical solution, but he chose not to.

2 Likes

Hi there,
maybe this can help more pp:

Program.cs file:

using System;
using System.Collections.Generic;

namespace post
{
    public class Program
    {
        public static void Main(string[] args)
        {
            // Create a new Post object
            Post post = new Post("My First Post", "This is the description of my first post.");

            // Up-vote and down-vote the post a few times
            post.UpVote();
            post.UpVote();
            post.DownVote();
            post.UpVote();

            // Display the current vote value
            Console.WriteLine("Post Title: " + post.Title);
            Console.WriteLine("Description: " + post.Description);
            Console.WriteLine("Date Created: " + post.DateTimeCreated);
            Console.WriteLine("Current Vote Count: " + post.GetVoteCount());
        }
    }
}

Post.cs file:

using System;

public class Post
{
    // Properties for title, description, and the date/time it was created
    public string Title { get; set; }
    public string Description { get; set; }
    public DateTime DateTimeCreated { get; private set; }

    // Private field to keep track of the vote count
    private int voteCount;

    // Constructor to initialize the title, description, and creation date
    public Post(string title, string description)
    {
        Title = title;
        Description = description;
        DateTimeCreated = DateTime.Now;
        voteCount = 0; // Initial vote count is zero
    }

    // Method to up-vote the post
    public void UpVote()
    {
        voteCount++;
    }

    // Method to down-vote the post
    public void DownVote()
    {
        voteCount--;
    }

    // Method to get the current vote count
    public int GetVoteCount()
    {
        return voteCount;
    }
}

There is no one way to solve the same problem. Also a newer developer will for sure not have to tool to provide a solution that compare with the goats.
Don’t worry too much about “ideal solution” while you are taking the course. You’ll have plenty of time for that later…

You already had some feedback.

What I see myself is in the CreatePost method, you are throwing an exception if either title or description is null so why embedding the rest of the code in another if ?

Your first if block is a guard clause. If anything gets in there, you leave the method by either a return or throw statement.

Then the rest of the method is the “happy path”.

Depending on the version of DotNet you are using, you could use expression body instead of block body when you have a oneliner. It is a bit clearner.
For instance:
public void PostUpVote => _vote +=1;

It is not in the assignment I guess, but I would write a way to display a Post instead of having several Console.WriteLine() calls

Pro tip

What I would do is override the ToString() method. This is handy in many situations in dotnet from simple displays in the console Console.WriteLine(myPost); to other technologies such as WPF of simply the Visual Studio debugger. I use it often with representative data. Otherwise, it gives a very long and difficult to read name.