1600 Zeilen Engine

[Network]

Hi,

Ich hab vor kurzem beim proggen bemerkt, dass meine Engine, die festlegt wo Gebäude hingebaut werden dürfen und wo nicht... jetzt ca. 1200 Zeilen lang ist.

Und verbraucht 9 Variablen. 3 Strings und 6 Integer.

Ich wollte einfach mal fragen, auch wenn es eine doofe Frage ist:
"Ist diese Zeilenanzahl ok?"


Ich will mich einfach nur mal an euch andere Progger richten. Und schauen wie ich im verhältnis dazu stehe.

Danke

 

[XHelp]

Quantität sagt so rein gar nichts über Qualität aus.

 

[Runtime]

1600 Zeilen sind OK, es ist egal wie lange die Datei ist, hauptsache es ist guter Code.

 

[illuminus]

Mehr uebersicht als nötig ist immer besser als zuwenig. ueberlesen kann man leicht das zuviel, aber dazudenken ist immer schwer

 

[ice-breaker]

jetzt ca. 1200 Zeilen lang ist.

Und verbraucht 9 Variablen. 3 Strings und 6 Integer.

auch wenn man nicht wirklich irgendwas anhand dieser Metriken aussagen kann, aber auf 1200 Zeilen nur 9 Variablen erscheint mir extrem wenig. Klingt sehr stark nach hunderten verschiedener If-Konstrukte, also alles andere als wirklich gut.

 

[Network]

auch wenn man nicht wirklich irgendwas anhand dieser Metriken aussagen kann, aber auf 1200 Zeilen nur 9 Variablen erscheint mir extrem wenig. Klingt sehr stark nach hunderten verschiedener If-Konstrukte, also alles andere als wirklich gut.

Natürlich sind es viele If-Konstrukte. Es handelt sich nunmal um eine:

...Engine, die festlegt wo Gebäude hingebaut werden dürfen und wo nicht...

Wie soll ich denn sonst die Daten auswerten?

 

[Gast2]

Zeig mal etwas Code. (Sehr) viele if konstrukte deuten meist auf nen designfehler hin.

 

[Noctarius]

Z.B. mit rekursiven Konstrukten könnte man sowas bestimmt auch lösen.

 

[Network]

Etwas Code...

				if( Inspektor1.equals( "FundamentDrawed" )) 
				{
					MainInspektor = MainInspektor+1;
					architect = architect + 1;
					Zuteiler = BarManagerV.get( Line-1 ).get( C ).substring(0, 5);
				}
				if( Inspektor2.equals( "FundamentDrawed" )) 
				{

				 if( architect == 1 )
				 {
				 	if( Zuteiler.equals( "A" ))
				 	{
				 		fcount = fcount + 1;
						Zuteiler = "F" + fcount;
						FundamentBetonManagement.add( FundamentMauer = new ArrayList<String>() );
					}
				 	
				 	if( Typ.equals( "V" ))
				 	{
				 		BarManagerV.get( Line ).set( C, Zuteiler + "FundamentDrawed" );
				 		MasterInspektor = "GREENLIGHT";
				 		
				 		FundamentBetonManagement.get( Integer.parseInt( Zuteiler.substring(1,5) )-1000 ).add( Typ + (Line+10) + (C+10) );
				 	}
				 	if( Typ.equals( "H" ))
				 	{
				 		BarManagerH.get( Line ).set( C, Zuteiler + "FundamentDrawed" );
				 		MasterInspektor = "GREENLIGHT";
				 		
				 		FundamentBetonManagement.get( Integer.parseInt( Zuteiler.substring(1,5) )-1000 ).add( Typ + (Line+10) + (C+10) );
					}
				}

 

[StrikeTom]

Soooo schlimm ist es doch nicht, das einzige, was du ändern solltest wäre statt

MainInspektor = MainInspektor+1;

das verwenden solltest:

MainInspektor += 1;

also variable += erhöhung

 

[Gelöschtes Mitglied 5909]

Etwas konstruktive Kritik (die nicht böse gemeint ist )

1. Wieso hast du z.b. für Typ kein enum gemacht?

2.

if( architect == 1 )

ich wette danach gehts noch weiter mit 2,3,4,5,6,...

-> auch da kann man ein enum verwenden
-> Ich wette die Methode ist sehr lang -> Teile auslagern! (refactoren)

3. Es sieht so aus als hättest du Variablen mit UpperCamelCase statt dem üblicherweise verwendeten lowerCamelCase versehen

4. Deutsch und Englisch mischen ist nicht so gut.

5.

Zuteiler = BarManagerV.get( Line-1 ).get( C ).substring(0, 5);

Das hier riecht förmlich nach Fehleranfälligkeit (einer NullPointerException, IndexOutOfBoundsException)

6.

1600 Zeilen in einer Klasse ist schon sehr viel. Schau nach wo du sachen rausziehn kannst. Mehr als gut 500 Zeilen Code sind idealerweise nicht in einer Klasse (gibt natürlich auch Außnahmen, wo es anders einfach nicht geht)

 

[mvitz]

Soooo schlimm ist es doch nicht, das einzige, was du ändern solltest wäre statt

MainInspektor = MainInspektor+1;

das verwenden solltest:

MainInspektor += 1;

also variable += erhöhung

Also abgesehen davon, dass das erste auch ok ist, würde sich wenn man in dem Falle einen Tipp gibt eher sagen:

MainInspektor++;

Davon abgesehen hat raiL wohl das meiste gesagt.

 

[Network]

@StrikeTom
Man lernt nie aus. Das wusste ich jetzt nicht. erfüllt zwar den selben Zweck ist aber um einiges leichter zu schreiben

Etwas konstruktive Kritik (die nicht böse gemeint ist )

1. Wieso hast du z.b. für Typ kein enum gemacht?

2.

if( architect == 1 )

ich wette danach gehts noch weiter mit 2,3,4,5,6,...

-> auch da kann man ein enum verwenden
-> Ich wette die Methode ist sehr lang -> Teile auslagern! (refactoren)

3. Es sieht so aus als hättest du Variablen mit UpperCamelCase statt dem üblicherweise verwendeten lowerCamelCase versehen

4. Deutsch und Englisch mischen ist nicht so gut.

5.

Zuteiler = BarManagerV.get( Line-1 ).get( C ).substring(0, 5);

Das hier riecht förmlich nach Fehleranfälligkeit (einer NullPointerException, IndexOutOfBoundsException)

6.

1600 Zeilen in einer Klasse ist schon sehr viel. Schau nach wo du sachen rausziehn kannst. Mehr als gut 500 Zeilen Code sind idealerweise nicht in einer Klasse (gibt natürlich auch Außnahmen, wo es anders einfach nicht geht)

Danke für die Tipps

1. Ja das ist allerdings war. Jedoch wusste ich von Anfang nicht wieviele Infos ich zurückzuschicken muss. Das ganze System ist ein Netz auf Variablen und ArrayLists, die jeweils die Infos der anderen Listen enthalten und erreichbar sind unter den Infos der anderen Listen... Evt. kann es deshalb passieren, dass eine Variable mehrere Infos enthalten muss für eine kurze Zeit.

2. Nein, es gibt nur 1 und 2. Alles höhere oder niedrigere wird ignoriert.

3. Eine doofe Angewohnheit von mir... ja das ist wahr.

4. Auch wieder wahr. Aber wenn man es sich merken kann "Verwaltungsvariablen" in offiziellen Deutschen Berufsbezeichnungen und untergeordnete "Infovariablen" in Englisch

5. Ohhhh jaaaaaa... Nicht durch das System selbst, aber durch nur ein vergessenes "-1" oder dergleichen. Das geht sehr schnell.

 

[XHelp]

Also abgesehen davon, dass das erste auch ok ist, würde sich wenn man in dem Falle einen Tipp gibt eher sagen:

MainInspektor++;

+=1

oder

++

nehmen sich nicht viel. Wohingegen

MainInspektor=MainInspektor+1

anders behandelt wird:

int i=2;
i=i+1;
//wird zu ByteCode:
0:   iconst_2
1:   istore_1
2:   iload_1
3:   iconst_1
4:   iadd
5:   istore_1

int i=2;
i+=1;
//und
int i=2;
i++;
//ergeben:
0:   iconst_2
1:   istore_1
2:   iinc    1, 1

 

[Noctarius]

Trotzdem wird der Hotspot das wegoptimieren

 

[maki]

Quantität sagt so rein gar nichts über Qualität aus.

Kommt darauf an

1600 Zeilen in einer Klasse in einem 5000-10000 Zeilen Programm?
Definitiv zuviel.
Wenn man sich an SRP hält, kann eine Klasse gar nicht so groß werden, ausser es ist eine GUI Klasse.

Es gibt Metriken wonach "gute" Klassen im Schnitt 250 Zeilen lang sind.

 

[Network]

Kommt darauf an

1600 Zeilen in einer Klasse in einem 5000-10000 Zeilen Programm?
Definitiv zuviel.
Wenn man sich an SRP hält, kann eine Klasse gar nicht so groß werden, ausser es ist eine GUI Klasse.

Es gibt Metriken wonach "gute" Klassen im Schnitt 250 Zeilen lang sind.

Die anderen Engines sind weitaus nicht so groß. Nur ca. 100 Zeilen lang. Die Build-Rule-Engine verpackt alle Daten in einem kleinen Päckchen und verschickt sie dann an die auszuführenden Engines.
Wie Beispielsweise die Graphic-Engine, Physic-Engine etc... Derren Aufgabe besteht mittlerweile nurnoch aus "Geschenk auspacken und damit spielen"

 

[maki]

Wie bereits bemerkt wurde, hast du da einerseits sehr viele if Konstrukte, die fragwürdige "Konstanten" abfragen, zu enum wurde ja schon geraten, da passt vielleciht schon eine Enum-Strategy (Effective Java - Google Bücher) oder eine normale Strategy.

Anonsten solltest du dir dringend die Java Code- und Namings Konventionen mal ansehen, sieht einerseits schrecklich aus, vergewaltigt sowohl die englische als auch die deutsche Sprache ( Denglisch, zB.[c]Zuteiler.equals[/c] :autsch und lässt viel Raum für Fehler wie du ja gesehen hattest

 

[Noctarius]

Ansonsten kann man sowas auch schön mit Drools (oder anderen Regel-Engines) machen

 

[Network]

Ok, wie kann man folgendes beispielsweise optimieren:
(Die Inhalte der If Befehle selbstverständlich größer, muss man sich denken, bin schreibfaul)

if( one < two )
{
       if( I42 == 10.10.10 )
       {
            Do this or that...
       }
       if( I42 == Intagonist )
       {
            "                  "
       }
}
if( two < one )
{
       if( I42 == 10.10.10 )
       {
            "                  "
       }
       if( I42 == Intagonist )
       {
            "                  "
       }
}

 

[XHelp]

Welche Datentypen spielen denn hier mit? Und vorallem was ist

10.10.10

für ein Wert? oO

 

[Noctarius]

Vermutlich eine Klasse xD

 

[Network]

Welche Datentypen spielen denn hier mit? Und vorallem was ist

10.10.10

für ein Wert? oO

Da das nur ein Beispiels ist, sagen wir einfach mal ".png"

10.10.10 ist doch gerade nur eine Anspielung auf das Datum 10.10.10. Das in der Binärdarstellung geschrieben die 42 darstellt, die wiederum die Antwort auf das Leben, das Universum und alles anderen ist, nach Douglas Adams!
Mir ist gerade nichts eingefallen...

 

[Antoras]

Nunja, dein Beispiel ist suboptimal, da es nicht beschreibt was die Abfragen prüfen. Es kam ja schon die Frage um was für Datentypen es sich handelt. Wenn es Objekte sind (was ich aufgrund deines anderen geposteten Codes annehme), dann wäre Polymorphie angebracht...