Codeoptimierung

Hallo

Meine erste gelungene Aufgabe ist geschafft - d.h. die Abfrage läuft und die Daten werden richtig in die dataGridView geschrieben.
Nun ist für mich die Frage, ob und wie ich meinen Code noch optimieren kann - so dass es einen "profesionelleren" Eindruck macht.
Wäre z.B. an manchen Stellen switch eine Verbesserung (wobei ich immer dachte, dass man dies nicht unbedingt verwenden soll )

Danke für Eure Tipps!

void DatenAnzeigen()
        {
            Kundenteile teil = new Kundenteile();

            foreach (var item in lstDistinct)
            {
                    string sCut = string.Empty;
                    string sA1 = string.Empty;
                    string sA2 = string.Empty;

                    teil = new Kundenteile();

                    // A1.
                    teil = KundenteileAuflistung(item.kurzzeichen, "A1");
                    if (teil != null)
                    {
                        sA1 = teil.kundenteilnummer;
                    }
                    else
                    {
                        sA1 = "-";
                    }

                    // A2.
                    teil = KundenteileAuflistung(item.kurzzeichen, "A2");
                    if (teil != null)
                    {
                        sA2 = teil.kundenteilnummer;
                    }
                    else
                    {
                        sA2 = "-";
                    }

                    string[] row = { item.kurzzeichen, sA1, sA2 };
                    dataGridView1.Rows.Add(row);
            }
        }

 public static Kundenteile KundenteileAuflistung(string sKurzzeichen, string sVorwahl)
        {
            try
            {
                using (dbEntities ctx = new dbEntities())
                {
                    Kundenteile teil = new Kundenteile();

                    if (sVorwahl == "A1")
                    {
                        var query = from p in ctx.kundenartikel
                                    where p.kurzzeichen.Contains(sKurzzeichen)
                                    where p.ausfuehrung.Contains("A1")
                                    select new Kundenteile
                                    {
                                        benennung = p.benennung,
                                        kundenteilnummer = p.kundenteilnummer
                                    };

                        if (query != null)
                        {
                            foreach (var item in query)
                            {
                                teil.benennung = item.benennung;
                                teil.kundenteilnummer = item.kundenteilnummer;
                            }
                            return teil;
                        }
                        else
                        {
                            return null;
                        }
                    }

                    if (sVorwahl == "A2")
                    {
                        var query = from p in ctx.kundenartikel
                                    where p.kurzzeichen.Contains(sKurzzeichen)
                                    where p.ausfuehrung.Contains("A2")
                                    select new Kundenteile
                                    {
                                        benennung = p.benennung,
                                        kundenteilnummer = p.kundenteilnummer
                                    };

                        if (query != null)
                        {
                            foreach (var item in query)
                            {
                                teil.benennung = item.benennung;
                                teil.kundenteilnummer = item.kundenteilnummer;
                            }
                            return teil;
                        }
                        else
                        {
                            return null;
                        }
                    }

                    else
                        return null;
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.InnerException.Message);
                return null;
            }
        }

Th69

Also, von professionell ist dein Code noch einiges entfernt...

Zum einen trennt man GUI und Daten, d.h. man verwendet DataBinding (anstatt, wie du, die Daten direkt in das DGV zu schreiben).
Und generell trennt man die Anwendung entsprechend der 3-Schichten-Architektur, s. z.B. Drei-Schichten-Architektur (d.h. GUI- und Datenbankzugriff stehen niemals in einer Klasse).

Aber selbst bei den Methoden kannst du noch einiges optimieren.
Frag dich mal, wieso du nach der Vorwahl ("A1"/"A2") unterscheidest (wenn der Query-Code der gleiche ist)?
Und der Code

foreach (var item in query)
{
    teil.benennung = item.benennung;
    teil.kundenteilnummer = item.kundenteilnummer;
}
return teil;

ergibt auch keinen Sinn (erst über die Schleife iterieren und dann nur die letzte Zuweisung nehmen). Dafür gibt es z.B. die LINQ-Erweiterungsmethoden First(), FirstOrDefault(), Last(), LastOrDefault() etc.

Und in Datenanzeigen() ist jeweils

new Kundenteile();

überflüssig, da die Objekte ja durch die KundenteileAuflistung-Methode erstellt werden.

Außerdem solltest du dich an den C#-CodingStyle halten und Eigenschaften mit einem Großbuchstaben anfangen lassen (und auf keinen Fall "public fields" verwenden!):
benennung -> Benennung
kundenteilnummer -> Kundenteilnummer

Und warum heißt die Datenklasse "Kundenteile" - also in der Mehrzahl, anstatt in der Einzahl "Kundenteil"?
Und der Name der Methode "KundenteileAuflistung" ist daher auch nicht richtig, denn du gibt ja nur ein Objekt zurück, keine Auflistung.

Th69 schrieb:

Also, von professionell ist dein Code noch einiges entfernt...

Das habe ich leider fast befürchtet

Th69 schrieb:

Zum einen trennt man GUI und Daten, d.h. man verwendet DataBinding (anstatt, wie du, die Daten direkt in das DGV zu schreiben).

Für die Abfragen der DB und den Code selbst habe ich bereits einzelne Dateien, hab dies nur hier der Einfachheit halber gemeinsam geschrieben.
Jedoch habe ich die Sache mit dem DataBinding nicht verstanden? Wie kommen die Daten dann einzelen in die DataGridView??

Th69 schrieb:

