1
\$\begingroup\$

Introduction

I want to create an Inventory App in C#. I was thinking of someone who sells a few products and only needs to have a few product properties in the program to search or sort by, and then click on ‘sold’ or click on ‘order’. An Excel export is also to be realised.

Today, I wrote my search function. The search term is entered in a text box. One can enclose the search term in inverted commas "to find exactly this whole sentence"; one can also write A Or B to find both products; one can also write A -B to find A, but not B. So with the latter example, you can exclude certain words.

There is a list of entries in my programme. It contains instances of the class Entry. The class Entry also includes an instance of the class Product.

At the end of the search function, however, another list – search_results – is filled. This is sorted.

My question

I would like to know if there is anything I can do to improve this. It seems like a lot of lines of code to me...

Edit: This is a Windows Forms App (.NET Framework) with .Net Framework 4.8

enter image description here

FormMain.cs

/// <summary>
        /// all Data
        /// </summary>
        public static readonly List<Entry> AllEntries = new List<Entry>();

        /// <summary>
        /// Suchergebnisse
        /// </summary>
        private List<Entry> search_results = null;

private enum Esort_criterion
        {
            alphabetical,
            by_Creation_Time
        }
        private Esort_criterion Sort_Criterion;
private void TextBox_seek_KeyUp(object sender, KeyEventArgs e)
        {
            if (e.KeyCode == Keys.Enter && !System.String.IsNullOrEmpty(TextBox_seek.Text))
            {
                seek(TextBox_seek.Text);
            }
        }
