Problem mit Arrays

[Weberknecht]

Hallo,

hier mal mein Code:

int[,] arrGruppen = new int[20];

...
con.Open();
            MySqlCommand cmd = new MySqlCommand("SELECT b.kn, b.gr FROM gruppen b WHERE b.kn = '" + Program.user + "'", con);
            dr = cmd.ExecuteReader();

            int i = 0;
            while (dr.Read())
            {
                for (i = 0; i < 20; i++)
                {
                    arrGruppen[i] = Convert.ToInt16(dr.GetString(1));
                }
            }

            conn.Close();

            foreach (int z in arrGruppen)
            {

                switch (arrGruppen[z])
                {
                    case 1:
                        xyz.Visible = false;
                        break;
                    case 3:
                        MessageBox.Show("TEST");
                        break;
                    default:
                        break;
                }
            }

Hintergrund:
Ich lese Benutzer aus der Tabelle aus, die zu einer bestimmten Gruppe gehören.
Ein Benutzer kann auch Mitgleid in mehreren Gruppen sein.

Nun stelle ich mir das so vor, dass die jeweilige Gruppennummer (kn) eines Benutzers in das Array eingetragen wird und beim Switch ausgelesen wird.

Problem:
Hab ich einen Benutzer in mehreren Gruppen, steht in meinem Array immer (also eben die gesamte Arraygröße) die selbe Gruppennummer.

Was mache ich falsch bzw. wo liegt mein Fehler?
Würde mich sehr über Tipps und Hilfe freuen! DANKE!

[EXDW]

Hast du mal mit dem Debugger durch gesteppt was bei arrGruppen[i] = Convert.ToInt16(dr.GetString(1)); jeweils zugewiesen wird?

Unabhängig deines Problems solltest du den Code nochmal Refactoren, das sieht ja Gruselig aus, weiß man gar nicht wo man anfangen soll.

- Multidimensionalen Array der als einzelnes Array verwendet wird (Oder? Find das komisch)
- Warum überhaupt ein Array?
- Warum eine feste Größe von 20?
- Warum ist das Lesen der Gruppen in der selben Methode wie das switch case (Scheint zumindest so kopiert)
- Warum Iterierst du durch das array um danach per Index darauf zu zu greifen?
- Warum hast du UI Zugriffe direkt aus dem Code heraus?
...

bäh

[Weberknecht]

@David W
Danke erst mal für Deine Hinweise, leider vestehe ich nur die Hälfte ...

- Multidimensionalen Array der als einzelnes Array verwendet wird (Oder? Find das komisch)

Tippfehler von mir - soll nur ein eindimensionales Array sein

- Warum überhaupt ein Array?

Keine Ahnung, was wäre eine gute Alternative?

- Warum eine feste Größe von 20?

Da es maximal 20 Gruppen gibt

- Warum ist das Lesen der Gruppen in der selben Methode wie das switch case (Scheint zumindest so kopiert)

- Warum Iterierst du durch das array um danach per Index darauf zu zu greifen?

- Warum hast du UI Zugriffe direkt aus dem Code heraus?

Leider verstehe ich da nur Bahnhof ....

[schmidt-webdesign.net]

Weberknecht schrieb:

hier mal mein Code:

con.Open();
MySqlCommand cmd = new MySqlCommand("SELECT b.kn, b.gr FROM gruppen b WHERE b.kn = '" + Program.user + "'", con);
dr = cmd.ExecuteReader();
...
conn.Close();

Das Pärchen

con.Open();
conn.Close();

stimmt mich nachdenklich. Noch nachdenklicher (oder bedenklicher) macht allerdings die leichtsinnige Art, in der du das SqlCommand zusammenfrickelst.

~Erst wenn alle Programmierer eingesperrt und alle Ideen patentiert wurden, werdet ihr merken, dass Juristen nicht programmieren können ...~

[EXDW]

Hi,

Scheinbar bist du lernbereit - finde ich gut

Weberknecht schrieb:

- Multidimensionalen Array der als einzelnes Array verwendet wird (Oder? Find das komisch)

Tippfehler von mir - soll nur ein eindimensionales Array sein

Achso, das erklärt einiges.

Weberknecht schrieb:

- Warum überhaupt ein Array?

Keine Ahnung, was wäre eine gute Alternative?

List<int>();
Arrays sind generell recht unsicher.

Weberknecht schrieb:

- Warum eine feste Größe von 20?

Da es maximal 20 Gruppen gibt

Noch, das ist schlecht für die Maintainbillity wenn man solche "Magic Numbers" im Code hat. Überarbeite dein Code am besten so das du gar nicht mit einer Festen Anzahl arbeitest, das erhöt die Qualität, wiederverwenbarkeit, einfachheit, sicherheit und weiß der Geier ^^

Weberknecht schrieb:

- Warum ist das Lesen der Gruppen in der selben Methode wie das switch case (Scheint zumindest so kopiert)

- Warum Iterierst du durch das array um danach per Index darauf zu zu greifen?

- Warum hast du UI Zugriffe direkt aus dem Code heraus?

