Re: Visual C# - Krótszy kod nie znaczy lepszy?

2016-01-26T18:36:34+01:00

Już wyjaśniam

Metoda GetAllGridObjects jest źle napisana, bo pętla while wykonuje się tyle razy ile jest sumarycznie obiektów co jest głupie, ponieważ powinna się wykonać tyle razy
ile obiektów ma najdłuższy Listing. Czyli jeśli childrenWire zawiera 100 elementów i to jest najwięcej, np childrenSquare ma tylko 20 elementów to pętla się powinna wykonać 100 razy, nie mniej i nie więcej. Aktualnie to działa tak, że jeśli listing childrenWire ma 100 elementów a childrenSquare 20, pętla wykonuje się 120 razy - głupota! Nie ma to jednak super mega wielkiego znaczenia, ponieważ ile można umieścić na stronie. Ta liczba nie przekracza nigdy 60 elementów z wszystkich listingów w sumie!

Z tym try{} catch{} jest tak, że jak mam wszystko osobno to w przypadku błędu ominie mi tylko jeden listing. Jeśli bym dał wszystkie listingi do jednego try'a to wtedy
jeżeli w pierwszym z góry listingu wywali wyjątek, to przechodzi do instrukcji catch{} pomijając całą resztę i w rezultacie tracę całą resztę o aktualnie przzeszukiwanym indeksie.

Zarówno metoda GetAlGridObject() jak i FindAll zwraca List. Nie wiem czy te metody wykonują się z każdy "obrotem" pętli foreach czy też działa to tak,
że tylko raz wykonuje te metody i w następnym obiegu pętli w tamtym miejscu już jest poprzednio zwrócony listing i tylko leci po indeksach.

Statystyki: Napisane przez komandos332 — 26 sty 2016, o 18:36

Re: Visual C# - Krótszy kod nie znaczy lepszy?

2016-01-26T18:08:55+01:00

Specem od C# nie jestem, ale na pewno nie musisz każdej instrukcji "try-catchować" wystarczy jeden obejmujący całość operacji, jak gdzieś wyskoczy wyjątek to Ci go ładnie złapie. Ten try-catch może chyba w sobie nawet obejmować całe ciało pętli.

Zapisy w stylu foreach z reguły są wolniejsze niż zwykłe fory czy while.

Nie wiem jak dokładnie działa foreach w C#, ale próbowałeś może kolekcję wrzucić do zmiennej przed foreach i po niej iterować? Być może problemem jest ciągła potrzeba wykonania "findAll" ?

Statystyki: Napisane przez marmur — 26 sty 2016, o 18:08

Visual C# - Krótszy kod nie znaczy lepszy?

2016-01-26T17:48:30+01:00

Witam

Podczas tworzenia pewnego kodu zaczęła mnie irytować jego obszerność w związku z czym postanowiłem go troszkę uprościć.
Napotkałem się przy tym jak dla mnie, na ciekawą zagwozdkę. Mam kila różnych klas, z których każda ma metodę
o nazwie Move ( PoinF p ); i wiele innych metod o tej samej nazwie.
Na początku, kod do przemieszczania obiektów wyglądał tak:

UWAGA! "ify" są pod komentarzem ponieważ wcześniej ich nie było.

