Zu viel Code?

[Mark110]

Ist folgender Code zu groß sollte ich das anders schrieben?
Funktionieren tut er.

ctChecked = pageEF.getCheckboxCombinationTruck()
						.isChecked();
				if (ct.equals("-1")) {
					if (ctChecked == false) {
						pageEF.getCheckboxCombinationTruck().click();
					}
				} else {
					if (ctChecked == true) {
						pageEF.getCheckboxCombinationTruck().click();
					}
				}
stChecked = pageEF.getCheckboxSemitrailerTruck()
						.isChecked();
				if (st.equals("-1")) {
					if (stChecked == false) {
						pageEF.getCheckboxSemitrailerTruck().click();
					}
				} else {
					if (stChecked == true) {
						pageEF.getCheckboxSmallVehicle().click();
					}
				}

				svChecked = pageEF.getCheckboxSmallVehicle()
						.isChecked();
				if (sv.equals("-1")) {
					if (svChecked == false) {
						pageEF.getCheckboxSmallVehicle().click();
					}
				} else {
					if (svChecked == true) {
						pageEF.getCheckboxSmallVehicle().click();
					}
				}

 

[Gast]

Besonders schön ist das nicht. Aber wie sagt man immer so schön, wenns läuft und nicht gerade der Kern der Applikation ist, dann passt es schon irgendwie.


Verbesserungsvorschläge:

Was ist den mit den inneren "else" Fällen? Zumindest ein Kommentar wäre nett.

Ich sehe drei analoge Abfrageblöcke, kann man das nicht über ein gemeinsamens Interface und eine entsprechende Methode machen?

equals("-1") was soll den das bitteschön sein? Wenn das schon so komisch sein muss, dann würde ich für das sv, st bzw. ct Objekt eine Methode bereitstellen, die mir true/false zurück gibt. isValid o.ä.

Dann noch ein Klassiker if("xxx == true"), da reicht if(xxx) bei false analog.

 

[SlaterB]

der Aufbau der drei Abschnitte läßt vermuten, dass in Zeile 20
auch getCheckboxSemitrailerTruck() stehen sollte, und nicht getCheckboxSmallVehicle(),
stimmts?
--------
in jedem Fall sollte die benötigte Checkbox nicht 3x mit pageEF.getCheckboxSmallVehicle() einzeln angesprochen werden,
das provoziert nur Fehler wie den obigen,
-> in einer Variablen speichern, currentCheckBox,

oder:
die drei Abschnitte könnten zu einem zusammengekürzt werde, der in einer Extra-Methode steht und mit drei einfachen Aufrufen a la
checkCheckBox(pageEF.getCheckboxSmallVehicle(), sv);
aufgerufen wird

-------