private async void seek(string SearchTerm)
        {

            // Remove entries from the previous search (if there are any).
            if (search_results != null && search_results.Count > 0)
            {
                foreach (Entry E in search_results)
                {
                    foreach (Bitmap Image in E.TheProduct.ListOfPhotos)
                    {
                        if (Image != null)
                        {
                            Image.Dispose();
                        }
                    }
                }
                search_results.Clear();
            }

            List<Entry> sorted_search_results = await Task.Run(() => seekAsync(SearchTerm));

            if (sorted_search_results == null)
            {
                return;
            }

            if (sorted_search_results.Count == 0)
            {
                MessageBox.Show("no matches found",
                                "Inventory App – Suche",
                                MessageBoxButtons.OK,
                                MessageBoxIcon.Information);// Do not write ‘return’
            }

            ListBox1.Items.Clear();
            ListBox1.Items.AddRange(search_results.Select(x => $"{x.TheProduct.BrandName}{"\t"}{x.TheProduct.ModelName}{"\t"}{x.TheProduct.DescriptionOrNotes}{"\t"}{Math.Round(x.TheProduct.SellingPrice, 2)} €").ToArray());
        }

        private List<Entry> seekAsync(string SearchTerm)
        {
            search_results = new List<Entry>();

            char[] CharArray = SearchTerm.ToCharArray();
            string[] StringArray1 = SearchTerm.Split(new[] { " Or ", " or " }, 3, StringSplitOptions.None);
            string[] StringArray2 = SearchTerm.Split(new[] { " -" }, 3, StringSplitOptions.None);

            // "Mehrere Wörter zusammenhängend" (exaktes Ergebnis)
            if (CharArray.First() == '"' && CharArray.Last() == '"')
            {
                foreach (Entry E in AllEntries)
                {
                    if (E.TheProduct.BrandName.Contains(SearchTerm.Split('"')[1]) ||
                        E.TheProduct.ModelName.Contains(SearchTerm.Split('"')[1]))
                    {
                        List<Bitmap> NewBitmaps = new List<Bitmap>();

                        foreach (string FullFileName in E.TheProduct.ListOfPathsOfThePhotos)
                        {
                            if (System.IO.File.Exists(FullFileName))
                            {
                                NewBitmaps.Add(new Bitmap(FullFileName));
                            }
                        }
                        // NEUE Instanzen erzeugen, sonst wird auf denselben Speicherbereich gezeigt.
                        // Create NEW instances, otherwise the same memory area is pointed to. 
                        search_results.Add(new Entry(
                                            E.Employee_who_created_Entry,
                                            E.CreationTime,
                                            E.Changed_by_Employee,
                                            E.LastChangeTime,
                                            new Product(
                                                E.TheProduct.BrandName,
                                                E.TheProduct.ModelName,
                                                E.TheProduct.Size,
                                                E.TheProduct.DescriptionOrNotes,
                                                NewBitmaps,
                                                E.TheProduct.ListOfPathsOfThePhotos,
                                                E.TheProduct.PurchasePrice,
                                                E.TheProduct.SellingPrice)
                                            )
                            );
                        search_results.Last().TheProduct.are_images_loaded = (new bool[E.TheProduct.ListOfPathsOfThePhotos.Count]);
                    }
                }
            }
            else if ((SearchTerm.Contains(" Or ") ^ SearchTerm.Contains(" or ")) && StringArray1.Length >= 2)
            {
                foreach (Entry E in AllEntries)
                {
                    foreach (string part in StringArray1)
                    {
                        if (E.TheProduct.BrandName.Contains(part) ||
                            E.TheProduct.ModelName.Contains(part))
                        {
                            List<Bitmap> NewBitmaps = new List<Bitmap>();

                            foreach (string FullFileName in E.TheProduct.ListOfPathsOfThePhotos)
                            {
                                if (System.IO.File.Exists(FullFileName))
                                {
                                    NewBitmaps.Add(new Bitmap(FullFileName));
                                }
                            }
                            // NEUE Instanzen erzeugen, sonst wird auf denselben Speicherbereich gezeigt.
                            // Create NEW instances, otherwise the same memory area is pointed to. 
                            search_results.Add(new Entry(
                                               E.Employee_who_created_Entry,
                                               E.CreationTime,
                                               E.Changed_by_Employee,
                                               E.LastChangeTime,
                                               new Product(
                                                        E.TheProduct.BrandName,
                                                        E.TheProduct.ModelName,
                                                        E.TheProduct.Size,
                                                        E.TheProduct.DescriptionOrNotes,
                                                        NewBitmaps,
                                                        E.TheProduct.ListOfPathsOfThePhotos,
                                                        E.TheProduct.PurchasePrice,
                                                        E.TheProduct.SellingPrice)
                                                    )
                                    );
                            search_results.Last().TheProduct.are_images_loaded = (new bool[E.TheProduct.ListOfPathsOfThePhotos.Count]);
                        }
                    }
                }
            }
            else if (StringArray2.Length >= 2) // A, but NOT B
            {
                foreach (Entry E in AllEntries)
                {

                    if ((E.TheProduct.BrandName.Contains(StringArray2[0]) ||
                         E.TheProduct.ModelName.Contains(StringArray2[0])) &&
                         !E.TheProduct.BrandName.Contains(StringArray2[1]) &&
                         (!E.TheProduct.ModelName.Contains(StringArray2[1])))
                    {
                         List<Bitmap> NewBitmaps = new List<Bitmap>();

                         foreach (string FullFileName in E.TheProduct.ListOfPathsOfThePhotos)
                         {
                             if (System.IO.File.Exists(FullFileName))
                             {
                                 NewBitmaps.Add(new Bitmap(FullFileName));
                             }
                         }
                         // NEUE Instanzen erzeugen, sonst wird auf denselben Speicherbereich gezeigt.
                         // Create NEW instances, otherwise the same memory area is pointed to. 
                         search_results.Add(new Entry(
                                            E.Employee_who_created_Entry,
                                            E.CreationTime,
                                            E.Changed_by_Employee,
                                            E.LastChangeTime,
                                            new Product(
                                                        E.TheProduct.BrandName,
                                                        E.TheProduct.ModelName,
                                                        E.TheProduct.Size,
                                                        E.TheProduct.DescriptionOrNotes,
                                                        NewBitmaps,
                                                        E.TheProduct.ListOfPathsOfThePhotos,
                                                        E.TheProduct.PurchasePrice,
                                                        E.TheProduct.SellingPrice)
                                                    )
                                    );
                         search_results.Last().TheProduct.are_images_loaded = (new bool[E.TheProduct.ListOfPathsOfThePhotos.Count]);
                    }       
                }
            }
            else
            {
                foreach (Entry E in AllEntries)
                {
                    if (E.TheProduct.BrandName.Contains(SearchTerm) ||
                        E.TheProduct.ModelName.Contains(SearchTerm))
                    {
                        List<Bitmap> NewBitmaps = new List<Bitmap>();

                        foreach (string FullFileName in E.TheProduct.ListOfPathsOfThePhotos)
                        {
                            if (System.IO.File.Exists(FullFileName))
                            {
                                NewBitmaps.Add(new Bitmap(FullFileName));
                            }
                        }
                        // NEUE Instanzen erzeugen, sonst wird auf denselben Speicherbereich gezeigt.
                        // Create NEW instances, otherwise the same memory area is pointed to. 
                        search_results.Add(new Entry(
                                           E.Employee_who_created_Entry,
                                           E.CreationTime,
                                           E.Changed_by_Employee,
                                           E.LastChangeTime,
                                           new Product(
                                                    E.TheProduct.BrandName,
                                                    E.TheProduct.ModelName,
                                                    E.TheProduct.Size,
                                                    E.TheProduct.DescriptionOrNotes,
                                                    NewBitmaps,
                                                    E.TheProduct.ListOfPathsOfThePhotos,
                                                    E.TheProduct.PurchasePrice,
                                                    E.TheProduct.SellingPrice)
                                                    )
                                    );
                        search_results.Last().TheProduct.are_images_loaded = (new bool[E.TheProduct.ListOfPathsOfThePhotos.Count]);
                    }
                }
            }

            // Einschub: Falls vor dem Bedienen der Suche verworfen wurden, müssen sie nun doch geladen werden. Das ist oben passiert. Hier wird das Boolean-Array befüllt.
            //If images were disposed before operating the search, they must now be loaded after all. This is what happened above. Here, the Boolean array is filled.

            foreach (Entry search_result in search_results)
            {
                for (int i = 0; i < search_result.TheProduct.are_images_loaded.Length; i++)
                {
                    if (!search_result.TheProduct.are_images_loaded[i])
                    {
                        search_result.TheProduct.are_images_loaded[i] = true;
                    }          
                }
            }

            switch (Sort_Criterion)
            {
                case Esort_criterion.alphabetical:
                    {
                        search_results.Sort((x, y) =>
                        {
                            int r = StringComparer.Ordinal.Compare(x.TheProduct.BrandName, y.TheProduct.BrandName);
                            return r;
                        });
                        if(CheckBox_reverse.Checked)
                        {
                            search_results.Reverse();
                        }
                        break;
                    }
                case Esort_criterion.by_Creation_Time:
                    {
                        search_results.Sort((x, y) => x.CreationTime.CompareTo(y.CreationTime));
                        if (CheckBox_reverse.Checked)
                        {
                            search_results.Reverse();
                        }
                        break;
                    }
                default:
                    {
                        break;
                    }
            }

            return search_results;
        }

