0
\$\begingroup\$

I wanted to know if i have used abstraction and encapsulation effectively here, and if not where can i improve thank you. (follow up from my unworking previous version)

MAIN

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;

namespace Voting_System
{
    class Program
    {  
        static void Main(string[] args)
        {
            // Puts each party into a list of Party and display Name + Votes
            Console.WriteLine("Type the name of the file your data is in with the .txt extension : ");
            string data = Console.ReadLine();

            // Make list of party classes to hold all parties from data.txt file
            List<Party> partys = FindData(data);

            // Ask user for thresh hold and also calculate total votes
            Console.WriteLine("What is the threshold for partys (%) (round number) ?");
            int threshHold = Convert.ToInt32(Console.ReadLine());

            // Calcutions for Dhon't method
            var votesAndSeats = GetSumVotesAndSeats(data);
            DisplayPercentageVotes(partys, threshHold, votesAndSeats.Item1);
            CalculateDhondt(partys, votesAndSeats.Item2);
            DisplayWinningParties(partys);

            Console.ReadKey();
        }

        // takes input of the name of the data file and outputs the required data
        private static List<Party> FindData(string exactpath)
        {
            // Store values in a list of string
            List<string> file = File.ReadAllLines(exactpath).ToList();
            List<Party> partys = new List<Party>();
            foreach (string line in file.Skip(3))
            {
                string[] items = line.Split(',');
                Party p = new Party(items[0], Convert.ToInt32(items[1]), items.Skip(2).ToArray());
                partys.Add(p);
            }
            return partys;
        }

        // Print out all partys that have seats and there properties to console
        private static void DisplayWinningParties(List<Party> partys)
        {
            foreach (Party p in partys)
            {   
                if (p.SeatsAmount > 0)
                {
                    Console.WriteLine(p);
                }
            }
        }

        // Find total votes for all parties and number of seats to be allocated
        private static (int,int) GetSumVotesAndSeats(string exactpath)
        {
            List<string> file = File.ReadAllLines(exactpath).ToList();
            int totalElectionVotes = Convert.ToInt32(file[2]);
            int numOfSeatAllocation = Convert.ToInt32(file[1]);

            return (totalElectionVotes, numOfSeatAllocation);
        }

        // Displays percent of votes for each party that meets the threshold
        private static void DisplayPercentageVotes(List<Party> partys, int threshold, int totalvotes)
        { 
            Console.WriteLine($"Parties that meet the {threshold}% threshold :");
            foreach (Party p in partys.ToList())
            {
                if (p.PercentOfVotes(totalvotes) > threshold)
                {
                    Console.WriteLine($"{p.Name} has {Math.Round(p.PercentOfVotes(totalvotes), 2)}" +
                                      $" % of total votes.");
                }
                else
                {
                    partys.Remove(p);
                }
            }
        }

        // Method to do the main calculations of the Dhon't method
        private static void CalculateDhondt(List<Party> partys, int seatsCount)
        {
            // Find intial party with highest votes
            Party biggestVote = partys.Aggregate((v1, v2) => v1.Votes > v2.Votes ? v1 : v2);
            biggestVote.SeatsAmount += 1;
            biggestVote.DivideParty();

            // Keep looping through partys and applying dhond't method until all seats are taken
            int totalSeatsCount = 0;
            while (totalSeatsCount != seatsCount)
            {
                // If we havent reached desired seats count reset the total seats variable
                totalSeatsCount = 0;

                Party biggestVotes = partys.Aggregate((v1, v2) => v1.NewVotes > v2.NewVotes ? v1 : v2);
                Console.WriteLine(biggestVotes);
                biggestVotes.SeatsAmount += 1;
                biggestVotes.DivideParty();

                foreach (Party p in partys)
                {
                    totalSeatsCount += p.SeatsAmount;
                }
            }
            Console.WriteLine($"\n{seatsCount} seats allocated :");
        }
    }
}

