Willkommen auf myCSharp.de! Anmelden | kostenlos registrieren
 | Suche | FAQ

Hauptmenü
myCSharp.de
» Startseite
» Forum
» Suche
» Regeln
» Wie poste ich richtig?

Mitglieder
» Liste / Suche
» Wer ist online?

Ressourcen
» FAQ
» Artikel
» C#-Snippets
» Jobbörse
» Microsoft Docs

Team
» Kontakt
» Cookies
» Spenden
» Datenschutz
» Impressum

  • »
  • Community
  • |
  • Diskussionsforum
Coding Styles Horror
LaTino
myCSharp.de - Experte

Avatar #avatar-4122.png


Dabei seit:
Beiträge: 3.003
Herkunft: Thüringen

beantworten | zitieren | melden

Falls jemand glaubt, hier waere nur nichts mehr los, weil es keinen schlechten Code mehr gibt:

What do you mean, bad style?

LaTino (beim auditing)
Attachments
"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)
private Nachricht | Beiträge des Benutzers
Alf Ator
myCSharp.de - Member



Dabei seit:
Beiträge: 656

beantworten | zitieren | melden

@LaTino Beeindruckend. Das toppt sogar meines..
Attachments
private Nachricht | Beiträge des Benutzers
Pippl
myCSharp.de - Member



Dabei seit:
Beiträge: 40

beantworten | zitieren | melden

Für einen Kunden musst wir mal eines unserer Java Frameworks nach C# konvertieren.

Leider blieb diese sinnvolle Hierarchie erhalten -> erleichtert ja die Schreibarbeit der armen Applikationsentwickler

Statt "VAGenStringUtils.[Methodenname](...)" sollen sie ja nur "[Methodenname](....)" schreiben müssen.
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von Pippl am .
Attachments
private Nachricht | Beiträge des Benutzers
inflames2k
myCSharp.de - Experte

Avatar #AARsmmPEUMee0tQa2JoB.png


Dabei seit:
Beiträge: 2.296

beantworten | zitieren | melden

Gerade in einem Projekt von 2004 das ich auf den aktuellen Stand bringen soll gefunden (Beispielhaft):


public class Config : ExecutorConfig
{
     public override void Copy(object source)
     {
           base.Copy(source);
           this.InputPath = new Document();
           this.InputPath.Path = source.InputPath.Path;
           // und so weiter..           
     }

     public Document InputPath { get; set; }

     public Document OutputPath { get; set; }
}

public class Executor
{
      public void Execute()
      {
            string inputPath = this._config.InputPath.Path;
            // Daten ermitteln
            // Daten aufbereiten
            ths.WriteOutput(this._config.OutputPath.Path, daten);
      }
}

War so frei und habe InputPath und OutputPath gegen Strings ersetzt.
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von inflames2k am .
Wissen ist nicht alles. Man muss es auch anwenden können.

PS Fritz!Box API - TR-064 Schnittstelle | PS EventLogManager | Spielkartenbibliothek
private Nachricht | Beiträge des Benutzers
LaTino
myCSharp.de - Experte

Avatar #avatar-4122.png


Dabei seit:
Beiträge: 3.003
Herkunft: Thüringen

beantworten | zitieren | melden

Bin neulich in einer Klasse über eine Exception gestolpert, die einen Tippfehler in der Nachricht hatte, die sie bekam. Hab den Typo, da ich schonmal dabei war, korrigiert.

Plötzlich schlägt ein Test in einem anderen Projekt fehl. Nachgeschaut, ob ich das war...und finde folgendes Stück Code in einer im Test verwendeten Klasse:


try 
{
    //dosomething
}
catch(SomeException ex)
{
    if(ex.Message == "Nachricht mit Tippfeler") DoSomethingElse();
}

Mal völlig abgesehen davon, dass man sich an der Message-Property orientiert - ich bin ja auch faul. Aber lieber auf die Nachricht mit Tippfehler prüfen, statt den zu korrigieren - das schlägt meine Faulheit nochmal :D.

LaTino
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von LaTino am .
"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)
private Nachricht | Beiträge des Benutzers
toxic
myCSharp.de - Member



Dabei seit:
Beiträge: 64
Herkunft: Franken ;-)

beantworten | zitieren | melden

Ein Klassiker ;-)


        public static double GetCacheSize()
        {
            return 0;
        }