> if (svChecked == false) {

-> if (!svChecked) {

-------

> if (svChecked == true) {

-> if (svChecked) {

--------

> if (sv.equals("-1")) {
> if (svChecked == false) {
> pageEF.getCheckboxSmallVehicle().click();
> }
> } else {
> if (svChecked == true) {
> pageEF.getCheckboxSmallVehicle().click();
> }
> }

->


if (sv.equals("-1") != svChecked) {
pageEF.getCheckboxSmallVehicle().click();
}

 

[Mark110]

Danke für die hilreichen Antworten.
Natürlich habt irh recht.. mit meinen IF Abfragen.. ich hatte gedacht es sähe überishclticher aus wenn ich dort == false oder true schreibe.

Das habe ich jetzt mal korrigiert.

		freightList = new FreightOfferList();

			tl = freightList.getFreightOffersFromDatabase();
			lp.getButtonEnterFreightOffers().click();

			for (FreightOffer fOffer : tl.getFreightOffers()) {
				date = tchelperclass.util.Date.getDatum();

				pageEF.getButtonNew().click();
				pageEF.getFieldOn().setText(date);
				pageEF.getListCountryFrom().click(3);
				pageEF.getListCountryTo().click(3);
				pageEF.getFieldPostalCodeFrom().setText(fOffer.getVonPLZ());
				pageEF.getFieldTownFrom().setText(fOffer.getVonOrt());
				pageEF.getFieldPostalCodeTo().setText(fOffer.getNachPLZ());
				pageEF.getFieldTownTo().setText(fOffer.getNachOrt());
				pageEF.getFieldLenght().setText(fOffer.getLadungLaenge());
				pageEF.getFieldWeight().setText(fOffer.getLadungGewicht());
				pageEF.getFieldLoadingPlaces().setText(fOffer.getBeladen());
				pageEF.getFieldDischarginPlaces().setText(fOffer.getEntladen());
				pageEF.getFieldTypOfGods().setText(fOffer.getLadung_ware());
				pageEF.getFieldPriceOfGoods().setText(fOffer.getLadung_preis());

				ct = fOffer.getLkw_gliederzug();
				st = fOffer.getLkw_sattelzug();
				sv = fOffer.getLkw_klein_fahrz();
				adr = fOffer.getLadung_adr();
				obtp = fOffer.getLkwAlternativAufbau();

				ctChecked = pageEF.getCheckboxCombinationTruck().isChecked();
				if (ct.equals("-1")) {
					if (!(ctChecked)) {
						pageEF.getCheckboxCombinationTruck().click();
					}
				} else {
					if (ctChecked) {
						pageEF.getCheckboxCombinationTruck().click();
					}
				}

				stChecked = pageEF.getCheckboxSemitrailerTruck().isChecked();
				if (st.equals("-1")) {
					if (!(stChecked)) {
						pageEF.getCheckboxSemitrailerTruck().click();
					}
				} else {
					if (stChecked) {
						pageEF.getCheckboxSemitrailerTruck().click();
					}
				}

				svChecked = pageEF.getCheckboxSmallVehicle().isChecked();
				if (sv.equals("-1")) {
					if (!(svChecked)) {
						pageEF.getCheckboxSmallVehicle().click();
					}
				} else {
					if (svChecked) {
						pageEF.getCheckboxSmallVehicle().click();
					}
				}

				adrChecked = pageEF.getCheckboxADR().isChecked();
				if (adr.equals("-1")) {
					if (!(adrChecked)) {
						pageEF.getCheckboxADR().click();
					}
				} else {
					if (adrChecked) {
						pageEF.getCheckboxADR().click();
					}
				}

				otherBodyTypsPossibleChecked = pageEF.getCheckboxOtherBodyTypesPossible().isChecked();
				if (obtp.equals("-1")) {
					if (!(otherBodyTypsPossibleChecked)) {
						pageEF.getCheckboxOtherBodyTypesPossible().click();
					}
				} else {
					if (otherBodyTypsPossibleChecked) {
						pageEF.getCheckboxOtherBodyTypesPossible().click();
					}
				}

				pageEF.getListTypOfBody().click(
						Integer.valueOf(fOffer.getLkwAufbau()).intValue());
				pageEF.getFieldRemarks().setText(fOffer.getBemerkung());
				pageEF.getListContactInYourCompany().click(1);
				pageEF.getFieldInternalRemarks()
						.setText(fOffer.getIBemerkung());
				pageEF.getButtonSave().click();
			}
			pageEF.getButtonExit().click();

Mein Vorteil ist, wenn ich die checkbox dreimal neu mit pageEF.getCheckboxSmallVehicle() angesprochen wird, dass ich bei einem Typ Wechsel in der klasse die mir die pageEF erzeugt in der klasse nichts mehr ändern muss.

 

[SlaterB]

> Mein Vorteil ist, wenn ich die checkbox dreimal neu mit pageEF.getCheckboxSmallVehicle() angesprochen wird [..]
verstehe ich nicht,

jedenfalls war das == true noch einer der kleinsten Tipps,
die anderen beiden
- drei ifs zu einem zusammenfassen
- die nun sogar 5 gleichartig aufgebauten Blöche in eine Methode ausgliedern
sind viel wichtiger

 

[Mark110]

hmm irgendwie hab ich keinen ansazt wie ich die drei abshcnitt zu einem zusammenfassen kann.

kannst du ein kleines codebeispiel machen?


EDIT: hab gerade gesheen, das du bereits ein Codebeispiel gemacht hast. Werd mal veruschen das umzusetzen

Danke

 

[SlaterB]

hatte ich doch schon, der Aufruf lautet
checkCheckBox(pageEF.getCheckboxSmallVehicle(), sv);

und in diese Methode muss offensichtlich einer dieser Blöcke:

otherBodyTypsPossibleChecked = pageEF.getCheckboxOtherBodyTypesPossible().isChecked();
if (obtp.equals("-1")) {
if (!(otherBodyTypsPossibleChecked)) {
pageEF.getCheckboxOtherBodyTypesPossible().click();
}
} else {
if (otherBodyTypsPossibleChecked) {
pageEF.getCheckboxOtherBodyTypesPossible().click();
}
}


ein solcher Block hängt nur von der CheckBox und dem String mit der -1 ab,
diese beiden werden ja als Parameter übergeben, also sind alle benötigten Infos da

programmieren musst du es aber schon alleine, sonst nützt das ja nix,

-------

falls du '3 if zu einem if' meinst, das hatte ich ja auch schon gepostet

 

[Gast]

Hier ein ganz banales Beispiel:

int a = 5;
int b = 7;
int c = 6;

a = a+5;
a = a*5;
a = a+1;

b = b+5;
b = b*5;
b = b+1;

c = c+5;
c = c*5;
c = c+1;

wird zu

a = init(5);
b = init(6);
c = init(7);
...

public static int init(int start){
  start = start+5;
  start = start*5;
  start = start+1;
  return start;
}