Aber selbst bei den Methoden kannst du noch einiges optimieren.
Frag dich mal, wieso du nach der Vorwahl ("A1"/"A2") unterscheidest (wenn der Query-Code der gleiche ist)?
Und der Code
foreach (var item in query)
{
    teil.benennung = item.benennung;
    teil.kundenteilnummer = item.kundenteilnummer;
}
return teil;
ergibt auch keinen Sinn (erst über die Schleife iterieren und dann nur die letzte Zuweisung nehmen). Dafür gibt es z.B. die LINQ-Erweiterungsmethoden First(), FirstOrDefault(), Last(), LastOrDefault() etc.

Nun habe ich die Abfrage folgerndermaßen angepasst

public static Kundenteil DatenKundenteil(string sKurzzeichen, string sVorwahl)
        {
            try
            {
                using (dbEntities ctx = new dbEntities())
                {
                    var query = (from p in ctx.kundenartikel
                                    where p.kurzzeichen.Contains(sKurzzeichen)
                                    where p.ausfuehrung.Contains(sVorwahl)
                                    select new Kundenteile
                                 {
                                     Benennung = p.benennung,
                                     Kundenteilnummer = p.kundenteilnummer
                                 }).FirstOrDefault();

                    return query;
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.InnerException.Message);
                return null;
            }
        }

Danke für den Hinweis mit der Bennung der Eigenschaften und Methoden!

Grundsätzlich habe ich noch das Problem, dass die ganze Abfrage seeeehr lange dauert! Kann dies an den kanpp 30.000 Datensätzen liegen oder doch wieder an meinem Code??

Als Geschwindigkeitsoptimierung habe ich bei der Abfrage noch folgendes versucht, ohne jedoch einen Fortschritt zu bemerken.

Anstatt

.Contains()

habe ich entsprechend

.StartsWith()

und

.Equals()

verwendet.

public static Kundenteil DatenKundenteil(string sKurzzeichen, string sVorwahl)
        {
            try
            {
                using (dbEntities ctx = new dbEntities())
                {
                    var query = (from p in ctx.kundenartikel
                                    where p.kurzzeichen.StartsWith(sKurzzeichen)
                                    where p.ausfuehrung.Equals(sVorwahl)
                                    select new Kundenteile
                                 {
                                     Benennung = p.benennung,
                                     Kundenteilnummer = p.kundenteilnummer
                                 }).FirstOrDefault();

                    return query;
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.InnerException.Message);
                return null;
            }
        }

Guten Abend
Zur Freund Google bin ich auf diesen Eintrag zum 3 Schichten Prinzip gestossen. Der Post ist allerdings schon älter.
Hier folgender Beitrag von Th69 dazu ...

Th69 schrieb:

Zum einen trennt man GUI und Daten, d.h. man verwendet DataBinding (anstatt, wie du, die Daten direkt in das DGV zu schreiben).
Und generell trennt man die Anwendung entsprechend der 3-Schichten-Architektur, s. z.B. Drei-Schichten-Architektur (d.h. GUI- und Datenbankzugriff stehen niemals in einer Klasse).

Meine Frage, woran erkenne ich in dem ersten Post, dass GUI- und Datenbankzugriff in einer Klasse stehen?

Th69

Ich ging davon aus, daß beide Methoden in einer Klasse sind.
Und in der ersten Methode wird direkt auf GUI-Elemente ("dataGridView1") und in der zweiten Methode auf die Datenbank zugegriffen ("dbEntities()" - wird wohl mittels des Entity Framework (EF) generiert sein).

Mechanics

giggle schrieb:

so dass es einen "profesionelleren" Eindruck macht.

Zusätzlich zu dem was Th69 geschrieben hat - für mich hört es eigentlich schon auf, wenn ich deutsche Bezeichner sehe. Ich kann es vielleicht grad noch nachvollziehen, wenn es um hochgradig domänenspezifischen Code geht und keiner die englischen Fachbegriffe ohne weiteres verstehen würde.
Aber wenn ich in 0815 Code sowas sehe

p.kurzzeichen.StartsWith(sKurzzeichen)

dann schaut es für mich von vornherein amateurhaft aus.

Th69 schrieb:

Ich ging davon aus, daß beide Methoden in einer Klasse sind.
Und in der ersten Methode wird direkt auf GUI-Elemente ("dataGridView1") und in der zweiten Methode auf die Datenbank zugegriffen ("dbEntities()" - wird wohl mittels des Entity Framework (EF) generiert sein).

Verstanden - danke für die schnelle Antwort!

Eine Frage dazu hätte ich nun aber nochmal.

Th69 schrieb:

...d.h. man verwendet DataBinding (anstatt, wie du, die Daten direkt in das DGV zu schreiben).

Diese Art habe ich ebenfalls einmal verwendet, weil ich Daten aus zwei DataTables zusammen in eine DataGridView schreiben wollte. Allerdings mit object[] anstatt string[], wobei dies wohl eher unerheblich ist. Was wäre in einem solchen Fall dann die saubere Lösung?

Habe hier selbst etwas zum Thema gefunden: http://stackoverflow.com/questions/25179745/creating-custom-columns-for-datagridview

asc

Mechanics schrieb:

giggle schrieb:

so dass es einen "profesionelleren" Eindruck macht.

Zusätzlich zu dem was Th69 geschrieben hat - für mich hört es eigentlich schon auf, wenn ich deutsche Bezeichner sehe.
...
dann schaut es für mich von vornherein amateurhaft aus.

Wenn in einer Firma die Vorgabe entsprechend ist, kannst du das gerne als amateurhaft bezeichnen, ist aber noch immer besser als schlechtes englisch das keiner versteht.