PARTY CLASS

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Voting_System
{
    class Party
    {
        // Fields
        private string _name = "unknown";
        private int _votes;
        private string[] _seatsCodeValues;
        private int _newVotes;
        private int _seatsAmount;

        // Properties 
        public string Name
        {
            get { return _name; }
            private set { _name = value; }
        }

        public int Votes
        {
            get { return _votes; }
            private set { _votes = value; }
        }

        public string[] SeatsCodeValues
        {
            get { return _seatsCodeValues; }
            private set { _seatsCodeValues = value; }
        }

        public int NewVotes
        {
            get { return _newVotes; }
            private set { _newVotes = value; }
        }

        public int SeatsAmount
        {
            get { return _seatsAmount; }
            set { _seatsAmount = value; }
        }

        // Constructor for party class 
        public Party(string name, int votes, string[] seatsCodeValues)
        {
            Name = name;
            Votes = votes;
            NewVotes = votes;
            SeatsCodeValues = seatsCodeValues;
        }


        // Returns percentage of votes for your party
        public double PercentOfVotes(double totalVotes) => (Votes / totalVotes) * 100;

        // When ever you print the object of this class return this
        public override string ToString()
        {
            return $"Name: {Name}, Votes: {Votes}, {SeatsAmount} " +
                   $"Seats Values : {string.Join(",", SeatsCodeValues.Take(SeatsAmount))}";
        }

        // Applies Dhond't method of division 
        public void DivideParty()
        {
            NewVotes = Votes / (1 + SeatsAmount);
        }
    }
}
```
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Please mention in your post that this question is a follow-up of your previous one \$\endgroup\$ – Peter Csala Mar 3 at 8:27
  • \$\begingroup\$ @PeterCsala i have now. \$\endgroup\$ – Mj _ Mar 3 at 10:42
2
\$\begingroup\$

Quick remarks:

  • Whenever I create a Console app, the first thing I do is add a Runner class with a public void Execute() method, and then call that from the Main() of the Program class. And then the whole logic is in that Runner class.

  • Your Program class is 100+ lines long, and contains various methods. Each of them does something fairly specific, but I'd actually move some of them to their own class (e;g. the ones that read the file data).

  • Follow the Microsoft guidelines WRT naming. For instance exactpath is a compound word, and thus should be exactPath. You also should not use non-alphanumeric characters, e.g. the underscore in Voting_System.

  • Use descriptive names. exactPath strikes me as odd: is there an inexactPath? data in string data = Console.ReadLine(); is wrong on so many levels: it is too generic a name and doesn't describe what it is. FindData is too generic a method name.

  • Console.WriteLine("Type the name of the file your data is in with the .txt extension : "); seems to require the user to type a file name, but it is clear you expect a path. That is a major difference.

  • Comments should be sparse and should tell me why, not what. For instance // Store values in a list of string doesn't tell me anything your code hasn't already told me. Same for // Constructor for party class and most other comments.

  • Use proper English. partys is wrong, it's parties.

  • Do not needlessly abbreviate. While the "p" in foreach (Party p in partys) isn't that bad and the context gives me enough information, it still would be better to just use the word party.

  • DisplayPercentageVotes does more than what its name says: it also alters the contents of partys. Thus it should be two methods.

  • Why does Party have these "old style" getter/setters?

  • Many of your foreach loops could be replaced with LINQ calls.

  • You don't check the user input. You don't check whether the user-provided file path exists, you don't check whether the contents of the file are structured properly,...

\$\endgroup\$
5
  • \$\begingroup\$ I appreciate the help but alot of the stuff likemlinq and error handling wasnt expected by us,as this is our first project in our c# module \$\endgroup\$ – Mj _ Mar 3 at 14:57
  • \$\begingroup\$ What do you mean by make a class for just the read file data? and when you say old style getters and setters should i do something like public string Name { get; private set; } instead? would this still meet encapsulation and abstraction pillars? \$\endgroup\$ – Mj _ Mar 3 at 15:13
  • \$\begingroup\$ An alternative way of doing encapsulation is dotnettutorials.net/lesson/encapsulation-csharp , but honestly, I haven't seen such code in "real life" in ages. If your teacher requires you to write it that way (with private fields etc.), you'll need to do it, but it isn't a coding style you see often these days (except for WPF and Unity etc.). \$\endgroup\$ – BCdotWEB Mar 3 at 16:31
  • \$\begingroup\$ Ok thanks,also can you explain why i would need to make a class for the file handling code segments? \$\endgroup\$ – Mj _ Mar 3 at 17:04
  • \$\begingroup\$ It's what I would do, for a simple reason: I don't like long class files where I need to scroll from method to method. Sometimes that is inevitable, but as much as possible I try to move specific logic to its own class file. Reading a file and converting its contents to a list of custom objects is something I'd move to its own class, for instance. \$\endgroup\$ – BCdotWEB Mar 3 at 17:15

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.