Ob sich die CacheSize mal ändert - ich bin gepsannt =)
private Nachricht | Beiträge des Benutzers
gfoidl
myCSharp.de - Team

Avatar #avatar-2894.jpg


Dabei seit:
Beiträge: 6.820
Herkunft: Waidring

beantworten | zitieren | melden

Hallo,

von mir selbst gecodet...


if ((_count == 0 && index == 0) || (index == _count))

Geht doch glatt einfacher:


if (index == _count)




mfG Gü

Stellt fachliche Fragen bitte im Forum, damit von den Antworten alle profitieren. Daher beantworte ich solche Fragen nicht per PM.

"Alle sagten, das geht nicht! Dann kam einer, der wusste das nicht - und hat's gemacht!"
private Nachricht | Beiträge des Benutzers
&(*null)->Hallo
myCSharp.de - Member



Dabei seit:
Beiträge: 4
Herkunft: Deutschland

beantworten | zitieren | melden

In C++ ist folgendes möglich:

++i = ++i + ++i;

Was würde die Console für i anzeigen?

int i=0; 
++i = ++i + ++i;
cout<<"i = "<<i;

Bei mir wurde der Wert 6 angezeigt, erst wurde i dreimal inkrementiert, dann wurde 3 + 3 addiert.

Im C++ -Forum wurde das als Undefined Behavior eingestuft und mir als UB-Test empfohlen:

int i=0, a, b, c; 
a = ++i = (b=++i) + (c=++i); 
cout<<"a = "<<a<<endl; 
cout<<"b = "<<b<<endl; 
cout<<"c = "<<c<<endl; 
cout<<"i = "<<i<<endl;
private Nachricht | Beiträge des Benutzers
LaTino
myCSharp.de - Experte

Avatar #avatar-4122.png


Dabei seit:
Beiträge: 3.003
Herkunft: Thüringen

beantworten | zitieren | melden

Ist ein Klassiker in C (fehlende SP -> undefined behavior). Ich hab' offenbar ein Brett vor dem Kopf - wo ist der coding horror? (Falls es die Tatsachge ist, dass die Reihenfolge der Abarbeitung in C eben nicht spezifiziert ist - naja, ist halt C, ne?)

Empfehle dazu Peter v.d. Linden: Expert C Programming - Deep C Secrets, er breitet da genüßlich noch weitere C-Seltsamkeiten aus.

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)
private Nachricht | Beiträge des Benutzers
toxic
myCSharp.de - Member



Dabei seit:
Beiträge: 64
Herkunft: Franken ;-)

beantworten | zitieren | melden


        public static double GetCacheSize()
        {
            return 0;
        }

...ähm ja, Klassiker
private Nachricht | Beiträge des Benutzers
RED-BARON
myCSharp.de - Member



Dabei seit:
Beiträge: 74

beantworten | zitieren | melden

hallo, ich finde es nicht lustig - es ist zum Heulen ...

if ((list.Count(p=>p.IsSelected) -1) == 0 )
=> Liste leer.
??

!! => Fix: (eingecheckt, Wochen später ...)

if ((list.Count(p=>p.IsSelected) -1) < 0 )
=> Liste leer.

wenn ich jetzt sage, dass die "-1" überflüssig ist, befürchte ich bleibt

if ((list.Count(p=>p.IsSelected)) < 0 )
=> Liste leer.
??

stehen und der Fehler ist wieder drin ....... es sei denn, in der Liste ist
ein "Blind"-Objekt enthalten, was immer da drin ist. Leider weit und breit
kein Kommentar und der Rest vom Code sieht genauso aus ...
private Nachricht | Beiträge des Benutzers
Palin
myCSharp.de - Member



Dabei seit:
Beiträge: 1.090

beantworten | zitieren | melden

Zitat von RED-BARON
hallo, ich finde es nicht lustig - es ist zum Heulen ...

if ((list.Count(p=>p.IsSelected) -1) == 0 )
=> Liste leer.
Wie so soll die Liste leer sein? Sie Enthält dann genau 1 Element mit IsSelected = true.

2. Fall ist die "Leere" Liste und der 3. kann nie eintreten.

Nur so als Hinweis falls in der Hitze des Gefechts was übersehen wurde.
Sollte man mal gelesen haben:

Clean Code Developer
Entwurfsmuster
Anti-Pattern
private Nachricht | Beiträge des Benutzers
RED-BARON
myCSharp.de - Member



Dabei seit:
Beiträge: 74