So that you can reproduce this, I'm giving you the code of the classes here.

Entries.cs

using System;

namespace IA
{
    public sealed class Entry
    {
        public int Employee_who_created_Entry;
        public DateTime CreationTime;
        /// <summary>
        ///      in years
        ///      </summary>
        public double Age;
        public int Changed_by_Employee;
        public DateTime LastChangeTime;
        public Product TheProduct;

        public Entry(int Employee_who_created_Entry,
                     DateTime CreationTime,
                     int Changed_by_Employee,
                     DateTime LastChangeTime,
                     Product TheProduct)
        {
            this.Employee_who_created_Entry = Employee_who_created_Entry;
            this.CreationTime = CreationTime;
            this.Age = (DateTime.Now - this.CreationTime).TotalDays / 365.0;
            this.Changed_by_Employee = Changed_by_Employee;
            this.LastChangeTime = LastChangeTime;
            this.TheProduct = TheProduct;
        }
    }
}

Product.cs

using System;
using System.Collections.Generic;

namespace IA
{
    public sealed class Product
    {
        public string BrandName;
        public string ModelName;
        /// <summary>
        /// in millimetres
        /// </summary>
        public struct SSize
        {
            public UInt32 Length;
            public UInt32 Width;
            public UInt32 Height;
        }
        public SSize Size;

