1
\$\begingroup\$

I am trying to implement a tiny image tagger with customized tag options in C#. The main window is as follows.

MainUIFigure

The left block is a picture box MainPictureBox and there are two buttons, PreviousButton and NextButton. Furthermore, there are some tags in a group box AttributeGroupBox. A save button SaveButton is used for saving tagged information.

The implemented functions:

  • Load images automatically in the folder which the program located.

  • Each image could be tagged.

  • The tag information of each image could be saved as a text file LabelResult.txt.

The experimental implementation

  • Attribute Class
class Attribute
{
    public enum AttributeEnum
    {
        Mountain,
        Cat
    }

    public AttributeEnum attributeEnum;

    public override string ToString()
    {
        if (this.attributeEnum.Equals(AttributeEnum.Cat))
        {
            return "Cat";
        }
        if (this.attributeEnum.Equals(AttributeEnum.Mountain))
        {
            return "Mountain";
        }
        return "";
    }
}
  • Main Winform
public partial class Form1 : Form
{
    List<string> ImagePaths;
    List<Attribute> AttributeList;
    int index = 0;
    string targetDirectory = "./";
    public Form1()
    {
        InitializeComponent();

        
        System.IO.FileSystemWatcher watcher = new FileSystemWatcher()
        {
            Path = targetDirectory,
            Filter = "*.jpg | *.jpeg| *.bmp | *.png"
        };
        // Add event handlers for all events you want to handle
        watcher.Created += new FileSystemEventHandler(OnChanged);
        // Activate the watcher
        watcher.EnableRaisingEvents = true;

        ImagePaths = new List<string>();
        
        string[] fileEntries = System.IO.Directory.GetFiles(targetDirectory);
        foreach (string fileName in fileEntries)
        {
            string filenameExt = System.IO.Path.GetExtension(fileName);
            if (filenameExt.Equals(".jpg") ||
                filenameExt.Equals(".jpeg") ||
                filenameExt.Equals(".bmp") ||
                filenameExt.Equals(".png")
                )
            {
                ImagePaths.Add(fileName);
            }
        }
        MainPictureBox.SizeMode = PictureBoxSizeMode.Zoom;
        MainPictureBox.Image = Image.FromFile(ImagePaths[0]);

        AttributeList = new List<Attribute>();
        
    }

    private void OnChanged(object source, FileSystemEventArgs e)
    {
        ImagePaths.Clear();
        string[] fileEntries = System.IO.Directory.GetFiles(targetDirectory);
        foreach (string fileName in fileEntries)
        {
            string filenameExt = System.IO.Path.GetExtension(fileName);
            if (filenameExt.Equals(".jpg") ||
                filenameExt.Equals(".jpeg") ||
                filenameExt.Equals(".bmp") ||
                filenameExt.Equals(".png")
                )
            {
                ImagePaths.Add(fileName);
            }
        }
    }

    private void PreviousButton_Click(object sender, EventArgs e)
    {
        index--;
        if (index <= 0)
        {
            index = 0;
        }
        MainPictureBox.Image = Image.FromFile(ImagePaths[index]);
        GC.Collect();
    }

    private void NextButton_Click(object sender, EventArgs e)
    {
        NextAction();
    }

    private void NextAction()
    {
        index++;
        if (index >= ImagePaths.Count)
        {
            index = ImagePaths.Count - 1;
        }
        MainPictureBox.Image = Image.FromFile(ImagePaths[index]);
        GC.Collect();
    }

    private void SaveButton_Click(object sender, EventArgs e)
    {
        Attribute attribute = new Attribute();
        
        if (radioButton1.Checked)
        {
            attribute.attributeEnum = Attribute.AttributeEnum.Mountain;
        }
        if (radioButton2.Checked)
        {
            attribute.attributeEnum = Attribute.AttributeEnum.Cat;
        }
        MessageBox.Show(attribute.ToString());
        AttributeList.Add(attribute);
        AttributeListToTxt();
        NextAction();
    }

    private void AttributeListToTxt()
    {
        StringBuilder sb = new StringBuilder();
        for (int i = 0; i < this.AttributeList.Count; i++)
        {
            sb.Append(this.ImagePaths[i] + "\t" + this.AttributeList[i].ToString() + Environment.NewLine);
        }
        File.WriteAllText("LabelResult.txt", sb.ToString());
    }
}

All suggestions are welcome.

If there is any possible improvement about:

  • Potential drawback or unnecessary overhead
  • The design of implemented methods

please let me know.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Is this WinForms? WPF? Please use better tags than something generic like "object oriented". \$\endgroup\$
    – BCdotWEB
    Commented Mar 1, 2021 at 13:47

1 Answer 1

2
\$\begingroup\$

There is plenty room for improvement

Attribute

  • There is a built-in class in the BCL with same name which help you define decorators.
    • That's why I would not recommend to call your class like this.
  • AttributeEnum: I personally don't like to include enums inside a class definition.
    • The usage is also really weird (stuttery): Attribute.AttributeEnum.Cat
  • You don't need perform branching like this: this.attributeEnum.Equals(AttributeEnum.Cat)
    • this.attributeEnum.ToString("G") would be enough. Reference

Form1

  • FileSystemWatcher is disposable, so please be sure to dispose it
  • File extensions: they are repeated at least 3times.
    • Please prefer class-wide constants
  • watcher.Created += OnChanged: This is really misleading, because watcher does have event for Changed as well.
  • System.IO.Directory.GetFiles(targetDirectory): Performing I/O operation inside the ctor is not really a good practice.
    • Please move every non-essential logic into an Init method.
  • GC.Collect();: Please do not ever call GC.Collect directly. Reference:

The GC.Collect method is called. In almost all cases, you don't have to call this method, because the garbage collector runs continuously. This method is primarily used for unique situations and testing.

  • NextAction: This naming does not provide any value.
    • Please try to use meaningful names.
  • radioButton1.Checked: Yet again please use meaningful naming.
    • For example rb_Montain would be a much better option.
  • AttributeListToTxt: Please try to name your methods in the way that they start with a verb, like: PersistAttributeListIntoTxt or SaveAttributeList, etc.
    • Why don't you simply write line-by-line instead of composing the whole content then write it once.
\$\endgroup\$

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