beantworten | zitieren | melden

ja, übersehen - unter dem IF() steht, dass kein Eintrag existiert und jetzt ein neuer angelegt werden soll. Deswegen funktioniert 1. Variante auch nicht.
2. Variante funktioniert und ja 3 Variante würde nie funktionieren. - deswegen sag ich auch besser nichts dazu - sonst wird's wieder falsch.

Ich hätte es realisiert mit
if(!liste.Any()) oder list.count()==0 - damit das ! nicht übersehen wird auch wenn es weniger performat sein mag
{
// es existiert kein Eintrag, jetzt neu anlegen ?
}

was das IsSelected überhaupt soll - ich weiß es nicht.
Wenn ein Eintrag in der Liste ist der selektiert ist .... kommt auch die
Frage ob jetzt ein neuer angelegt werden soll, weil keiner existiert.

Das Variante 2 funktioniert im Sinne des Programmes ist reiner Zufall
und kann über die Selektierung vom Nutzer selbst bestimmt werden ...

Aber auch wieder nicht - weil diese Prüfung beim Öffnen der Ansicht/Laden
kommt und da hat niemals nie jemand was selektiert ...
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von RED-BARON am .
private Nachricht | Beiträge des Benutzers
p!lle
myCSharp.de - Member

Avatar #avatar-3556.jpg


Dabei seit:
Beiträge: 1.040

beantworten | zitieren | melden

Zitat von RED-BARON
was das IsSelected überhaupt soll - ich weiß es nicht.

Nur kurz, ohne den genauen Kontext zu kennen:
Es kann natürlich ein Unterschied sein, ob eine Liste leer ist oder ob in einer Liste kein Eintrag ausgewählt ist. Pauschal kann man das also nicht "verurteilen".
private Nachricht | Beiträge des Benutzers

Moderationshinweis von gfoidl (28.06.2017 - 10:30)

Genau, daher in den "Horrors" bitte nicht weiter vertiefen, ggf. ein neuen Thema dazu erstellen und den Kommentar am Beginn dieses Thema beachten.

FormFollowsFunction
myCSharp.de - Member

Avatar #avatar-4087.png


Dabei seit:
Beiträge: 4

beantworten | zitieren | melden

Moin

Ich muss gestehen, meinen Einstand im Lesterthread zu geben, fühlt sich etwas komisch an, aber was solls. ^^

Ursprünglich war das mal VB, habs leicht abgeändert um zu anonymisieren.

private void Button_Click(object sender, RoutedEventArgs e)
{
	Button btn = sender as Button;
	if (btn != null) {
		MessageBox.Show(((Button)sender).Name);
	} else {
		MessageBox.Show("Error: Es wurde kein Button geklickt!!");
	}
}

Moderationshinweis von gfoidl (21.09.2017 - 10:42)

Diskussion dazu in Diskussion zu Coding Styles Horror: Button_Click-Eventhandler

private Nachricht | Beiträge des Benutzers
markl
myCSharp.de - Member

Avatar #fz7qMfY8MI8pYqaUBFH8.png


Dabei seit:
Beiträge: 78

beantworten | zitieren | melden

Wenn Ihr immer schon einen "FancyRandomNameGenerator" haben wolltet, hier das Snippet for free:


var randomString = new Random(DateTime.Now.Second).ToString();

Bitte aber im UnitTest nur einmal aufrufen... wird schon laufen.
http://dotnet-paderborn.azurewebsites.net/
private Nachricht | Beiträge des Benutzers
Abt
myCSharp.de - Team

Avatar #avatar-4119.png


Dabei seit:
Beiträge: 15.832

beantworten | zitieren | melden

Ich hab mal wieder was bei einem Review gefunden, nachdem ich erst mal nen Kaffee brauchte:


public const String TRUE = "true";
public const String FALSE = "false";

Und dazu

if (obj.Value.ToString().ToUpper() == TRUE)
private Nachricht | Beiträge des Benutzers
leafar
myCSharp.de - Member

Avatar #avatar-4093.jpg


Dabei seit:
Beiträge: 5
Herkunft: Raum Soest

beantworten | zitieren | melden

Folgendes habe ich mal beim Review gesehen und ging mir seitdem nicht mehr aus dem Kopf:

string xyz = textBox1.Text.ToString();

Grüße
leafar
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von leafar am .
Developers, Developers, Developers, Developers,
Developers, Developers, Developers, Developers,
Developers, Developers, Developers, Developers ...

:evil:
private Nachricht | Beiträge des Benutzers
MrSparkle
myCSharp.de - Team

Avatar #avatar-2159.gif


Dabei seit:
Beiträge: 5.655
Herkunft: Leipzig

beantworten | zitieren | melden

Ich bin ja schon immer ein großer Fan davon gewesen, Konvertierungen zwischen Datentypen durchzuführen, indem man zuerst den Urprungswert in einen String konvertiert, und dann den String in den Zieltyp parst, so z.B.:

short currentYear = short.Parse(DateTime.Now.Year.ToString());

Das hat eigentlich nur Vorteile:
- Es ist unperformant,
- Es ist unlesbar und
- Es ist eine potentielle Fehlerquelle

Wenn man das nur konsequent durchzieht, dann sieht das z.B. so aus:


short currentYear = short.Parse(DateTime.Now.Year.ToString());
short currentMonth = short.Parse(DateTime.Now.Month.ToString());

if (selectedYear == 0 || (selectedYear < model.StartYear || selectedYear > model.EndYear))
{
	selectedYear = short.Parse(DateTime.Now.Year.ToString());
}

if (selectedYear != short.Parse(DateTime.Now.Year.ToString()))
{
	currentMonth = 0;
}

Das ist jetzt kein Code von einem Praktikanten, sondern von einer großen internationaler Firma, deren Hauptgeschäft darin besteht, Software für andere große internationale Firmen zu entwickeln. Das Problem ist hier wahrscheinlich, daß man Leuten, die sich mit der Geschäftslogik auskennen, den Auftrag gibt: "Setzt das mal in C# um, ihr wißt ja am besten wie das funktioniert, und programmiert habt doch auch schonmal."
Weeks of programming can save you hours of planning
private Nachricht | Beiträge des Benutzers
LaTino
myCSharp.de - Experte

Avatar #avatar-4122.png


Dabei seit:
Beiträge: 3.003
Herkunft: Thüringen

beantworten | zitieren | melden

Mal etwas von den hochgeschätzten Webfrickelkollegen:

Bin gerade über ein react-Projekt gestolpert, das im Ordner für die statischen Inhalte einen Unterordner für die Icons und dort in -zig Unterordnern für verschiedene Bildgrößen (Schema "8x8", "16x24", "16x16" usw.) ausschließlich .svg-Icons gesammelt haben. Hm - wofür stand das s in svg gleich wieder?

X(

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)
private Nachricht | Beiträge des Benutzers
Khalid
myCSharp.de - Experte

Avatar #avatar-2534.gif


Dabei seit:
Beiträge: 3.511
Herkunft: Hannover

Themenstarter:

beantworten | zitieren | melden

Static. Eindeutig Static...
"Jedes Ding hat drei Seiten, eine positive, eine negative und eine komische." (Karl Valentin)
private Nachricht | Beiträge des Benutzers
Palladin007
myCSharp.de - Member

Avatar #avatar-4140.png


Dabei seit:
Beiträge: 1.782
Herkunft: Düsseldorf

beantworten | zitieren | melden

Ich belebe das Mal wieder neu mit diesem kleinen Snippet:

private string _ConnectionString = @"data source=1.2.3.4\My;initial catalog=MyDB;Application Name=MyApp;user id=sa;password=;persist security info=False;packet size=4096;";

private async Task OnBerechnen()
{
	return await Task.Factory.StartNew(() =>
	{
		using (System.Data.SqlClient.SqlConnection conn = new System.Data.SqlClient.SqlConnection(_ConnectionString))
		{
			conn.Open();
			
			// ...
		}
	});
}

Das stand so im Code, am ConnectionString habe ich nur IP und Name geändert, UserId und Passwort standen genau so im Code.
Mich macht sowas sprachlos ...

Natürlich ist das nicht mehr der Fall und den Server gibt's auch nicht mehr, aber das lief jahrelang so.
private Nachricht | Beiträge des Benutzers
Stefan.Haegele
myCSharp.de - Member

Avatar #avatar-3068.jpg


Dabei seit:
Beiträge: 457
Herkunft: Untermeitingen

beantworten | zitieren | melden

Zitat von Palladin007
Natürlich ist das nicht mehr der Fall und den Server gibt's auch nicht mehr, aber das lief jahrelang so.

