Laden...

Coding Styles Horror

Erstellt von Khalid vor 15 Jahren Letzter Beitrag vor 3 Jahren 364.577 Views
4.221 Beiträge seit 2005
vor 15 Jahren

Hier auch noch einer (allerdings in VB) XXX anonymisiert.


Dim value As String = Me.XXX.Text
If value.Length > 0 Then
   Me.XXX.Text = value
End If

PS: Das XXX-TextBox ist weder gebunden... noch mit irgendwelchen Events versehen.

Gruss
Programmierhans

Früher war ich unentschlossen, heute bin ich mir da nicht mehr so sicher...

1.200 Beiträge seit 2007
vor 15 Jahren

Ich könnte einen ganzen Client hier posten....werde es aber nicht tun. Und ihr alle würdet beim Anblick des Codes sofort erblinden.

Ich nicht, ich bin stark. Ich hab eine rosarote Spezialbrille die mir durch den Tag hilft.

Ausschnitt:


public void ExportData1()
{
     object oExcelName;
     object oFileName;
     object[] oHeaders;
     object[] oDataTypes;
     object[] oAgg;
     object[] oData;
     object specColToReplace;

     oExcelName = "blablub";
     oFileName = "blabla";
     GetConfig(ref oHeaders, ref oDataTapyes, ref oAgg, ref specColToReplace);
     data = GetData(ref oHeaders, ref oDataTypes, reg oAgg, ref specColToReplace);
     ExcelSheet sh = new ExcelSheet(oHeaders, oDataTypes, oAgg, specColToReplace, data);
     ArrayList arlExcSheets = new ArrayList();
     arlExcSheets.Add(sh);
     Excel.Exp2ExcelMap(arlExcSheets, oFileName);
}

Kein Witz. Von dieser Sorte Methode gibt es ca 200 im ganzen Code. Und innerhalb der GetConfig Methoden etc. werden immer mehr Argumente angehäuft, die dann immer weiter per ref übergeben werden, immer tiefer in den CallStack hinein.

Try Catch Blöcke werden im Code so benutzt:


try
{
    //stuff
}
catch(Exception ex)
{
    throw ex;  //nichts weiter
}

Und den größten Horror, das "XML" Config File, hab ich euch noch erspart.

Shift to the left, shift to the right!
Pop up, push down, byte, byte, byte!

YARRRRRR!

691 Beiträge seit 2007
vor 15 Jahren

Und den größten Horror, das "XML" Config File, hab ich euch noch erspart.

Solange es valide ist, kann doch nix schlimm dran sein? Oder etwa doch? ^^

mit freundlichen Grüßen,
Tomot

Projekte: www.gesellschaftsspieler-gesucht.de

1.200 Beiträge seit 2007
vor 15 Jahren

Du hast keine Ahnung, was für Perversionen man mit "XML" erstellen kann. Das Config File ist eine Mark-Up Vergewaltigung, die ihres gleichen sucht. Werds demnächst mal posten, hab z.Z. Urlaub. Dann weisst du, was ich meine.

Shift to the left, shift to the right!
Pop up, push down, byte, byte, byte!

YARRRRRR!

1.200 Beiträge seit 2007
vor 15 Jahren

Naja, so mal ausm Kopf raus (auszugsweise):

<add key="ExcelSheet1HeadCol1" value="SomeHeader" />
<add key="ExcelSheet1HeadCol2" value="AnotherHeader" />

<!-- snip -->

<add key="ExcelSheet1HeadCol45" value="YetAnotherHeader" />

<add key="ExcelSheet1ColorCol1" value="45" />
<add key="ExcelSheet1ColorCol2" value="45" />

<!-- snip -->

<add key="ExcelSheet1DataTypeCol1" value="V200" /> <!-- V200 ist son Marke Eigenbau String typ - wird für die Formatierung benötigt -->
<add key="ExcelSheet1DataTypeCol2" value="N" />

<!-- snip -->

<add key="ExcelSheet1ForeignKey1" value="BLABLADONTKNOWSHIT" />

Ja, und davon halt 3000 Zeilen.

Shift to the left, shift to the right!
Pop up, push down, byte, byte, byte!

YARRRRRR!

390 Beiträge seit 2008
vor 15 Jahren

Was ich gerade gefunden habe:

War ein C-Programm für Mikrocontroller



#define ever ;;

int main(){

for(ever){
//Anweisungen
}

}


using Skill

1.130 Beiträge seit 2007
vor 15 Jahren

auch ned schlecht:


void* buffer=...
...
void* indexPos=(Byte*)buffer+*((int*)(((Byte*)buffer)+8));
...
void* filePos=((Byte*)buffer)+*((int*)(((Byte*)indexPos)+16+(i*4)));

Projekte:Jade, HttpSaver
Zum Rechtschreiben gibts doch schon die Politiker. Aber die bauen auch nur mist!

5.742 Beiträge seit 2007
vor 15 Jahren

Wobei die "forever" Schleife IMHO etwas Tiefgründiges hat. 🙂
Irgendwie eine nette Idee.

49.485 Beiträge seit 2005
vor 15 Jahren

Hallo edsplash,

for(ever) ist doch gerade guter Stil, weil es kryptische Zeichen durch einen sprechenden Namen ersetzt.

herbivore

390 Beiträge seit 2008
vor 15 Jahren

vielleicht ist es weniger horror und mehr lustig 😉 Ich musste auf jedenfall spontan lachen als ich es gesehen habe. Allerdings lösen defines bei mir immernoch Gänsehaut aus. Vorallem wenn ich solche Konstrukte angetroffen habe:


#define irgendeinefunktion(a,b,c,d)                    a<<24 | b<<16 | c<<8 | d

Das war z.T. sehr mühsam, wo es doch viel einfacher zu debuggen wäre, wenn man dafür eine echte Funktion schreibt.

Gruss

using Skill

F
10.010 Beiträge seit 2004
vor 15 Jahren

Klar, aber die "alten" C/C++ Compiler haben nicht wirklich optimiert, und haben
soetwas dann mit den entsprechenden Übergaben auf dem Stack usw erzeugt.

Und gerade wenn es um Geschwindigkeit ging war das die einzige Möglichkeit
das letzte bischen herauszuholen.

Und da es leider immernoch die gleichen sind, die heute solchen Code schreiben
oder lehren, wird das auch immernoch so eingesetzt.

Und im Embedded Bereich wird es auch noch ein bischen so bleiben.

467 Beiträge seit 2007
vor 15 Jahren

was haltet ihr davon: (natürlich stark vereinfacht, war eine halbe Seite mit tatsächlich beabsichtigter Aufgabe)


public void Action(cracyStuff[] CS)
{
while(true)
{
//DoCrazyStuff
Action(CS2);
if(blabla) break;
}
}

mein Freund wolle wissen, warum das nicht funktioniert. Er hat angemerkt, dass, obwohl man ja keine Endlosschleifen verwenden soll, diese hier gerechfertigt ist, da sie eine Abbruchsbedingung hat.

1.346 Beiträge seit 2008
vor 15 Jahren

8o 8o 8o
Boah! Das nenn ich mal Rekursion

pdelvo

49.485 Beiträge seit 2005
vor 15 Jahren

Hallo ANSI_code,

da dies kein Thread für Fragen ist, nur in alle Kürze:

mein Freund wolle wissen, warum das nicht funktioniert.

Die Rekursion erfolgt, bevor die Abbruchbedingung erreicht wird. Also bekommt er eine Endlosrekursion.

Er hat angemerkt, dass, obwohl man ja keine Endlosschleifen verwenden soll, diese hier gerechfertigt ist, da sie eine Abbruchsbedingung hat.

Das kann man in der Tat so machen und ist kein schlechter Stil. Nennt sich Middle-Check(ed)-Loop.

herbivore

4.506 Beiträge seit 2004
vor 15 Jahren

Hallo herbivore,

bist Du sicher, dass das so akzeptabel ist?