Leider verstehe ich da nur Bahnhof ....

[/quote]
Du wirkst verwirrt ^^ Lass es mich ausführen.

- Warum ist das Lesen der Gruppen in der selben Methode wie das switch case (Scheint zumindest so kopiert)
Dein Code den du hier Präsentierst sieht so aus als wäre alles in einer Methode. Es wäre aber besser wenn du mehrere Kleine Methoden hast die auch je nur eine Aufgabe erledigen. Ist um Welten besser für die Übersicht.

- Warum Iterierst du durch das array um danach per Index darauf zu zu greifen?

foreach (int z in arrGruppen)
{
    switch (arrGruppen[z])

Ehrlich gesagt versteh ich das gerade nicht ^^

angenommen du hast solch ein array { 1, 2, 3 }
dann

foreach (int z = arrGruppen)
{
    //z == 1
    switch (arrGruppen[z]) // zugriff auf 2

- Warum hast du UI Zugriffe direkt aus dem Code heraus?
xyz.Visible = false;
Du hast Zugriffe zu einer Datenbank & Zugriff auf die Oberfläche in einer Klasse und sogar in einer Methode.
Unschön, das sollte getrennt werden.

Was mir noch so auffällt - du hast alles auf Deutsch - unschön, genauso das du Worte unnötig abkürzt ^^

Wenn ich mal dein Code nehme und da etwas dran herum Doktor bekomme ich sowas raus:

private void DefineVisibilities(List<int> groups)
{
	// Blos kopiert, da ich es wie gesagt nicht verstehe was du da vor hast
	foreach (int z in arrGruppen)
	{
		switch (arrGruppen[z])
		{
			case 1:
				xyz.Visible = false;
				break;
			case 3:
				MessageBox.Show("TEST");
				break;
			default:
				break;
		}
	}
}

private List<int> GetGroups()
{
	var groups = GetGroups();

	// Die Connection kommt irgendwo her, das SQL statement sollte auch verschoben werden an einer prominenteren stelle
	connection.Open();
	var command = new MySqlCommand("SELECT b.kn, b.gr FROM gruppen b WHERE b.kn = '" + Program.user + "'", con);
	var reader = command.ExecuteReader();

	// Sollte man den reader evtl noch testen ob er gültig ist?
	// Auch die Azahl der Elemente (GetString) ist sehr unsicher da die Länge nicht geprüft wird
	while (reader.Read())
	{
	    var number = 0;
	    if (int.TryParse(dr.GetString(1), out number))
			groups.Add(number);
	}

	connection.Close();

	return groups;
}

Aufrufmethode:

var groups = GetGroups();
DefineVisibilities(groups);

Es ist weit davon entfernt wirklich sauber zu sein, aber wenigstens ein Anfang.

Ich sah nun auch dein Problem, bei jeder Gruppenzeile die du Liest, setzt du alle Werte im Array auf die neue Nummer, dadurch hast du bei jedem Durchlauf natürlich das gesamte Array auf einem Wert.

while (dr.Read())
{
	// der Wert im dr ist nun eine Zahl
	for (i = 0; i < 20; i++)
	{
		arrGruppen[i] = Convert.ToInt16(dr.GetString(1)); // Jetzt wird diese Zahl allen Elementen im Array zu gewiesen
	}
}

Hättest du mit dem Debugger in ein Paar Sekunden raus finden können

[Weberknecht]

@schmidt-webdesign.net
Danke für die Info mit der SQL-Injection.
Ich denke aber, dass es bei mir relativ "sicher ist (??) - es handelt sich um kene Webanwendung, außerdem nimmt kein Nutzer eine Eingabe vor - es wird der angemeldete Nutzer am PC verwendet. Mag nicht ganz korrekt sein, da ja ein Nutzer an den PC des Kollegen gehen kann, aber wir sind zum einen nicht so viele Mitarbeiter und zum anderen haben wir auch keine so "heiklen" Daten, die für manche komplett verboten wären.

Program.user = System.Security.Principal.WindowsIdentity.GetCurrent().Name;

Wegen con und conn => Tippfehler, weil ich manche Teile hier nicht mit Copy und Paste rein gemacht habe

[Gast]

David W schrieb:

List<int>();
Arrays sind generell recht unsicher.

Bei List fliegt eine ArgumentOutOfRangeException. Bei Arrays eine IndexOutOfRangeException. Das ist natürlich viel unsicherer

[EXDW]

µ schrieb:

David W schrieb:

List<int>();
Arrays sind generell recht unsicher.

Bei List fliegt eine ArgumentOutOfRangeException. Bei Arrays eine IndexOutOfRangeException. Das ist natürlich viel unsicherer

Sofern man mit einem [] direkt drauf zu greift. Das würde ich generell lassen.
Wenn ich wüsste was er mit seinen switch vor hast würde ich den direkten Zugriff dort auch komplett eliminieren (Du siehst an meinem Code Schnipsel das es sonst nirgends direkte Zugriffe gibt)

[Weberknecht]

Zu meiner Switch-Anweisung:

Wenn der Mitarbeiter in Gruppe 1 ist, soll das passieren
wenn er in Gruppe 2 ist soll was anderes passieren
und wenn er in beiden Gruppen ist, soll beides passieren.

Insgesamt habe ich so zwischen 6-10 Gruppen ...

Falls eine schöne Alternative gibt, hab ich nichts dagegen ...

[Weberknecht]

Ich habe das nun wie in Deinem Beispiel versucht zu lösen - leider erscheint immer die Fehlermeldung

StackOverflowException wurde nicht behandelt

bzw. dies taucht bei der Variablen cmd und groups auf

cmd Der Wert von "local" oder von Argument "cmd" ist in diesem Anweisungszeiger nicht verfügbar und kann daher nicht ermittelt werden. Möglicherweise wurde er bei der Optimierung entfernt. MySql.Data.MySqlClient.MySqlCommand

groups Der Wert von "local" oder von Argument "groups" ist in diesem Anweisungszeiger nicht verfügbar und kann daher nicht ermittelt werden. Möglicherweise wurde er bei der Optimierung entfernt. System.Collections.Generic.List<int>

private void DefineVisibilities(List<string> groups)
        {
            foreach (string group in groups)
            {
                switch (group)
                {
                    case "1":
                        verwaltungToolStripMenuItem.Visible = false;
                        menuStrip.Visible = false;
                        btnAdressverwaltung.Visible = false;
                        btnAuftragsverwaltung.Visible = false;
                        btnKontrakte.Visible = false;
                        break;
                    case "2":
                        MessageBox.Show("TEST");
                        break;
                    default:
                        break;
                }
            }
        }

        private List<int> GetGroups()
        {
            var groups = GetGroups();

            conn.Open();
            MySqlCommand cmd = new MySqlCommand("SELECT b.kennzeichen, b.gruppen_id FROM benutzergruppen b WHERE b.kennzeichen = '" + Program.user + "'", conn);
            dr = cmd.ExecuteReader();

            while (dr.Read())
            {
                var number = 0;
                if (int.TryParse(dr.GetString(1), out number))
                    groups.Add(number);
            }

            conn.Close();
            return groups;
        }

[Weberknecht]

Hab in dem Beispiel leider <string> und <int> gemischt ...
Das sollte natürlich wieder nicht sein - hab ich korrigiert, aber die Fehlermeldungen bleiben die selben.

[EXDW]

"cmd" gibts an der stelle noch nicht? Welche Zeile wird denn Moniert?

Wegen deinen Gruppen, es wäre besser die Benutzer direkt in "Group" Objekte zu packen, statt von oben nach unten die Gruppen zu merken.

public class Group
{
    public List<User> Users { get; set; }
}

Was ich aber nicht ganz verstehe, warum die Schleife, wenn am Ende jeder Benutzer ein einer anderen Gruppe ist, wurde jedes case einmal (mindestens) ausgeführt.
D.h. du brauchst gar nicht jeden Benutzer durchlaufen, sondern nur alle Gruppen wo es mindestens 1 Benutzer gibt Und da landen wir wieder bei dem eben genannten Objekt.

[EXDW]

Ich hab grad durch Zufall gemerkt das du nur einen User hast und eine Liste von Gruppen.
Ich hatte fälschlicherweise angenommen das du X User und pro User Y Gruppen hast.

Ich denk man könnte das mit Polymorphie besser lösen, indem du von jeder möglichen Gruppe ein spezielles Objekt hast (abgeleitet von einer Basisklasse) aber das muss in die Architektur passen.

Hier mal etwas Pseudocode wie ich das meine

public abstract class Group
{
    public abstract void HandleView(Form form);
}

public class FirstGroup : Group
{
    public override void HandleView(Form form)
    {
        verwaltungToolStripMenuItem.Visible = false;
        menuStrip.Visible = false;
        btnAdressverwaltung.Visible = false;
        btnAuftragsverwaltung.Visible = false;
        btnKontrakte.Visible = false;
    }
}

public class SecondGroup : Group
{
    public override void HandleView(Form form)
    {
        MessageBox.Show("TEST");
    }
}

public static class GroupFactory
{
    public static Group CreateGroup(int id)
    {
        switch (id)
        {
            case 1: return FirstGroup();
            case 2: return SecondGroup();
        }
        throw new InvalidArgumentException("Unknown group id");
    }
}

public partial class Demo : Form
{
    private List<Group> GetGroups()
    {
        var groups = GetGroups();

        conn.Open();
        var cmd = new MySqlCommand("SELECT b.kennzeichen, b.gruppen_id FROM benutzergruppen b WHERE b.kennzeichen = '" + Program.user + "'", conn);
        var dr = cmd.ExecuteReader();

        while (dr.Read())
        {
            var number = 0;
            if (int.TryParse(dr.GetString(1), out number))
                groups.Add(GroupFactory.CreateGroup(number));
        }

        conn.Close();
        return groups;
    }

    private void ExecuteGroups(List<Group> groups) // War mal "DefineVisibilities"
    {
        foreach (var group in groups)
            group.HandleView(this);
    }
}