        public string DescriptionOrNotes;
        public List<System.Drawing.Bitmap> ListOfPhotos;
        public List<string> ListOfPathsOfThePhotos;
        public bool[] are_images_loaded;
        public decimal PurchasePrice;
        public decimal SellingPrice;

        public Product(string BrandName,
                       string ModelName,
                       SSize Size,
                       string DescriptionOrNotes,
                       List<System.Drawing.Bitmap> ListOfPhotos,
                       List<string> ListOfPathsOfThePhotos,
                       decimal PurchasePrice,
                       decimal SellingPrice)
        {
            this.BrandName = BrandName;
            this.ModelName = ModelName;
            this.Size = Size;
            this.DescriptionOrNotes = DescriptionOrNotes;
            this.ListOfPhotos = ListOfPhotos;
            this.ListOfPathsOfThePhotos = ListOfPathsOfThePhotos;
            this.PurchasePrice = PurchasePrice;
            this.SellingPrice = SellingPrice;
        }
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ please use English \$\endgroup\$
    – iSR5
    Feb 26, 2022 at 21:56

1 Answer 1

3
\$\begingroup\$

Quick remarks:

  • Please follow the guidelines. No underscores in names of classes etc., proper casing (pascal/camel),...

  • Don't put all your code in a single file. Most of your seek method should be in a separate class that returns a result of sort. Separate your UI from your back-end logic.

  • You have a seekAsync method that isn't async, and a seek method that is.

  • Don't pointlessly abbreviate. E is totally unclear.

  • Use descriptive name. StringArray1 and StringArray2 tell me nothing. StringArray2[0] tells me even less.

  • ListOfPathsOfThePhotos is a bad name in several ways. Why not simply call it PhotoPaths? TheProduct is also a bad property name: just call it Product.

  • Your code contains repeated blocks. Each time you call new Entry( I see the same parameters. Have a constructor that accepts an Entry plus any additional data (e.g. NewBitmaps) and do all the necessary actions in that constructor. Same with foreach (string FullFileName in E.TheProduct.ListOfPathsOfThePhotos).

  • search_results.Last().TheProduct.are_images_loaded = (new bool[E.TheProduct.ListOfPathsOfThePhotos.Count]); seems like a hacky solution. Why not return a class with one property that is the list of results, and another that is this Boolean?

  • Why don't you use LINQ instead of things like this: search_results.Sort((x, y) => x.CreationTime.CompareTo(y.CreationTime));? Arguably, even much of your search logic could be replaced by much simpler and easier to read LINQ commands that filter through AllEntries, you can then use the results of those commands to construct the list of search_results.

  • So much of your code looks "ancient". Did you base this on very old example code?

\$\endgroup\$
3
  • \$\begingroup\$ Thank you for your reply :) I'll take a look and improve it. I wanted to mention one more thing: This is just my source code. The reason I don't use LINQ is that I'm actually programming in VB.NET and am now gradually trying my hand at C#. I've already seen Visual Studio make suggestions with LINQ code. This is still too "confusing" for me. \$\endgroup\$
    – Daniel
    Feb 27, 2022 at 10:18
  • \$\begingroup\$ What else would you call "StringArray1"? Strings separated by the word or? \$\endgroup\$
    – Daniel
    Feb 27, 2022 at 10:20
  • \$\begingroup\$ I agree with you that the method is now so big that it should be in a separte class. 😅 \$\endgroup\$
    – Daniel
    Feb 27, 2022 at 10:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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