Ich bin zwar grundsätzlich gegen "Middle-Check(ed)-Loops", aber das was ANSI_code hier gepostet hat ist doch das Prüfen der Abbruchbedingung NACH einem Rekursionsaufruf.

Das ist der Grund warum zumindest ich einen Horror bekomme, wenn ich "Middle-Check(ed)-Loops" sehe 😉

Grüße
Norman-Timo

A: “Wie ist denn das Wetter bei euch?”
B: “Caps Lock.”
A: “Hä?”
B: “Na ja, Shift ohne Ende!”

49.485 Beiträge seit 2005
vor 15 Jahren

Hallo norman_timo,

bist Du sicher, dass das so akzeptabel ist?

warum die konkrete Schleife nicht funktioniert, hatte ich ja schon oben geschrieben. Aber im Allgemeinen sind Middle-Check(ed)-Loops vollkommen ok. Und ja, ich bin mir da sicher. Für mich wäre ... um mal auf das Thema des Threads zurückzukommen ... Coding Style Horror, wenn man statt

while (true) {
   ...
   if (bed) break;
   ...
}

schreiben würde


bool stop;
do {
   ...
   stop = bed;
   if (!stop) {
      ...
   }
} while (!stop);


nur um eine Middle-Check(ed)-Loop zu vermeiden.

herbivore

467 Beiträge seit 2007
vor 15 Jahren