[syntax=csharp]// if ( selectedObject is Relay || selectedObjects.Contains ( typeof ( Relay ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenRelay.Count - 1; i++ )
{

Relay relay = (Relay)gridList[actualGrid].childrenRelay[i];
if ( relay.Selected )
{
relay.Move ( gridList[actualGrid].ActualGridPoint );
}

if ( relay.TableSelected )
{
relay.MoveTable ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is RelaySlave || selectedObjects.Contains ( typeof ( RelaySlave ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenRelaySlave.Count - 1; i++ )
{

RelaySlave relaySlave = (RelaySlave)gridList[actualGrid].childrenRelaySlave[i];
if ( relaySlave.Selected )
{
relaySlave.Move ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is CircuitBreaker || selectedObjects.Contains ( typeof ( CircuitBreaker ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenCircuitBreaker.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenCircuitBreaker[i].Selected )
{
gridList[actualGrid].childrenCircuitBreaker[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is Lamp || selectedObjects.Contains ( typeof ( Lamp ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenLamp.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenLamp[i].Selected )
{
gridList[actualGrid].childrenLamp[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is MainSwitch || selectedObjects.Contains ( typeof ( MainSwitch ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenMainSwitch.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenMainSwitch[i].Selected )
{
gridList[actualGrid].childrenMainSwitch[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is FrontSwitch || selectedObjects.Contains ( typeof ( FrontSwitch ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenFronSwitch.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenFronSwitch[i].Selected )
{
gridList[actualGrid].childrenFronSwitch[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is SensorSwitch || selectedObjects.Contains ( typeof ( SensorSwitch ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenSensorSwitch.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenSensorSwitch[i].Selected )
{
gridList[actualGrid].childrenSensorSwitch[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is DescriptionText || selectedObjects.Contains ( typeof ( DescriptionText ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenText.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenText[i].Selected )
{
gridList[actualGrid].childrenText[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is Square || selectedObjects.Contains ( typeof ( Square ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenSquare.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenSquare[i].Selected )
{
gridList[actualGrid].childrenSquare[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is ObjectImage || selectedObjects.Contains ( typeof ( ObjectImage ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenImage.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenImage[i].Selected )
{
gridList[actualGrid].childrenImage[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is Ellipse || selectedObjects.Contains ( typeof ( Ellipse ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenEllipse.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenEllipse[i].Selected )
{
gridList[actualGrid].childrenEllipse[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}

//if ( selectedObject is Cable || selectedObjects.Contains ( typeof ( Cable ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenCable.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenCable[i].Selected )
{
gridList[actualGrid].childrenCable[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}

// if ( selectedObject is UserObject || selectedObjects.Contains ( typeof ( UserObject ) ) || ModifierKeys == Keys.Control )
for ( int i = 0; i <= gridList[actualGrid].childrenUserObject.Count - 1; i++ )
{
if ( gridList[actualGrid].childrenUserObject[i].Selected )
{
gridList[actualGrid].childrenUserObject[i].Move ( gridList[actualGrid].ActualGridPoint );
}
}[/syntax]

Duży, tłusty i rozlazły, jak się wydawało niepotrzebnego kodu. Pomyślałem sobie, że skoro każda z tych klas ma nazwe Move ( PoinF p ); to pyknę ją i resztę wspólnych metod do interfejsu. Całość o identycznym działaniu zawarłem w takim kodzie:

[syntax=csharp]foreach ( object obj in ( gridList[actualGrid].GetAllGridObjects ( ) ).FindAll ( i => ( i as BasicObject ).Selected ) )
{
if ( !( obj is Wire ) && !( obj is Line ) && !( obj is CableClamp ) )
( obj as IBasics ).Move ( gridList[actualGrid].ActualGridPoint );
}[/syntax]

I tu niespodzianka. Program zaczął klatkować podczas przemieszczania obiektów. Moje pytanie brzmi, dlaczego?

Tak wygląda metoda GetAllGridObiects ( ); - Też źle napisana wiem. Zmienię to, mimo wszystko nie jest tak obszerna, żeby w takim stopniu
zwalniać procek. Jaka jest różnica czasu wykonywania kodu? Za pomocą klasy Stopwatch zmierzyłem, że wykonywanie się długiego, obrzydliwego
i przerażającego kodu wynosi mniej niż 1 ms. Zaś czas wykonywania krótkiego foreach trwa nawet 10ms!?!?!?! Czy metoda FindAll jest tak prockożerna?

[syntax=csharp]public List GetAllGridObjects ( )
{
AllGridObjects = new List ( );
int index = 0, passedAdd = 0;

int totalItemsCount = childrenCircuitBreaker.Count
+ childrenFronSwitch.Count
+ childrenMainSwitch.Count
+ childrenRelay.Count
+ childrenRelaySlave.Count
+ childrenSensorSwitch.Count
+ childrenSquare.Count
+ childrenLine.Count
+ childrenText.Count
+ childrenWire.Count
+ childrenEllipse.Count
+ childrenUserObject.Count
+ childrenCable.Count
+ childrenCableClamp.Count
+ childrenLamp.Count
+ childrenImage.Count;

while ( passedAdd != totalItemsCount )
{
try { AllGridObjects.Add ( childrenCircuitBreaker[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenFronSwitch[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenMainSwitch[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenRelay[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenRelaySlave[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenSensorSwitch[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenSquare[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenText[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenWire[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenLine[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenEllipse[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenUserObject[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenCable[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenCableClamp[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenLamp[index] ); passedAdd++; }
catch { }
try { AllGridObjects.Add ( childrenImage[index] ); passedAdd++; }
catch { }
index++;
}
return AllGridObjects;
}[/syntax]

Pozdrawiam, Maciek.

Statystyki: Napisane przez komandos332 — 26 sty 2016, o 17:48

ATNEL tech-forum

Re: Visual C# - Krótszy kod nie znaczy lepszy?

Re: Visual C# - Krótszy kod nie znaczy lepszy?

Visual C# - Krótszy kod nie znaczy lepszy?