Allon Mureinik has posted comments on this change. Change subject: findbugs: use StringBuilder in a for loop ......................................................................
Patch Set 1: I would prefer that you didn't submit this (20 inline comments) .................................................... File backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/LinkHelper.java Line 847: * @param params the URL params to append Line 848: * @return the combined head and params Line 849: */ Line 850: public static String combine(String head, List<ParametersSet> params) { Line 851: StringBuilder combined_params = new StringBuilder(""); The empty string in the constructor is redundant. Line 852: if (params != null) { Line 853: for (ParametersSet ps : params) { Line 854: for (Parameter param : ps.getParameters()) { Line 855: combined_params.append(String.format(MATRIX_PARAMETER_TEMPLATE, param.getName(), param.getValue())); .................................................... File backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ssh/SSHDialogTest.java Line 377: final String LINE = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAASDSSSSSSSSSSSSSSSSSSSSSSSDDDDDD"; Line 378: final int NUM = 10000; Line 379: final int FACTOR = 5; Line 380: Line 381: StringBuilder longText = new StringBuilder(""); The empty string in the constructor is redundant. Line 382: for (int i=0;i<NUM/FACTOR;i++) { Line 383: longText.append(LINE).append("\n"); Line 384: } Line 385: .................................................... File backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/DomainsConfigurationEntry.java Line 28: } Line 29: } Line 30: Line 31: public String getDomainsConfigurationEntry() { Line 32: StringBuilder configurationEntry = new StringBuilder(""); The empty string in the constructor is redundant. Line 33: boolean firstEntry = true; Line 34: Line 35: for ( Entry<String,String> currEntry: valuePerDomain.entrySet() ) { Line 36: if (!firstEntry) { .................................................... File backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/PasswordDomainsConfigurationEntry.java Line 10: Line 11: // This method returns the entry for logging purposes Line 12: @Override Line 13: public String getDomainsLoggingEntry() { Line 14: StringBuilder configurationEntry = new StringBuilder(""); The empty string in the constructor is redundant. Line 15: boolean firstEntry = true; Line 16: Line 17: for (Entry<String, String> currEntry : valuePerDomain.entrySet()) { Line 18: if (!firstEntry) { .................................................... File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/view/popup/AbstractDiskRemoveConfirmationPopupView.java Line 63: for (int i = 0; i < notes.size(); i++) { Line 64: String note = notes.get(i); Line 65: formattedNote.append(constants.lineBreak()); Line 66: formattedNote.append(constants.htmlTab()); Line 67: formattedNote.append(note); Consider doing this in a single line: formattedNote.append(constants.lineBreak().append(constants.htmlTab().append(note); Line 68: Line 69: } Line 70: Line 71: return formattedNote.toString(); .................................................... File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/AbstractUiCommandButton.java Line 56: getButtonWidget().setEnabled(command.getIsExecutionAllowed()); Line 57: Line 58: // Use prohibition reasons for tooltip if exist. Line 59: String title = ""; //$NON-NLS-1$ Line 60: StringBuilder sb = new StringBuilder(""); The empty string here is redundent Line 61: if (!command.getExecuteProhibitionReasons().isEmpty()) { Line 62: for (String reason : command.getExecuteProhibitionReasons()) { Line 63: sb.append(reason).append(","); //$NON-NLS-1$ Line 64: } Line 62: for (String reason : command.getExecuteProhibitionReasons()) { Line 63: sb.append(reason).append(","); //$NON-NLS-1$ Line 64: } Line 65: title = sb.toString(); Line 66: if (!title.equals("")) //$NON-NLS-1$ Don't use toString() here, just check if title.length()==0 Line 67: { Line 68: title = title.substring(0, title.length() -1); Line 69: } Line 70: } else { .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/CommonModel.java Line 982: String[] array = source.split("[:]", -1); //$NON-NLS-1$ Line 983: String entityClause = array[0]; Line 984: String searchClause = array[1]; Line 985: Line 986: StringBuilder tagsClause = new StringBuilder(""); //$NON-NLS-1$ The empty string here is redundent Line 987: for (TagModel tag : tags) Line 988: { Line 989: tagsClause.append("tag=").append(tag.getName().getEntity()); //$NON-NLS-1$ Line 990: if (tag != tags.get(tags.size() - 1)) .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterStorageListModel.java Line 518: Line 519: private String GetLocalStoragesFormattedString() Line 520: { Line 521: StringBuilder localStorages = new StringBuilder(""); //$NON-NLS-1$ Line 522: for (StorageDomain a : Linq.<StorageDomain> cast(getSelectedItems())) The empty string here is redundent Line 523: { Line 524: if (a.getStorageType() == StorageType.LOCALFS) Line 525: { Line 526: localStorages.append(a.getStorageName()).append(", "); //$NON-NLS-1$ .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/TaskListModel.java Line 55: entry.setValue(new ArrayList<Job>()); Line 56: correlationTaskMap.put(rootTask.getCorrelationId(), entry); Line 57: String[] taskDescreptionArray = Line 58: rootTask.getCorrelationId().replace(_WEBADMIN_, "").split("_"); //$NON-NLS-1$ //$NON-NLS-2$ Line 59: StringBuilder taskDesc = new StringBuilder(""); //$NON-NLS-1$ The empty string here is redundent Line 60: for (int i = 1; i < taskDescreptionArray.length; i++) { Line 61: taskDesc.append(taskDescreptionArray[i]).append(" "); //$NON-NLS-1$ Line 62: } Line 63: rootTask.setId(task.getId()); .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageDataCenterListModel.java Line 469: } Line 470: Line 471: private String GetLocalStoragesFormattedString() Line 472: { Line 473: StringBuilder localStorages = new StringBuilder(""); //$NON-NLS-1$ The empty string here is redundent Line 474: for (StorageDomain a : Linq.<StorageDomain> cast(getSelectedItems())) Line 475: { Line 476: if (a.getStorageType() == StorageType.LOCALFS) Line 477: { .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java Line 523: { Line 524: ArrayList<StoragePool> dataCenterList = Line 525: (ArrayList<StoragePool>) getDataCenter().getItems(); Line 526: ArrayList<StoragePool> localDCList = new ArrayList<StoragePool>(); Line 527: StringBuilder dataCenterQueryLine = new StringBuilder(""); //$NON-NLS-1$ The empty string here is redundent Line 528: Line 529: for (StoragePool storagePool : dataCenterList) Line 530: { Line 531: if (storagePool.getstorage_pool_type() == StorageType.LOCALFS) .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/TemplateBackupModel.java Line 230: (ArrayList<VdcReturnValueBase>) result.getReturnValue(); Line 231: if (retVals != null && templateBackupModel.getSelectedItems().size() == retVals.size()) Line 232: { Line 233: Line 234: StringBuilder importedTemplates = new StringBuilder(""); //$NONNLS1$ The empty string here is redundent Line 235: int counter = 0; Line 236: boolean toShowConfirmWindow = false; Line 237: for (Object a : templateBackupModel.getSelectedItems()) Line 238: { Line 249: confirmModel.setTitle(ConstantsManager.getInstance() Line 250: .getConstants() Line 251: .importTemplatesTitle()); Line 252: confirmModel.setHashName("import_template"); //$NON-NLS-1$ Line 253: importedTemplates = new StringBuilder(StringHelper.trimEnd(importedTemplates.toString().trim(), ',')); why do you need a StringBuilder here? all you do is invoke toString() on it in a couple of lines Line 254: confirmModel.setMessage(ConstantsManager.getInstance() Line 255: .getMessages() Line 256: .importProcessHasBegunForTemplates(importedTemplates.toString())); Line 257: UICommand tempVar = new UICommand("CancelConfirm", templateBackupModel); //$NON-NLS-1$ .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java Line 446: .getReturnValue(); Line 447: if (retVals != null Line 448: && vmBackupModel.getSelectedItems().size() == retVals Line 449: .size()) { Line 450: StringBuilder importedVms = new StringBuilder(""); //$NON-NLS-1$ The empty string here is redundent Line 451: int counter = 0; Line 452: boolean toShowConfirmWindow = false; Line 453: for (Object item : vmBackupModel.getSelectedItems()) { Line 454: VM vm = (VM) item; Line 467: confirmModel.setTitle(ConstantsManager.getInstance() Line 468: .getConstants() Line 469: .importVirtualMachinesTitle()); Line 470: confirmModel.setHashName("import_virtual_machine"); //$NON-NLS-1$ Line 471: importedVms = new StringBuilder(StringHelper.trimEnd(importedVms.toString().trim(), ',')); why use a StringBuilder here? Line 472: confirmModel.setMessage(ConstantsManager.getInstance() Line 473: .getMessages() Line 474: .importProcessHasBegunForVms(importedVms.toString())); Line 475: UICommand tempVar2 = new UICommand("CancelConfirm", //$NON-NLS-1$ .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/BootSequenceModel.java Line 74: } Line 75: Line 76: public BootSequence getSequence() Line 77: { Line 78: StringBuilder str = new StringBuilder(""); //$NON-NLS-1$ The empty string here is redundent Line 79: for (EntityModel a : getItems()) Line 80: { Line 81: if (a.getIsChangable()) Line 82: { .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/CustomPropertyValidation.java Line 88: } Line 89: } Line 90: if (!contains) Line 91: { Line 92: StringBuilder parameters = new StringBuilder(""); //$NON-NLS-1$ The empty string here is redundent Line 93: Line 94: for (String keyValue : getCustomPropertiesKeysList()) Line 95: { Line 96: parameters.append(keyValue.substring(0, keyValue.indexOf('='))).append(", "); //$NON-NLS-1$ Line 94: for (String keyValue : getCustomPropertiesKeysList()) Line 95: { Line 96: parameters.append(keyValue.substring(0, keyValue.indexOf('='))).append(", "); //$NON-NLS-1$ Line 97: } Line 98: parameters = new StringBuilder(parameters.substring(0, parameters.length() - 2)); replace with parameters.delete(parameters.length()-2,parameters.length()-1) Line 99: Line 100: String reasonStr = Line 101: ConstantsManager.getInstance() Line 102: .getMessages() .................................................... File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/renderer/DetailsRenderer.java Line 16: } Line 17: Line 18: @Override Line 19: public String render(ArrayList<ValueLabel<V>> widgets) { Line 20: StringBuilder formattedStr = new StringBuilder(""); //$NON-NLS-1$ The empty string here is redundent Line 21: Line 22: for (int i = 0; i < widgets.size(); i++) { Line 23: formattedStr.append(widgets.get(i).getElement().getInnerHTML()).append(" ").append(delimiters[i]); //$NON-NLS-1$ Line 24: if (i < widgets.size() - 1) { -- To view, visit http://gerrit.ovirt.org/14367 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If11e9b335a741aaa364893474573d226ca9ece72 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Asaf Shakarchi <a...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches