Skip to content

CLOUDSTACK-10365: Change the "getXXX" boolean-related method names to… #2602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Kui-Liu wants to merge 1 commit into from
Closed

CLOUDSTACK-10365: Change the "getXXX" boolean-related method names to… #2602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/src/main/java/com/cloud/agent/api/to/S3TO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public DataStoreRole getRole() {
return DataStoreRole.Image;
}

public boolean getEnableRRS() {
public boolean isEnableRRS() {
Copy link
Contributor

@DaanHoogland DaanHoogland Apr 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the is prefix for in this case? is the question enableRRS() ?

Copy link
Member

@rafaelweingartner rafaelweingartner May 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not enableRSS give the idea that it is a method that "enables" RSS?
At least, that is what sounds to me.

return enableRRS;
}

Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/com/cloud/offering/NetworkOffering.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public enum Detail {

boolean getIsPersistent();

boolean getInternalLb();
boolean isInternalLb();

boolean getPublicLb();

Expand Down
4 changes: 2 additions & 2 deletions api/src/main/java/com/cloud/offering/ServiceOffering.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public enum StorageType {
/**
* @return Does this service plan offer HA?
*/
boolean getOfferHA();
boolean isOfferHA();
Copy link
Contributor

@DaanHoogland DaanHoogland Apr 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the java doc implies it should be called doesOfferHA()

Copy link
Contributor

@fmaximus fmaximus May 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHoogland I would prefer to only have methods starting with either get or is.
Any other method name in a VO would not be usable for filtering,
according to the implementation of the VO MethodInterceptor

Copy link
Contributor

@DaanHoogland DaanHoogland May 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point @fmaximus so the generic dao breaks if we go any other way is your message, is it?

Copy link
Member

@rafaelweingartner rafaelweingartner May 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know the name of the class that does this interception, so we can inspect it?
Is it using method names or POJO's properties directly via their annotation?


/**
* @return Does this service plan offer VM to use CPU resources beyond the service offering limits?
Expand All @@ -86,7 +86,7 @@ public enum StorageType {
/**
* @return Does this service plan support Volatile VM that is, discard VM's root disk and create a new one on reboot?
*/
boolean getVolatileVm();
boolean isVolatileVm();
Copy link
Contributor

@DaanHoogland DaanHoogland Apr 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supportVolitileVMs() would be more pleasing on the human eye. Not that that should hamper this change but what is the objective? to adhere to another naming convention or to make better code for reading? and seriously both are valid motives under circumstances, but I am really wondering.

Copy link
Contributor Author

@Kui-Liu Kui-Liu May 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention of my pull requests is to make code more readable and understandable, thus further making developers to understand the code more easily with method names without more context information.
But about method naming things, developers have two different opinions as you mentioned:
(1) keep the same naming convention as similar methods.
(2) make code more readable.

Copy link
Contributor

@DaanHoogland DaanHoogland May 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brucekuiliu I am with 2 all the way, with the exception of the use of these methods in code generation tools. In this last method I would expect a name like isForVolatileVms() or the ealier sugested name. Of cause the accessor is named after the field implying that the field is not properly named either.
I am +0 on this one. Try to find more reviews.


/**
* @return the rate in megabits per sec to which a VM's network interface is throttled to
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/com/cloud/storage/GuestOS.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,5 @@ public interface GuestOS extends InternalIdentity, Identity {

Date getRemoved();

boolean getIsUserDefined();
boolean isUserDefined();
}
4 changes: 2 additions & 2 deletions api/src/main/java/com/cloud/template/VirtualMachineTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ public enum TemplateFilter {

String getDisplayText();

boolean getEnablePassword();
boolean isEnablePassword();

boolean getEnableSshKey();
boolean isEnableSshKey();

boolean isCrossZones();

Expand Down
2 changes: 1 addition & 1 deletion .../main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public String getDeploymentPlanner() {
return deploymentPlanner;
}

public boolean getCustomized() {
public boolean isCustomized() {
return (cpuNumber == null || memory == null || cpuSpeed == null);
}

Expand Down
2 changes: 1 addition & 1 deletion api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public Long getSize() {
return size;
}

public boolean getShrinkOk() {
public boolean isShrinkOk() {
return shrinkOk;
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ public long download(boolean resume, DownloadCompleteCallback callback) {
PutObjectRequest putObjectRequest = new PutObjectRequest(s3TO.getBucketName(), s3Key, inputStream, objectMetadata);

// If reduced redundancy is enabled, set it.
if (s3TO.getEnableRRS()) {
if (s3TO.isEnableRRS()) {
putObjectRequest.withStorageClass(StorageClass.ReducedRedundancy);
}

Expand Down
2 changes: 1 addition & 1 deletion engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3195,7 +3195,7 @@ public boolean upgradeVmDb(final long vmId, final long serviceOfferingId) {
final VMInstanceVO vmForUpdate = _vmDao.createForUpdate();
vmForUpdate.setServiceOfferingId(serviceOfferingId);
final ServiceOffering newSvcOff = _entityMgr.findById(ServiceOffering.class, serviceOfferingId);
vmForUpdate.setHaEnabled(newSvcOff.getOfferHA());
vmForUpdate.setHaEnabled(newSvcOff.isOfferHA());
vmForUpdate.setLimitCpuUse(newSvcOff.getLimitCpuUse());
vmForUpdate.setServiceOfferingId(newSvcOff.getId());
return _vmDao.update(vmId, vmForUpdate);
Expand Down
2 changes: 1 addition & 1 deletion engine/schema/src/main/java/com/cloud/network/vpc/PrivateIpVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public Long getVpcId() {
return vpcId;
}

public boolean getSourceNat() {
public boolean isSourceNat() {
return sourceNat;
}

Expand Down
2 changes: 1 addition & 1 deletion engine/schema/src/main/java/com/cloud/offerings/NetworkOfferingVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ public boolean getIsPersistent() {
}

@Override
public boolean getInternalLb() {
public boolean isInternalLb() {
return internalLb;
}

Expand Down
8 changes: 4 additions & 4 deletions engine/schema/src/main/java/com/cloud/service/ServiceOfferingVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,15 @@ public ServiceOfferingVO(ServiceOfferingVO offering) {
speed = offering.getSpeed();
rateMbps = offering.getRateMbps();
multicastRateMbps = offering.getMulticastRateMbps();
offerHA = offering.getOfferHA();
offerHA = offering.isOfferHA();
limitCpuUse = offering.getLimitCpuUse();
volatileVm = offering.getVolatileVm();
volatileVm = offering.isVolatileVm();
hostTag = offering.getHostTag();
vmType = offering.getSystemVmType();
}

@Override
public boolean getOfferHA() {
public boolean isOfferHA() {
return offerHA;
}

Expand Down Expand Up @@ -296,7 +296,7 @@ public int getSortKey() {
}

@Override
public boolean getVolatileVm() {
public boolean isVolatileVm() {
return volatileVm;
}

Expand Down
2 changes: 1 addition & 1 deletion engine/schema/src/main/java/com/cloud/storage/GuestOSVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public Date getRemoved() {
}

@Override
public boolean getIsUserDefined() {
public boolean isUserDefined() {
return isUserDefined;
}

Expand Down
4 changes: 2 additions & 2 deletions engine/schema/src/main/java/com/cloud/storage/VMTemplateVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ private VMTemplateVO(Long id, String uniqueName, String name, ImageFormat format
}

@Override
public boolean getEnablePassword() {
public boolean isEnablePassword() {
return enablePassword;
}

Expand Down Expand Up @@ -573,7 +573,7 @@ public boolean isDynamicallyScalable() {
}

@Override
public boolean getEnableSshKey() {
public boolean isEnableSshKey() {
return enableSshKey;
}

Expand Down
8 changes: 4 additions & 4 deletions ...storage/image/src/main/java/org/apache/cloudstack/storage/image/store/TemplateObject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,13 +403,13 @@ public String getDisplayText() {
}

@Override
public boolean getEnablePassword() {
return imageVO.getEnablePassword();
public boolean isEnablePassword() {
return imageVO.isEnablePassword();
}

@Override
public boolean getEnableSshKey() {
return imageVO.getEnableSshKey();
public boolean isEnableSshKey() {
return imageVO.isEnableSshKey();
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions engine/storage/src/main/java/org/apache/cloudstack/storage/image/TemplateEntityImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,13 @@ public String getDisplayText() {
}

@Override
public boolean getEnablePassword() {
public boolean isEnablePassword() {
// TODO Auto-generated method stub
return false;
}

@Override
public boolean getEnableSshKey() {
public boolean isEnableSshKey() {
// TODO Auto-generated method stub
return false;
}
Expand Down
8 changes: 4 additions & 4 deletions framework/quota/src/main/java/org/apache/cloudstack/quota/vo/ServiceOfferingVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,15 @@ public ServiceOfferingVO(ServiceOfferingVO offering) {
speed = offering.getSpeed();
rateMbps = offering.getRateMbps();
multicastRateMbps = offering.getMulticastRateMbps();
offerHA = offering.getOfferHA();
offerHA = offering.isOfferHA();
limitCpuUse = offering.getLimitCpuUse();
volatileVm = offering.getVolatileVm();
volatileVm = offering.isVolatileVm();
hostTag = offering.getHostTag();
vmType = offering.getSystemVmType();
}

@Override
public boolean getOfferHA() {
public boolean isOfferHA() {
return offerHA;
}

Expand Down Expand Up @@ -251,7 +251,7 @@ public int getSortKey() {
}

@Override
public boolean getVolatileVm() {
public boolean isVolatileVm() {
return volatileVm;
}

Expand Down
2 changes: 1 addition & 1 deletion ...ments/elastic-loadbalancer/src/main/java/com/cloud/network/lb/LoadBalanceRuleHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ private DomainRouterVO deployELBVm(Network guestNetwork, final DeployDestination
ServiceOfferingVO elasticLbVmOffering = _serviceOfferingDao.findDefaultSystemOffering(ServiceOffering.elbVmDefaultOffUniqueName, ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dest.getDataCenter().getId()));
elbVm = new DomainRouterVO(id, elasticLbVmOffering.getId(), vrProvider.getId(), VirtualMachineName.getSystemVmName(id, _instance, ELB_VM_NAME_PREFIX),
template.getId(), template.getHypervisorType(), template.getGuestOSId(), owner.getDomainId(), owner.getId(), userId, false, RedundantState.UNKNOWN,
elasticLbVmOffering.getOfferHA(), false, null);
elasticLbVmOffering.isOfferHA(), false, null);
elbVm.setRole(Role.LB);
elbVm = _routerDao.persist(elbVm);
_itMgr.allocate(elbVm.getInstanceName(), template, elasticLbVmOffering, networks, plan, null);
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/api/ApiResponseHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3203,7 +3203,7 @@ public GuestOSResponse createGuestOSResponse(GuestOS guestOS) {
GuestOSResponse response = new GuestOSResponse();
response.setDescription(guestOS.getDisplayName());
response.setId(guestOS.getUuid());
response.setIsUserDefined(Boolean.valueOf(guestOS.getIsUserDefined()).toString());
response.setIsUserDefined(Boolean.valueOf(guestOS.isUserDefined()).toString());
GuestOSCategoryVO category = ApiDBUtils.findGuestOsCategoryById(guestOS.getCategoryId());
if (category != null) {
response.setOsCategoryId(category.getUuid());
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/api/SerializationContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public static SerializationContext current() {
return context;
}

public boolean getUuidTranslation() {
public boolean isUuidTranslation() {
return _doUuidTranslation;
}

Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/api/query/dao/AsyncJobJoinDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public AsyncJobResponse newAsyncJobResponse(AsyncJobJoinVO job) {
}
jobResponse.setJobResultCode(job.getResultCode());

boolean savedValue = SerializationContext.current().getUuidTranslation();
boolean savedValue = SerializationContext.current().isUuidTranslation();
SerializationContext.current().setUuidTranslation(false);

Object resultObject = ApiSerializerHelper.fromSerializedString(job.getResult());
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ protected Map<String, Object> createProxyInstance(long dataCenterId, VMTemplateV
}
ConsoleProxyVO proxy =
new ConsoleProxyVO(id, serviceOffering.getId(), name, template.getId(), template.getHypervisorType(), template.getGuestOSId(), dataCenterId,
systemAcct.getDomainId(), systemAcct.getId(), _accountMgr.getSystemUser().getId(), 0, serviceOffering.getOfferHA());
systemAcct.getDomainId(), systemAcct.getId(), _accountMgr.getSystemUser().getId(), 0, serviceOffering.isOfferHA());
proxy.setDynamicallyScalable(template.isDynamicallyScalable());
proxy = _consoleProxyDao.persist(proxy);
try {
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/com/cloud/network/NetworkServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1376,7 +1376,7 @@ public Network doInTransaction(TransactionStatus status) throws InsufficientCapa
if (_configMgr.isOfferingForVpc(ntwkOff)) {
throw new InvalidParameterValueException("Network offering can be used for VPC networks only");
}
if (ntwkOff.getInternalLb()) {
if (ntwkOff.isInternalLb()) {
throw new InvalidParameterValueException("Internal Lb can be enabled on vpc networks only");
}

Expand Down Expand Up @@ -2751,7 +2751,7 @@ private boolean canMoveToPhysicalNetwork(Network network, long oldNetworkOfferin

//can't update from internal LB to public LB
if (areServicesSupportedByNetworkOffering(oldNetworkOfferingId, Service.Lb) && areServicesSupportedByNetworkOffering(newNetworkOfferingId, Service.Lb)) {
if (oldNetworkOffering.getPublicLb() != newNetworkOffering.getPublicLb() || oldNetworkOffering.getInternalLb() != newNetworkOffering.getInternalLb()) {
if (oldNetworkOffering.getPublicLb() != newNetworkOffering.getPublicLb() || oldNetworkOffering.isInternalLb() != newNetworkOffering.isInternalLb()) {
throw new InvalidParameterValueException("Original and new offerings support different types of LB - Internal vs Public," + " can't upgrade");
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2511,7 +2511,7 @@ public void isLbServiceSupportedInNetwork(long networkId, Scheme scheme) {
throw new InvalidParameterValueException("Scheme " + scheme + " is not supported by the network offering " + off);
}
} else {
if (!off.getInternalLb()) {
if (!off.isInternalLb()) {
throw new InvalidParameterValueException("Scheme " + scheme + " is not supported by the network offering " + off);
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ public DomainRouterVO deployRouter(final RouterDeploymentDefinition routerDeploy
continue;
}

final boolean offerHA = routerOffering.getOfferHA();
final boolean offerHA = routerOffering.isOfferHA();

// routerDeploymentDefinition.getVpc().getId() ==> do not use
// VPC because it is not a VPC offering.
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/network/vpc/PrivateIpAddress.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public PrivateIpAddress(PrivateIpVO privateIp, String broadcastUri, String gatew
this.netmask = netmask;
this.macAddress = macAddress;
this.networkId = privateIp.getNetworkId();
this.sourceNat = privateIp.getSourceNat();
this.sourceNat = privateIp.isSourceNat();
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/com/cloud/server/ManagementServerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2179,7 +2179,7 @@ public GuestOS updateGuestOs(final UpdateGuestOsCmd cmd) {
throw new InvalidParameterValueException("Guest OS not found. Please specify a valid ID for the Guest OS");
}

if (!guestOsHandle.getIsUserDefined()) {
if (!guestOsHandle.isUserDefined()) {
throw new InvalidParameterValueException("Unable to modify system defined guest OS");
}

Expand Down Expand Up @@ -2221,7 +2221,7 @@ public boolean removeGuestOs(final RemoveGuestOsCmd cmd) {
throw new InvalidParameterValueException("Guest OS not found. Please specify a valid ID for the Guest OS");
}

if (!guestOs.getIsUserDefined()) {
if (!guestOs.isUserDefined()) {
throw new InvalidParameterValueException("Unable to remove system defined guest OS");
}

Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
Long newMinIops;
Long newMaxIops;
Integer newHypervisorSnapshotReserve;
boolean shrinkOk = cmd.getShrinkOk();
boolean shrinkOk = cmd.isShrinkOk();

VolumeVO volume = _volsDao.findById(cmd.getEntityId());
if (volume == null) {
Expand Down
Loading