Coding Styles Horror

Da möcht ich jetzt auch mal was zu beitragen. Folgendes ist ein Schnipsel aus einem Projekt das ich um ein paar Funktionen erweitern darf. ich bin grad dabei zu versuchen zu verstehen was mein Vorgänger wo und wie gemacht hat und versuch gleich mal zu benutzen was ich gestern dank gfoidl über CCD gelesen hab.

    
private void InitializePortData()
    {
      // Zähler
      int i;
      // Namen
      string[] PortNames = SerialPort.GetPortNames();
      // Index
      int CurIndex = -1;

      // Portnamen setzen
      for( i = 0; i < PortNames.Length; i++ )
      {
        // Index
        int Index = this.PortNameCmbBox.Items.Add(PortNames[i]);

        if (PortNames[i] == this.SrPort.PortName)
          CurIndex = Index;
      }

      if (this.PortNameCmbBox.Items.Count > 0 &&
        CurIndex >= 0 && CurIndex < this.PortNameCmbBox.Items.Count)
      {
        this.PortNameCmbBox.SelectedIndex = CurIndex;
      }
...
}

Ich kann endlich auch mal was zu dem Thread beitragen:

 public MyServerClass(Form window)
        {
            RichTextBox rtb = (RichTextBox) window.Controls["rtbMessages"];
            try
            {
                Uri uri1 = new Uri(window.Controls["txtAddressField1"].Text);
            }
[...]

Zum Hintergrund: Server-Klasse nimmt Config-Parameter von einer WinForms-Test Anwendung.
Das schöne ist nun, MyServerClass liegt in einer DLL und alle Entwickler welche diese benutzt haben, haben ohne murren ihre Parameter in eine Form gepackt und dann weitergereicht m(

Soeben hier im Forum:

if (e.KeyData == Keys.A)
    {
        sw.Write("A");
    }

    else if (e.KeyData == Keys.B)
    {
        sw.Write("B");
    }
    [....]
else if (e.KeyData == Keys.Z)
    {
        sw.Write("Z");
    }

Da hat sich aber einer eine Mühe gemacht 😁

Gruß,
Michael

Ebenfalls aus dem Forum:

public string SendCommand(string strCommand)
        {
            UdpClient client = new UdpClient();
            IPEndPoint ipe = new IPEndPoint(IPAddress.Parse(m_strGameServerIP), m_iGameServerPort);
            var result1 = string.Empty;
            var result2 = string.Empty;
            bool sent = false;
            bool DoubleTrame = false;
            Byte[] bufferTemp = Encoding.ASCII.GetBytes(m_strPassword + " " + strCommand);
            Byte[] bufferSend = new Byte[bufferTemp.Length + 5];
            Byte[] bufferRec;
            Byte[] bufferRec2;

            bufferSend[0] = Byte.Parse("255");
            bufferSend[1] = Byte.Parse("255");
            bufferSend[2] = Byte.Parse("255");
            bufferSend[3] = Byte.Parse("255");
            bufferSend[4] = Byte.Parse("00");

            for (int i = 0; i < bufferTemp.Length; i++)
            {
                bufferSend[i + 5] = bufferTemp[i];
            }

            while (!sent)
            {
                client.Send(bufferSend, bufferSend.Length, ipe);
                Thread.Sleep(200);
                if (client.Available > 0)
                {
                    sent = true;
                    if (client.Available > 1200)
                    {
                        DoubleTrame = true;
                    }
                }
            }
            bufferRec = client.Receive(ref ipe);
            if (DoubleTrame)
            {
                bufferRec2 = client.Receive(ref ipe);
                result2 = Encoding.ASCII.GetString(bufferRec2);
                if (result2.Contains("\n\n"))
                {
                    result2 = result2.Remove(result2.IndexOf("\n\n"));
                }
                result2 = result2.Substring(12);
            }
            result1 = Encoding.ASCII.GetString(bufferRec);
            string strResponse = result1 + result2;
            return strResponse;
        }

Mal abgesehen von der seltsamen Logik, finde ich die vor allem die Initialisierung des Arrays wunderschön

            
Byte[] bufferSend = new Byte[bufferTemp.Length + 5];
...
bufferSend[0] = Byte.Parse("255");
bufferSend[1] = Byte.Parse("255");
bufferSend[2] = Byte.Parse("255");
bufferSend[3] = Byte.Parse("255");
bufferSend[4] = Byte.Parse("00");

Timer in VB:

duration = Timer + siBreak ' Warten
    
Do Until Timer > duration
   DoEvents
Loop

Hallo,

Quizfrage: Was macht der Code?* 😁

                StreamGeometry geometry = new StreamGeometry();
                using (StreamGeometryContext sgc = geometry.Open())
                {
                    //Top->Bottom
                    sgc.BeginFigure(new Point((ellipseWheel.Width / 2), 0), false, false);
                    sgc.LineTo(new Point(ellipseWheel.Width / 2, ellipseWheel.Height), true, false);
                    //Left->Right
                    sgc.BeginFigure(new Point(0, (ellipseWheel.Height / 2)), false, false);
                    sgc.LineTo(new Point(ellipseWheel.Width, (ellipseWheel.Height / 2)), true, false);
                    //Left_Top_Center->Right_Bottom_Center
                    sgc.BeginFigure(new Point(ellipseWheel.Height / 2 + Math.Sqrt(((ellipseWheel.Height / 2) * (ellipseWheel.Height / 2) / 2)), ellipseWheel.Height / 2 + Math.Sqrt(((ellipseWheel.Height / 2) * (ellipseWheel.Height / 2) / 2))), false, false);
                    sgc.LineTo(new Point((ellipseWheel.Height / 2) - Math.Sqrt(((ellipseWheel.Height / 2) * (ellipseWheel.Height / 2) / 2)), (ellipseWheel.Height / 2) - Math.Sqrt(((ellipseWheel.Height / 2) * (ellipseWheel.Height / 2) / 2))), true, false);
                    //Left_Bottom_Center->Right_Top_Center
                    sgc.BeginFigure(new Point(ellipseWheel.Height / 2 - Math.Sqrt(((ellipseWheel.Height / 2) * (ellipseWheel.Height / 2) / 2)), ellipseWheel.Height / 2 + Math.Sqrt(((ellipseWheel.Height / 2) * (ellipseWheel.Height / 2) / 2))), false, false);
                    sgc.LineTo(new Point(ellipseWheel.Height / 2 + Math.Sqrt(((ellipseWheel.Height / 2) * (ellipseWheel.Height / 2) / 2)), (ellipseWheel.Height / 2) - Math.Sqrt(((ellipseWheel.Height / 2) * (ellipseWheel.Height / 2) / 2))), true, false);
                    //Left_Top_Bottom->Right_Bottom_Top
                    sgc.BeginFigure(new Point((ellipseWheel.Height / 2) - (Math.Cos(22.5 * Math.PI/180) * (ellipseWheel.Height / 2)), (ellipseWheel.Height / 2) - (Math.Sin(22.5 * Math.PI/180) * (ellipseWheel.Height / 2))), false, false);
                    sgc.LineTo(new Point((ellipseWheel.Height / 2) + (Math.Cos(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2)), (ellipseWheel.Height / 2) + (Math.Sin(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2))), true, false);
                    //Right_Top_Bottom->Left_Bottom_Top
                    sgc.BeginFigure(new Point((ellipseWheel.Height / 2) + (Math.Cos(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2)), (ellipseWheel.Height / 2) - (Math.Sin(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2))), false, false);
                    sgc.LineTo(new Point((ellipseWheel.Height / 2) - (Math.Cos(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2)), (ellipseWheel.Height / 2) + (Math.Sin(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2))), true, false);
                    //Left_Top_Top->Right_Bottom_Bottom
                    sgc.BeginFigure(new Point((ellipseWheel.Height / 2) - (Math.Sin(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2)), (ellipseWheel.Height / 2) - (Math.Cos(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2))), false, false);
                    sgc.LineTo(new Point((ellipseWheel.Height / 2) + (Math.Sin(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2)), (ellipseWheel.Height / 2) + (Math.Cos(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2))), true, false);
                    //Right_Top_Top->Left_Bottom_Bottom
                    sgc.BeginFigure(new Point((ellipseWheel.Height / 2) + (Math.Sin(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2)), (ellipseWheel.Height / 2) - (Math.Cos(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2))), false, false);
                    sgc.LineTo(new Point((ellipseWheel.Height / 2) - (Math.Sin(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2)), (ellipseWheel.Height / 2) + (Math.Cos(22.5 * Math.PI / 180) * (ellipseWheel.Height / 2))), true, false);
                }

mfG Gü

* ist natürlich nur eine rhetorische Frage da ich sonst gegen [Hinweis] Wie poste ich richtig? 4.c verstoßen würde 😉

Den hab ich selbst hingekriegt und zuerst nicht mal gemerkt. X( 8o

           
for (int i = 1; i < this.Fitness.Length;i++)
{
      double cost = double.MaxValue;
      if (Fitness[i] < cost)
      {
           cost = Fitness[i];              
      }
}

cost gehört natürlich nicht in die schleife...

int monthOffset = DateTime.Now.Month % 3;
DateTime vonDatum = new DateTime(DateTime.Now.Year, DateTime.Now.Month - monthOffset - 2, 1);
DateTime bisDatum = vonDatum.AddMonths(2);
bisDatum = bisDatum.AddDays(DateTime.DaysInMonth(bisDatum.Year, bisDatum.Month) - 1);

Auch wenn mans nicht glauben mag, aber dieser Code sollte den Quartalsbeginn und das dazugehörige Quartalsende ermitteln. Bis Dezember hat niemand das Ergebnis kontrolliert, heute hab ich zum ersten mal in diesem Jahr diesen Code aufgerufen ...

Hallo Leute,

heute musste ich mich mal wieder demütigen und habe mich UNI-Projekt-bedingt in Java vertieft.

Was ich da als einen FAIL sehe sehen Java-Entwickler wahrscheinlich als normal. Mann beachte:

String s = null;
s+="hallo";
System.out.println(s);

Nun ratet mal was der Code druckt?
Er druckt tatsächlich "nullhallo".
Nun dachte ich mir: Na gut vergleiche ich mal Strings:

"nullhallo".equals(s)

gibt false. Nun nach Inspektion aller chars bin ich draufgekommen dass der String mit dem null am Ende noch 4 leere chars hat. Ich kann dazu nur eins sagen WTF, und Gott sei dank ist Samstag Abend und man kann jetzt sofort abtanzen gehen.

Gruß
Michael

Ich bin schon länger mit C# vertraut, aber das mir das nochmal passiert. 😃

        public override int GetHashCode()
        {
            return 0;
        }

Das Hinzufügen mit Abfrage auf ContainsKey für über 30k Dictionary-Elemente hat mich Minuten gekostet. 😄

Gerade bin ich wieder auf ein kleines Schmankerl gestoßen:

            if (this.cbxSuggestionList.SelectedIndex != -1)
            {
                this.cbxSuggestionList.SelectedIndex = -1;
            }

            if (this.cbxSuggestionList.SelectedIndex == -1 || this.cbxSuggestionList.DroppedDown)
            ...

public new void Foo()
{
  BaseClass.Foo();
}

Das klingt jetzt komisch aber es macht sinn, weil es ein interface für die Exception gibt, welches COM visible ist und welches die benannte Methode vorschreibt. daher muss die Klasse Exception diese Methode implementieren, damit nicht .NET COM Bibliotheken an den Type kommen können.

ich schäme mich dafür, dass mir das passiert ist, aber vorenthalten möcht ichs euch nicht ^^

myString = myString.Trim(" ".ToCharArray());

Hallo zusammen,

hier ein Horror von James Michael Hare, gerade auf Twitter gesehen:

Saw most awesome useless piece of legacy code at work in loop: if (strWebId == strSearchId) { bool bFound = true; bFound = false; } #WTF???

m0rius

public ImageList SmallImageList { get; set; }
 
Declaring Type: System.Windows.Forms.ListView 
Assembly: System.Windows.Forms, Version=4.0.0.0 

-->

...
                if (this.View == View.SmallIcon)
                {
                    this.View = View.LargeIcon;
                    this.View = View.SmallIcon;
                }
...

Heute gesehen in einem Projekt eines Kommilitonen....
Erstaunlicherweise funktioniert es, ich frage mich nur ernsthaft wie man sowas hinbekommt, da kriegt man doch einen Knoten im Kopf. Ich habs nun 10min angeschaut und könnte immernoch nicht sagen wie der Ausgangsstring aussehen müsste =)

if(line.Contains("label:"))
{
    line = (line.Substring(line.IndexOf("targetname:"),
                line.IndexOf("label:") - line.IndexOf("targetname:")))
                .Substring((line.Substring(line.IndexOf("targetname:"),
                line.IndexOf("label:") - line.IndexOf("targetname:")))
                .IndexOf("\"") + 1,
                (line.Substring(line.IndexOf("targetname:"),
                line.IndexOf("label:") - line.IndexOf("targetname:")))
                .LastIndexOf("\"") -
                (line.Substring(line.IndexOf("targetname:"),
                line.IndexOf("label:") - line.IndexOf("targetname:")))
                .IndexOf("\"") - 1);

    int target = Int32.Parse(line);
    targets.Add(target);
}

Gestern in einem unserer Projekte entdeckt: Der sog. throw-catch Block!

public void DoSomething() {
    try {
        throw new NotSupportedException("Methode wird nicht unterstützt.");
    
        // Etliche
        // Zeilen
        // auskommentierter
        // Sourcecode

    } catch {
    }
}

Dieses kleine Schmankerl habe ich gerade in einem unserer DB Scripte gefunden.

Das ist im übrigen das ganze script und es mach nur das und nicht mehr, aber auch nicht weniger 😁

UPDATE Tabelle1 SET xxx = NULL WHERE xxx IS NULL

Hab soeben etwas idiotisches in meinem eigenen Code entdeckt 😄

if (instance.Prop1 != null && instance != null)

Status: Fixed 😉

Sinn macht das nicht. Es erfüllt vielleicht die Funktion, die man haben will aber gut ist so ein Vorgehen dennoch nicht. Besser wäre eine Methode ala "Render" gewesen aber über das wiederholte setzen eines Properties eine Funktion zu erreichen ist Bad Style.

Ich habe mal irgend wo das gesehen:

public class IrgendeineKlasse
{
  public static readonly bool Value = true;
  public static bool WertvonValue(bool checkInput)
  {
  hier:
    if((checkInput == new Boolean()) != false)
    {
      switch(this.Value.ToString())
      {
        case Boolean.TrueString: return Boolean.Parse(Boolean.TrueString);
        case Boolean.FalseString: return Boolean.Parse(Boolean.FalseString);
        default: 
        if(WertvonValue(Boolean.Parse(Boolean.TrueString)) == Boolean.Parse(Boolean.TrueString))
        {
          return true;
        }
        else
        {
          return false;
        }
      }
    }
    else
    {
      try
      {
         throw new Exception("Bool-Fehler");
      }
      catch(Exception ex)
      {
         goto hier;
      }
  }
}

HILFE!!!
Gruß,
C#ler

Das hat garantiert jemand mit Absicht gemacht. Unerfahrene kommen z.B. in der Regel nicht auf die Idee, statics als readonly zu deklarieren.

leider kein Witz:

public static class AttributeNames
{
// gekürzt, ca 30 string Konstante, zb:

        /// <summary>
        /// Currency
        /// </summary>
        public const string Currency = "Currency";
     
// und dann:

        /// <summary>
        /// M
        /// </summary>
        public const string M = "M";

        // was ist das? Boss von James Bond?
}

Hallo xxMUROxx, danke für die Erklärung. Jetzt kann ich die Problematik nachvollziehen. Mein Problem mit dem Beispiel war, dass ich das Schlüsselwort "new" überlesen/ignoriert habe (war auch schon spät). Jetzt strahlt der Horror in einem ganz anderen Licht. Jack

Hab auch wiedermal was tolles beim durchforsten von Code in der Firma gefunden:

public new void Hide()
{
     base.Hide();
}

private bool IsMASelectionAllowed()
{
    bool isallowed = false;

    if (IsFunktion((int)MitarbeiterFunktionArt.hauptberuflicheSachwalterIn)
    || IsFunktion((int)MitarbeiterFunktionArt.ClearingSachwalterIn)
    || IsFunktion((int)MitarbeiterFunktionArt.ehrenamtlicheSachwalterIn))
    {
        isallowed = true;
    }
    else
    {
        isallowed = false;
    }

    return isallowed;
}

ganz toll auch die Funktion dafür, Implementierung und Namensgebung ...

private bool IsFunktion(int funktion)
{
    if ((int)CurrentKidsLogin.Current.CurrentFunktion == funktion)
    {
        return true;
    }
    else
    {
        return false;
    }
}

So, von mir auch mal ein schönes Beispiel aus einem Projekt an der Uni.
Musste echt erst überlegen was das da macht und danach bin ich aus dem lachen nicht mehr raus gekommen.

private void addTowerLevels()
{
	for (int i = 1; i < 5; i++)
	{
		switch (i)
		{
			case 1:
				towerLvlTextures[i] = contentManager.Load<Texture2D>("Gui/towerlvl/1");
				break;
			case 2:
				towerLvlTextures[i] = contentManager.Load<Texture2D>("Gui/towerlvl/2");
				break;
			case 3:
				towerLvlTextures[i] = contentManager.Load<Texture2D>("Gui/towerlvl/3");
				break;
			case 4:
				towerLvlTextures[i] = contentManager.Load<Texture2D>("Gui/towerlvl/4");
				break;
		}
	}
}

Gruß,
Kami

Soeben in einer Abfrage gefunden, is zwar kein Horror aber trotzdem wollte ich es hier posten:

SELECT * FROM from WHERE true = false AND false = true

Wusste garnicht dass dies so funktioniert.lg.

Ein Kollege hat mich neulich gebeten, ihm bei einem Code zu helfen, den er von einem ausgeschiedenen Mitarbeiter übernommen hat.

Verteilt über eine etwas längliche Klasse mit wenigen hundert Zeilen (Zur anonymisierung des Projekts Namen etwas geändert, waren vorher aber auch nicht sprechender...):

public class SolverService
{
    private static readonly Random random = new Random(DateTime.Now.Millisecond);        
    delegate Item ApplyDelegate(List<Item> candidates);
    private const string ChoiceRandomName = "Random";
    private const string ChoiceFrequencyName = "Frequency";
    Dictionary<string, ApplyDelegate> choicesDelegates;
    
    public SolverService()
    {
        choicesDelegates = new Dictionary<string, NextCandidate>();
        MethodInfo mi = typeof(SolverService).GetMethod("ChoiceRandom", BindingFlags.NonPublic | BindingFlags.Static);
        ApplyDelegate applyDelegate = (ApplyDelegate)Delegate.CreateDelegate(typeof(ApplyDelegate), mi);
        choicesDelegates.Add(ChoiceRandomName, applyDelegate);

        mi = typeof(SolverService).GetMethod("ChoiceFrequency", BindingFlags.NonPublic | BindingFlags.Static);
        applyDelegate = (ApplyDelegate)Delegate.CreateDelegate(typeof(ApplyDelegate), mi);
        choicesDelegates.Add(ChoiceFrequencyName, applyDelegate);
    }

    public Item Perform(List<Item> candidates)
    {
        int next = random.Next(0, choicesDelegates.Count);
        string currentChoice = (next == 0) ? ChoiceRandomName : ChoiceFrequencyName;
        return choicesDelegates[currentChoice].Invoke(candidates);
    }

    private static Item ChoiceRandom(List<Item> candidates) { /*....*/ }
    private static Item ChoiceFrequency(List<Item> candidates) { /*....*/ }
}

Wir haben den Teil dann reduziert bekommen zu

public class SolverService {
    private static readonly Random random = new Random();

    public Item Perform(List<Item> candidates) {
        if (random.NextDouble() < 0.5) {
            return ChoiceRandom(candidates);
        } else {
            return ChoiceFrequency(candidates);
        }
    }

    private static Item ChoiceRandom(List<Item> candidates) { /*....*/ }
    private static Item ChoiceFrequency(List<Item> candidates) { /*....*/ }
}

Davon, dass nun eine IList im öffentlichen Interface schöner wäre, will ich garnicht erst anfangen...

Sowas gibt's ja öfters, wie mir scheint.
Jedoch war ich trotzdem fast am verzweifeln als ich vorhin folgendes gefunden habe:

        private bool IsNull(int? i)
        {
            return i == null ? true : false;
        }

        private bool IsNull(decimal? d)
        {
            return d == null ? true : false;
        }

@Zu den Hinweisen unten:

Genau, ich würde das mit HasValue oder direkt mit == null verwenden. Auch wenn der Aufruf mit int machbar wäre, bringt dies ja nichts. int kann im Gegensatz zu int? sowieso nicht null sein. 😉

Ist zwar VB, aber ich hoffe ihr versteht das geniale Prinzip dennoch :

Dim Anzahl As Integer
 
Private Sub Command1_Click()
    Anzahl = 0;
    For i = 1 To objMapi.GetDefaultFolder(olFolderInbox).UnReadItemCount
        Anzahl = Anzahl + 1
    Next
    MsgBox Anzahl & " ungelesene Nachricht(en) !", vbInformation, "Outlook" & _
      "Postfach"

😉

Grüße Bernd

if (true || connection_id == null)

O_o

Hallo,

Thread t = new Thread(monitor);
t.Start();
t.Join();

Quelle: hier im Forum gesehen 😉

mfG Gü

PS: Erklärung und Abhilfe findet sich in [FAQ] Warum blockiert mein GUI?.

Gerade eben über dieses geniale Stück Code gestolpert:

            QObject finanzierung = context.QKernelClient.GetObject(context.QPointer);
            if (finanzierung != null)
            {
            }

            Refresh(context);

Hallo,

hab gerade eine (SL-) Anwendung betrachtet die fremdvergeben wurde. Ich finds lustig und irgendwie genugtuend denn diejenigen die das gemacht haben waren vor dem Auftrag so gescheit - im Sinne von: wir wissen alles und können alles und das was wir nicht wissen schlagen wir kurz bei Wikipedia nach und dann wissen wir das auch (v.a. als es um sehr fachspezifisches ging) - dass es für mich schon kaum auszuhalten war. Warum die den Auftrag trotzdem bekommen haben? Nicht alle Entscheidungen treffe ich...tw. leider....

Die ganze Eingabe-Validierung findet mit Methoden in den "ViewModel"-Klassen statt, die nach der Art

public bool IsValidDoubleInput(RadComboBox cb)
{
	if (cb.SelectedValue == null)
	{
		try
		{
			if (cb.Text == null)
			{
				throw new Exception();
			}
			cb.Text = cb.Text.Replace(",", m_cDecimalSeperator);
			cb.Text = cb.Text.Replace(".", m_cDecimalSeperator);
			Convert.ToDouble(cb.Text);
			cb.BorderBrush = this.ValidBrush;
			return true;
		}
		catch (Exception)
		{
			cb.BorderBrush = this.InValidBrush;
			return false;
		}
	}
	cb.BorderBrush = this.ValidBrush;
	return true;
}

aufgebaut sind. Falls irgendwo eine Validierung fehlschlägt wird zusätzlich zum roten Rahmen (die BorderBrush im Code) um das Control an immer gleicher Stelle nur "Fehlerhafte Eingabe" angezeigt. Sehr hilfreich...
Von Validierungen bezüglich nicht zulässigen negativen Werten ganz zu schweigen und auch dass null-Eingabe als valide betrachtet wird...

Hinzu kommt sonst auch noch Geldverschwendung denn die zusätzliche Funktionalität von den Telerik-Radcontrols wird gar nicht genutzt...

Womöglich muss ich einen Wiki-Artikel über double.TryParse und deren Überladungen schreiben 😁

mfG Gü

Ist es denn "ok" oder guter Stil ein Steuerelement als Parameter an eine Methode zu übergeben?