Best practices for decouple classes in C#

Question

So this is a lot more confusing than it has to be (I could just stick all of this in the main class with the ui event handlers) but I wanted to decouple this class for learning purposes.

Basic information:

I pulled out a bunch of code and put it in a separate class. This separate class opens files so I used exception handling. When an exception is thrown it should update the UI with an error message. To decouple this class I created an event handler and event listeners.

Questions:

1. Is this a common way to decouple classes?

2. Is this too loosely coupled where it creates too much overhead

3. Is this so decoupled that it makes it completely too complicated?

4. My friend suggested passing Form1 to the function, but I would still need to use the name of the label. So it would be less coupled, but not completely decoupled. Is this an acceptable approach?

5. Are there some other approaches that would work better?

Original class with UI event handlers:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.IO;

namespace compiler
{
public partial class Form1 : Form
{
    CompilerControls controls = new CompilerControls();
    bool ErrorFlag = false;

    public Form1()
    {
        InitializeComponent();

        //add an event listener to handle exceptions
        controls.HandleException += new ExceptionCaught(CatchException);
    }

    private void Form1_Load(object sender, EventArgs e)
    {
    }

    public void CatchException(CustomEventArgs e)
    {
        UpdateStatus(e.Message, Color.Red);
        ErrorFlag = true;
    }

    private void ctrlOpenFile_Click(object sender, EventArgs e)
    {
        DialogResult sourceFile = openFileDialog1.ShowDialog();
        if (sourceFile == DialogResult.OK)
        {
            // Read the lines into a list from the file
            controls.ReadFile(openFileDialog1.FileName);

            //Print the source file to the text box
            txtMainBox.Clear();
            txtMainBox.Text = controls.GetSourceFile();

            if (!ErrorFlag)
            {
                //Show status message and move forward
                UpdateStatus("File Opened Successfully", Color.Green);
                ctrlCreateChFile.Enabled = true;
                ctrlOpenFile.Enabled = false;
            }
            else
                ErrorFlag = false;
        }
    }

    private void UpdateStatus(string message, Color color)
    {
        lblStatus.ForeColor = color;
        lblStatus.Text = message;
    }

    private void ctrlCreateChFile_Click(object sender, EventArgs e)
    {
        //delete everything in the main text box and get/create the contents of the character file
        txtMainBox.Clear();
        txtMainBox.Text = controls.GetChFile();
        //deslect all of the text in the main text box
        txtMainBox.GotFocus += delegate { txtMainBox.Select(0, 0); };

        //if there wasn't an exception thrown
        if (!ErrorFlag)
        {
            //Show status message and move forward
            UpdateStatus("Successfully Created Character File", Color.Green);
            ctrlCreateChFile.Enabled = false;
            ctrlCreateTokens.Enabled = true;
        }
        else
            ErrorFlag = false;  //if there was an exception thrown, ignore the above statements only ONCE
    }

}
}

Decoupled class with exception handling:

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

namespace compiler
{
public delegate void ExceptionCaught(CustomEventArgs e);

class CompilerControls
{
    private List<String> fileLines = new List<string>();

    //add an event handler
    public event ExceptionCaught HandleException;

    //handle the event of an exception being thrown
    private void OnCaught(CustomEventArgs e)
    {
        if (HandleException != null)
            HandleException(e);
    }

    public void ReadFile(String FileName)
    {
        try
        {
            using (StreamReader sr = new StreamReader(FileName))
            {
                string line;
                while ((line = sr.ReadLine()) != null)
                    fileLines.Add(line);
            }
        }
        catch (IOException)
        {
            OnCaught(new CustomEventArgs("File could not be Opened"));
        }
        catch (OutOfMemoryException)
        {
            fileLines.Clear();
            OnCaught(new CustomEventArgs("File too large"));
        }
    }

    public string GetSourceFile()
    {
        string text;
        text = "/*******************************************************************" + Environment.NewLine;
        text += "/       Stephen Granet" + Environment.NewLine;
        text += "/       CS 451 Compiler" + Environment.NewLine;
        text += "/" + Environment.NewLine;
        try
        {
            //Format each line from the file and print it to the text box
            for (int i = 0; i < fileLines.Count(); i++)
            {
                text += "/       " + (i + 1) + ":  " + fileLines[i] + Environment.NewLine;
            }
        }
        catch (OutOfMemoryException)
        {
            OnCaught(new CustomEventArgs("File too large"));
            return "";
        }
        //Print footer information to the text box
        text += "/******************************************************************/";
        return text;
    }

    public String GetChFile()
    {
        String text = "";
        //Convert the fileLines into one long string, and split each character into its own array element
        char[] symbols = (string.Join("", fileLines)).ToCharArray();

        //cycle through each symbol and print it to the text box
        foreach (char symbol in symbols)
        {
            if ((symbol != '\n') && (symbol != ' ') && (symbol != '\t'))
                text += symbol + Environment.NewLine;
        }
        CreateChFile(text);
        return text;
    }
    private void CreateChFile(string content)
    {
        //Write the data to the ch.txt file
        try
        {
            File.WriteAllText("ch.txt", content);
        }
        catch (IOException)
        {
            OnCaught(new CustomEventArgs("Could not create Character File"));
        }
    }
}
}

Note: This is a homework assignment. However, I'm not asking a question on the homework part of the program. This is for my own practice.

Answer 1

Thanks to ANeves for pointing out that I could also submit my comments as an answer by invoking question 5 :)

Original comment:

Not a direct answer to your question, so a comment: your approach hides useful information from the caller. You handle the exception, which has lots of information in it, by passing much less information to an event. For example, an IOException will tell you why the file couldn't be opened; the event does not. Exceptions have a stack trace; the event does not. Sometimes it's best to let the caller handle the exception, since the caller knows best how to react to a given exceptional condition.

So I should just not check for exceptions in the decoupled class, and make whoever calls it check for exceptions? – Stephen Granet 8 mins ago

Most likely, yes. The ReadFile method takes a file path and fills a collection of strings with the lines of the file. That's pretty simple. By using the event-based pattern you've developed, you require the consumer of the method to subscribe to an event if they want to know about exceptions. The built-in exception handling mechanism, on the other hand, comes for free.

Indeed, the direct caller of ReadFile might not need to catch the exception. It might be appropriate to allow the exception to bubble up to a higher point in the call stack. In general, any given method should only handle exceptions that it knows about, and for which it has some specific course of action. (Such a course of action could be, for example, informing the user that the path was invalid and asking for new input.)

At the entry point of your application (the entry point of each thread, actually), you'll usually want a general exception handler for logging exceptions that weren't handled more specifically.

Back to your program: to decouple the ReadFile logic from the calling class, you could have the method return a List<string> rather than operate on a private member of the class. This has many advantages: easier testing and greater reusability come to mind.

Another suggestion: Since you concatenate all the lines in the end, you could skip a lot of this and use the File.ReadAllLines method instead.

In general, it seems, you might want to focus on the single responsibility principle as a way of arriving at a decoupled design.

Answer 2

Answer to question 5: you should use Stringbuilder to create your strings.

Because strings are immutable, whenever you concatenate two strings you create a third. This will quickly ramp up the resources used, as you duplicate longer and longer strings.
So, for more than a handful of concatenations, you want to use StringBuilder. (For a few, it does not matter.)

Additionally, StringBuilder has methods to concatenate line-ends.
I find it deliciously ironic that lazyness can be an incentive for excellence. :)