Home > Net >  it isn't cohesive, and it doesn't abstract away any of the implementation details
it isn't cohesive, and it doesn't abstract away any of the implementation details

Time:03-25

My professor doesn't want all my code in one class. I am new to C# as well so I don't know how to make my code cohesive and have it abstract away any of the implementation details. Here is my code I have so far. I am trying to make multiple classes because this class has too many responsibilities and I don't know how to do that.

using System;
using System.Collections.Generic;
using System.Drawing;
using System.IO;

namespace SvgGenerator
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Please enter the name of the output file.");
            string outputFile = Console.ReadLine()   ".svg";
            Console.WriteLine("Do you want to manually enter the squares or read them from a file? Man or File?");
            string fileRead = Console.ReadLine();

            if (fileRead.Trim() == "Manually" || fileRead.Trim() == "manually" || fileRead.Trim() == "Man" || fileRead.Trim() == "man")
            {
                ManInput(outputFile);
            }
            if (fileRead.Trim() == "file" || fileRead.Trim() == "File")
            {
                FileInput(outputFile);
            }
        }

        private static void FileInput(string outputFile)
        {
            Console.WriteLine("What is the name of the file?");
            string titleFileName = Console.ReadLine();
            StreamReader reader;
            reader = new StreamReader(titleFileName);
            string textFile = reader.ReadToEnd();
            reader.Close();
            string[] values = textFile.Split(',', '\n');
            List<Square> squares = new List<Square>();
            for (int i = 0; i < values.Length;)
            {
                int valueNumsX = int.Parse(values[i].Trim());
                int valueNumsY = int.Parse(values[i   1].Trim());
                Square squareQ = new Square(Color.FromName(values[i   2].Trim()), valueNumsX, valueNumsY);
                squares.Add(squareQ);
                if (i == values.Length - 3)
                {
                    SvgBuilder svgBuilder = new SvgBuilder();
                    string SVG = svgBuilder.Build(squares);

                    FileCreator Myfilecreater = new FileCreator();
                    Myfilecreater.Create(outputFile, SVG);
                }
                i = i   3;
            }
        }

        private static void ManInput(string outputFile)
        {
            Console.WriteLine("How many squares do you want in your SVG file?");
            string squareCount = Console.ReadLine();
            int numSquareCount = Convert.ToInt32(squareCount);
            Console.WriteLine("What are the colors of your squares?");
            string[] squareColor = new string[numSquareCount];

            List<Square> squares = new List<Square>();
            for (int i = 0; i < numSquareCount; i  )
            {
                squareColor[i] = Console.ReadLine();
                Square squareQ = new Square(Color.FromName(squareColor[i]), i*4, 0, 200);
                squares.Add(squareQ);
                if (i == numSquareCount - 1)
                {
                    SvgBuilder svgBuilder = new SvgBuilder();
                    string SVG = svgBuilder.Build(squares);

                    FileCreator Myfilecreater = new FileCreator();
                    Myfilecreater.Create(outputFile, SVG);
                }
            }
        }
    }
}`

CodePudding user response:

First of all you should separate classes or methods handling input from classes handling output. If is also typically a poor idea to mix UI from the functional parts, even if the the UI is a console for this case.

I would suggest using the following methods:

private static IEnumerable<Square> ReadSquaresFromFile(string filePath)
private static IEnumerable<Square> ReadSquaresFromConsole()
private static WriteToFile(IEnumerable<Square> squares, string filePath)

For such a simple program procedural programming should be just fine. You do not have to use object. But if you want to, you could for example create a interface like:

public interface ISquareSource(){
    IEnumerable<Square> Get();
}

With a file-implementation, console-implementation etc.

Note that I have used string filePath as the file source/destination. If you ever write a library or API, please ensure you have an overlay that takes a stream. It is super annoying to have some data in memory and being forced to write it to a temporary file just because some developer only imagined reading from actual files.

You could also consider using switch statements for handling input, for example

switch(fileRead.Trim().ToLower()){
    case "manually":
        ...
    break;
    case "file":
        ...
    break;
    default:
        Console.WriteLine("Invalid input, expected 'manually' or 'file'
    break;

CodePudding user response:

Cohesion is the idea that code that does the same thing belongs together, and code that doesn't do the same thing doesn't belong together.

So, let's consider the FileInput function. At a glance, we can see that it does the following:

  1. Prompts the user for a file name to load.
  2. Opens the file.
  3. Reads all of its content into memory.
  4. Closes the file.
  5. Parses the file content into an array of strings.
  6. For each item in the array:
    1. Parses some integral values.
    2. Creates a Square object from those values.
    3. If the index of the current item is equal to the length of the array less 3:
      1. Instantiates a new SvgBuilder.
      2. Invokes its Build method.
      3. Instantiates a new FileCreator.
      4. Invokes its Create method.

There's a lot going on here. Essentially, there are three separate kinds of work going on here that could be broken out into individual functions:

  1. User input (arguably this could be part of the main function).
  2. Call file deserialization function (reads the input file into memory and returns it as an array of strings).
  3. Call main processing function (iterates over the array)
    1. Performs calculations and creates of Square object.
    2. If index of the current item is array length less 3:
      1. Call Build SVG File function
      2. Call File Creation function.

This is what your instructor is getting at.

  •  Tags:  
  • c#
  • Related