@herbivore dir ist schon klar, dass ich natürlich wusste, warum es nicht funktioniert, oder? Sonst hätte ich das in einem anderen Forum gepostet (welch eine Vorstellung, wenn ihr nur diesen code gesehen hättet!!! Leider hab ich die e-mail jetzt schon gelöscht. Hab´s ihm auch erklärt, was falsch war. Fand er dann auch lustig.

edit: übrigens verwende ich persönlich sehr gerne Middle-Checked-Loops.

Gelöschter Account
vor 15 Jahren
					int maximumTrys = 5;
					int currentTry = 0;
					while (true)
					{
						try
						{
							File.Delete(path);
							break;
						}
						catch
						{
							if (currentTry >= maximumTrys )
							{
								throw;
							}
							Thread.Sleep(100);
							currentTry ++;
						}
					}

das wurde mir als bugfix verkauft....

Gelöschter Account
vor 15 Jahren

noch was schönes...

				if (String.Compare(irgendeinstring, nocheinstring) == 0)
					return true;

........
			return false;


1.130 Beiträge seit 2007
vor 15 Jahren

zu JAck30lena´s letztem Post: Dadurch, dass zwischen return false und return true ja noch was steht, gehts ja noch. Zum .Compare: derjenige hat wohl zuviel c++ geprogt.

Projekte:Jade, HttpSaver
Zum Rechtschreiben gibts doch schon die Politiker. Aber die bauen auch nur mist!

Gelöschter Account
vor 15 Jahren

ich zielte je eher auf das compare 😃

du hast natürlich recht. die returns sind so korrekt.

Gelöschter Account
vor 15 Jahren

object whatIwantToHave;
foreach(object o in somecollection)
{
whatIwantToHave = o;
break;
}

OOOOOHHHHH MEIN GOTTT.......

458 Beiträge seit 2007
vor 15 Jahren

Schwester, den Defibrilator!

be the hammer, not the nail!

3.003 Beiträge seit 2006
vor 15 Jahren

Vor einigen Wochen im Code Repository entdeckt (gekürzte Fassung):



String[] CopyStringArray(String[] source)
{
   String[] result = new String[source.Length];
   for(int i = 0; i < source.Length; i++)
   {
        switch(i)
        {
               case 0: result[0] = source[i]; break;
               case 1: result[1] = source[i]; break;
               /* you get the idea */
               case 30: result[30] = source[i]; break;
               default: break;
        }
   }
   return result;
}

Ja, es ging nur bis 30.
Ich hab's nicht übers Herz gebracht, das zu löschen. Nur verbessert und auskommentiert. Und bedauert, dass der Kollege seine Probezeit nicht überstanden hat. Wenigstens bisschen Spass an der Arbeit...

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

M
198 Beiträge seit 2007
vor 15 Jahren

Dieses Programm ist mittlerweile in der dritten Generation und ein Typischer Fall von FAD und zwar alle Punkte ausser Learn To Deal.

Kleine Auszüge (VB6, ' = Kommentareinleitung):


'End If' ? Warum war es hier? Geht auch ohne


If a= b Or a = c Or b= c Then   
Label37.Visible = False
Label38.Visible = False
Label39.Visible = False
Label41.Visible = False
Label45.Visible = True
End If


Private Sub Timer4_Timer()
If Form1.Label17.Caption <> Date Then
If Hour(Time) = 0 Then
If datum3<> Date Then
Form1.Label17 = Date
datum3= Date
End If
End If
End If
End Sub


        'Glaube das ist für die Buttonaktivierung
        'If b_Dummy3= True Then
            If Text2.Text <> "" And Text3.Text <> "" And Text4.Text <> "" And Text5.Text <> "" And Text7.Text <> "" And Text9.Text <> "" Then
                Command1.Enabled = True
            Else
                Command1.Enabled = False
            End If
        'End If

Khalid Themenstarter:in
3.511 Beiträge seit 2005
vor 15 Jahren

'Glaube das ist für die Buttonaktivierung

Das steht da nicht wirklich, oder? 8o

"Jedes Ding hat drei Seiten, eine positive, eine negative und eine komische." (Karl Valentin)

1.985 Beiträge seit 2004
vor 15 Jahren

Hallo zusammen,

'Glaube das ist für die Buttonaktivierung
Das steht da nicht wirklich, oder? 8o

genau das habe ich mich auch gerade gefragt 🙂. Aber sowas in die Richtung kenne ich auch, wäre also wirklich möglich 😦.

Gruß,
Fabian

"Eine wirklich gute Idee erkennt man daran, dass ihre Verwirklichung von vornherein ausgeschlossen erscheint." (Albert Einstein)

Gefangen im magischen Viereck zwischen studieren, schreiben, lehren und Ideen umsetzen…

Blog: www.fabiandeitelhoff.de

M
198 Beiträge seit 2007
vor 15 Jahren

Leider steht es wirklich da ^^

Khalid Themenstarter:in
3.511 Beiträge seit 2005
vor 15 Jahren

Ist das dann eigentlich ein Kündigungsgrund wenn man sowas als Programmierer in sein Code schreibt? 😁

Unglaublich...

"Jedes Ding hat drei Seiten, eine positive, eine negative und eine komische." (Karl Valentin)

Gelöschter Account
vor 15 Jahren

ich habe bereits einmal mitbekommen wie vergleichbarer code zur kündigung geführt hat.

1.985 Beiträge seit 2004
vor 15 Jahren

Hallo zusammen,

ich habe bereits einmal mitbekommen wie vergleichbarer code zur kündigung geführt hat.

verständlich, wie ich finde. Irgendwo hört der Spaß auch auf und der Programmierer hat damit ja auch ganz klar zu verstehen gegeben, dass er eigentlich gar nicht weiß, was er da macht.

Gruß,
Fabian

"Eine wirklich gute Idee erkennt man daran, dass ihre Verwirklichung von vornherein ausgeschlossen erscheint." (Albert Einstein)

Gefangen im magischen Viereck zwischen studieren, schreiben, lehren und Ideen umsetzen…

Blog: www.fabiandeitelhoff.de

M
198 Beiträge seit 2007
vor 15 Jahren

Nun dazu muss man sagen, dieser Kommentar kommt wohl vom zweiten Programmierer, also jemand der diesen Code nicht zu anfang geschrieben hat.

Der eigentliche Ersteller hat nicht einen einzigen Kommentar benutzt, trotz der absolut selbsterklärenden Variablennamen.

Edit:
Ok damit habe ich mich gerade selbst übertroffen, in zwei Sätzen 8 überflüssige Wörter ^^

Also, das Programm wurde von jemandem geschrieben, der jetzt seit mehreren Jahren nicht mehr hier ist, der Zweite, der nun auch nicht mehr hier ist, hat an vielen stellen mehr oder weniger überflüssige Kommentare hinterlassen, einfach weil er selbst nicht mehr durchgestiegen ist und wohl dachte, dass diese paar Kommentare helfen würden (wenn Command1.Enabled = true da steht ist es zwar klar, dass es sich hierbei um eine Buttonaktivierung handelt aber naja er hats versucht ^^)

5.742 Beiträge seit 2007
vor 15 Jahren

Hallo zusammen,

als ich kürzlich in etwa folgendes auf einem Blog sah, musste ich an diesen Thread denken.

Grob gesagt ging es um ein paar Möglichkeiten ("Shorthands" wurden sie genannt), C# effizienter zu verwenden.

Unter anderem wurde die Verwendung der "?:" und "??"-Operatoren sowie Auto-implemented Properties empfohlen und noch einige "Features" in dieser Art.

Für die Methode String.IsNullOrEmpty gab es sogar ein Beispielcode:


if (String.IsNullOrEmpty(s) == true)
    return "is null or empty";
else
     return String.Format("(\"{0}\") is not null or empty", s);

Da musste ich mich doch fragen, wie ernst der Autor seine eigenen Tipps nahm...

3.003 Beiträge seit 2006
vor 15 Jahren
  
if (String.IsNullOrEmpty(s) == true)  

Ein Juwel 😉.


if(booleanValue == true) return true; 
else if(booleanValue == false) return false;
else throw new Exception("Gotcha.");

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

T
574 Beiträge seit 2008
vor 15 Jahren

@LaTino

Besser find ich aber noch:

if(booleanValue == true)
   return false;
else
   return true;
390 Beiträge seit 2008
vor 15 Jahren

das nenn ich mal dynamik 😉


int crc32berechnen(char* wert, int fehlercode){

//Im Fehlerfall
return fehlercode;

}

using Skill

Gelöschter Account
vor 15 Jahren
           myclass1 m1 = new myclass1();
           if (m1 != null)
           {
               myclass2 m2 = new myclass2();
               if (m2 != null)
               {
                   myclass3 m3 = new myclass3();
                   if (m3 != null)
                   {
                       myclass4 m4 = new myclass4();
                       if (m4 != null)
                       {
                           myclass5 m5 = new myclass5();
                           if (m5 != null)
                           {
                               // tu was.....
                           }
                       }
                   }
               }
           }

da denkt noch wer in c++? und idese klammerung um alles...... grauenhaft.

4.931 Beiträge seit 2008
vor 15 Jahren

Selbst in Standard-C++ werfen Konstruktoren Ausnahmen (Exceptions), außer man verwendet explizit 'nothrow'.

Aber der Code ist wirklich grauenvoll!

3.003 Beiträge seit 2006
vor 15 Jahren

Moooaaaaah ist das fies.


//Code war ausdokumentiert weil er - Zitat - "nicht funktioniert"

for (byte i = 0; i <= 255; i++)
{
   doSomething();
} 

Wenigstens ein leichter fix 😄.

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

5.742 Beiträge seit 2007
vor 15 Jahren

Was mir kürzlich passiert ist (zwar nicht wirklich "schlimm" aber im Nachhinein...):

string s = //...
if (s.ToUpper().StartsWith("abc".ToUpper()))
    //Do Something

49.485 Beiträge seit 2005
vor 15 Jahren

Hallo winSharp93,

das ist doch eher ein best practice Beispiel. Insbesondere, wenn "abc" an einer anderen Stelle definiert ist:

const String Start = "abc";

// viel anderer Code

if (s.ToUpper().StartsWith(Start.ToUpper()))

Selbst wenn man const String Start = "ABC"; geschrieben hätte, wäre ja nicht sichergestellt, dass das später - von jemand anders - in "Xyz" geändert wird. Und dann schlägt die mit dem zweien ToUpper eingezogene Sicherheitsebene zu.

Das nur mal als Beispiel, wie sehr Wahnsinn (Coding Styles Horror) und Genie (best practice) zusammen liegen.

herbivore

E
200 Beiträge seit 2006
vor 15 Jahren

Mal wieder ein kleines Schmankerl gefunden, auch wenns dieses Mal VB.NET ist:


        SqlString = "SELECT * FROM Parameter"

        PARAMETERRECSET.Open(SqlString, Property_Renamed.con, ADODB_CursorTypeEnum.adOpenForwardOnly, ADODB_LockTypeEnum.adLockReadOnly)
        ReDim Preserve Property_Renamed.theParams(0)

        With PARAMETERRECSET
            i = 1
            Do While Not .EOF
                ReDim Preserve Property_Renamed.theParams(i)
                'Mach was mit dem Parameter
                i = i + 1
                .MoveNext()
            Loop
        End With
        PARAMETERRECSET.Close()

Und ich wunder mich warum die Software langsam ist. Schädel -> Wand

946 Beiträge seit 2008
vor 15 Jahren

Hallo herbivore

das ist doch eher ein best practice Beispiel. [...] Und dann schlägt die mit dem zweien ToUpper eingezogene Sicherheitsebene zu.

Als best practice würde ich das noch nicht bezeichnen.
Ich hätte da die Methode s.StartsWith("abc", true, null) vorgezogen.

Als Coding Style Horror würde ich die ToUpper's auch nicht abstempeln, da sie imho etwas übersichtlicher sind, aber best practice finde ich übertrieben.

mfg
SeeQuark

3.003 Beiträge seit 2006
vor 14 Jahren

*röchel*


function javascriptEnabled(){
  return true;
}

function checkJavascript(){
  if(!javascriptEnabled()){
     location.href="no_javascript.html";
  }
}

Gottseidank nicht bei uns passiert.

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

W
49 Beiträge seit 2007
vor 14 Jahren

Zwar kein wirklicher "Style Horror", aber ich frage mich schon was in manchen Leuten vorgeht 😁


if (User.IsAuthenticated != false)
{
	...
}

2.082 Beiträge seit 2005
vor 14 Jahren
if (User.IsAuthenticated != false)  
{  
    ...  
}   

2 NAND sind doch billiger als 1 OR Baustein oder wie war das 😁

function javascriptEnabled(){  
  return true;  
}  

Da fehlt ein //TODO! 🙂

Es ist toll jemand zu sein, der nichts von der persönlichen Meinung Anderer hält. - frisch-live.de

3.003 Beiträge seit 2006
vor 14 Jahren

Da fehlt ein //TODO! 😃

Äh, nein. Das war eine Javascript-Prüfung auf der Intranetseite eines Kunden. Die komischerweise nicht ging.

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)

N
89 Beiträge seit 2006
vor 14 Jahren

Äh, nein. Das war eine Javascript-Prüfung auf der Intranetseite eines Kunden. Die komischerweise nicht ging.

Das ist aber äußerst komisch 😛 . Da ist wohl das Beste, sie kaufen einen neuen Webserver, danach gehts bestimmt 😉

Je mehr ich weiß, desto mehr erkenne ich, dass ich nichts weiß.
(Albert Einstein) ...und ich kanns bestätigen 😉

360 Beiträge seit 2005
vor 14 Jahren
  
function javascriptEnabled(){  
  return true;  
}  
  

Auch thedailywtf.com gelesen? 😉

3.003 Beiträge seit 2006
vor 14 Jahren

Ähm, nein, obwohl das eigentlich zu meiner täglichen Lektüre gehört. JETZT versteh ich den Code aber - da hat sich einer der Entwickler wohl einen Scherz erlaubt...ich dachte schon, ich spinne.

LaTino

"Furlow, is it always about money?"
"Is there anything else? I mean, how much sex can you have?"
"Don't know. I haven't maxed out yet."
(Furlow & Crichton, Farscape)