Böse gesagt - alles richtig gemacht wenn es jahrelang so lief, oder? :-) Wir machen uns eindeutig zu viele Gedanken über Sicherheit und Kennwortstärke...
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von Stefan.Haegele am .
private Nachricht | Beiträge des Benutzers
Palladin007
myCSharp.de - Member

Avatar #avatar-4140.png


Dabei seit:
Beiträge: 1.782
Herkunft: Düsseldorf

beantworten | zitieren | melden

Aber es ist doch so einfach, Mal eben einen simplen User anzulegen, der nur Lese-Rechte hat :/
Ich verstehe nicht, warum man das damals nicht getan hat, ich kann nur froh sein, dass es bisher keinen Angreifer gab, der auf die Idee kam, die Anwendung zu decompilen und nach ConnectionStrings zu suchen, immerhin läuft das Programm auf DAU-Rechnern.
private Nachricht | Beiträge des Benutzers
KroaX
myCSharp.de - Member

Avatar #avatar-4080.jpg


Dabei seit:
Beiträge: 301
Herkunft: Köln

beantworten | zitieren | melden

Und ich hab die ersten 30 Sekunden darüber nachgedacht was daran so schlimm sein könnte eine Connection in nem eigenen Task aufzumachen :-D
private Nachricht | Beiträge des Benutzers
LaTino
myCSharp.de - Experte

Avatar #avatar-4122.png


Dabei seit:
Beiträge: 3.003
Herkunft: Thüringen

beantworten | zitieren | melden


public static Nullable<T> GetEnum<T>(int? EnumInt) where T : struct {
    foreach (var item in Enum.GetValues(typeof(T))) {
        if ((int)item == EnumInt) {
            return (T?)item;
        }
    }
    return null;
}

Okay, soweit, so...ungewöhnlich, aber gut, wenn einer das lieber selber macht als die .NET-Methoden zu nehmen, des Menschen Wille ist sein Himmelreich.

Aber dann. Die Methode wird zweimal verwendet. Beide Male so:


(MyEnumType) Utils.GetEnum<MyEnumType>((int) data.Property)

Die Punchline? data.Property ist vom Typ MyEnumType.
Wir nehmen das Enum, casten nach int, werfen das int in eine Methode, die einen Nullable von dem Enum baut, von dem wir kommen, und casten dieses Nullable dann hart in den Ursprungstyp.

*mikedrop*

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)
private Nachricht | Beiträge des Benutzers
T-Virus
myCSharp.de - Member



Dabei seit:
Beiträge: 1.977
Herkunft: Nordhausen, Nörten-Hardenberg

beantworten | zitieren | melden

Sieht nach einem "2 Uhr Morgens" Code aus und macht auch genau soviel Sinn :)

T-Virus
Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.
private Nachricht | Beiträge des Benutzers
LaTino
myCSharp.de - Experte

Avatar #avatar-4122.png


Dabei seit:
Beiträge: 3.003
Herkunft: Thüringen

beantworten | zitieren | melden

Der Urheber ist nicht mehr bei meinem AG beschäftigt. Seinem Code nach zu urteilen, hat er aber sehr oft "nachts gearbeitet". Der ist jetzt übrigens (wieder) in der Lehre beschäftigt - lehrt OOP. Manche Geschichten kann man sich nicht ausdenken.

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)
private Nachricht | Beiträge des Benutzers
Palladin007
myCSharp.de - Member

Avatar #avatar-4140.png


Dabei seit:
Beiträge: 1.782
Herkunft: Düsseldorf

beantworten | zitieren | melden

Sieht für mich wie Code aus, den man über einen längeren Zeitraum von Int-Flags auf ein Enum umstelen musste und dabei die eine oder andere Zwischenlösung vergessen hat.
Hab ich so in der Art schon öffter gesehen :D
private Nachricht | Beiträge des Benutzers
Urza
myCSharp.de - Member



Dabei seit:
Beiträge: 68

beantworten | zitieren | melden

Ich habe am 29.02.2020 lernen müssen, dass dieser Codeschnipsel wahrscheinlich nicht der geschickteste ist.
Zum Glück war das ein Samstag...
Autor war ich leider selbst.

Due = DateTime.Now;
From = new DateTime(Due.Year - 1, Due.Month, Due.Day);

Urza
“Knowledge cannot replace friendship. I'd rather be an idiot than lose you.”

- Patrick to Spongebob
private Nachricht | Beiträge des Benutzers