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

Reply via email to