# 1600 Zeilen Engine



## Network (23. Okt 2010)

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 (23. Okt 2010)

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


----------



## Runtime (23. Okt 2010)

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


----------



## illuminus (23. Okt 2010)

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


----------



## ice-breaker (23. Okt 2010)

Network hat gesagt.:


> 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 (24. Okt 2010)

ice-breaker hat gesagt.:


> 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:


Network hat gesagt.:


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



Wie soll ich denn sonst die Daten auswerten?


----------



## Gast2 (24. Okt 2010)

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


----------



## Noctarius (24. Okt 2010)

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


----------



## Network (24. Okt 2010)

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 (24. Okt 2010)

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 (24. Okt 2010)

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 (24. Okt 2010)

StrikeTom hat gesagt.:


> Soooo schlimm ist es doch nicht, das einzige, was du ändern solltest wäre statt
> 
> ```
> MainInspektor = MainInspektor+1;
> ...



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 (24. Okt 2010)

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




raiL hat gesagt.:


> Etwas konstruktive Kritik (die nicht böse gemeint ist  )
> 
> 1. Wieso hast du z.b. für Typ kein enum gemacht?
> 
> ...



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 (24. Okt 2010)

mvitz hat gesagt.:


> 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 (24. Okt 2010)

Trotzdem wird der Hotspot das wegoptimieren


----------



## maki (24. Okt 2010)

XHelp hat gesagt.:


> 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 (24. Okt 2010)

maki hat gesagt.:


> Kommt darauf an
> 
> 1600 Zeilen in einer Klasse in einem 5000-10000 Zeilen Programm?
> Definitiv zuviel.
> ...



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 (24. Okt 2010)

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 (24. Okt 2010)

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


----------



## Network (25. Okt 2010)

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 (25. Okt 2010)

Welche Datentypen spielen denn hier mit? Und vorallem was ist 
	
	
	
	





```
10.10.10
```
 für ein Wert? oO


----------



## Noctarius (25. Okt 2010)

Vermutlich eine Klasse xD


----------



## Network (25. Okt 2010)

XHelp hat gesagt.:


> Welche Datentypen spielen denn hier mit? Und vorallem was ist
> 
> 
> 
> ...



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 (25. Okt 2010)

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...


----------

