Refactor how to get an entity name

@liuchanggang

Thank you for the refactoring. Is there any reason why you put the convert methods into CloudService? Could we place the methods to Drupal\cloud\Traits\CloudContentEntityTrait?

1. +++ b/src/Traits/CloudContentEntityTrait.php
   @@ -188,7 +188,7 @@ trait CloudContentEntityTrait {
   -    return str_replace('_', ' ', $this->getModuleName($entity));
   +    return self::getLowerCaseSpace($this->getModuleName($entity));
   

   This can be replaced to getModuleNameWhitespace().

2. +++ b/src/Traits/CloudContentEntityTrait.php
   @@ -533,4 +533,46 @@ trait CloudContentEntityTrait {
   +  public static function getUpperCaseSpace($words) {
   

   The method name can be getUpperCaseWithWhitespace().

   Also, the method can be used in the following methods:
   CloudContentEntityTrait::getShortEntityTypeNameWhitespace()
CloudContentEntityTrait::getShortEntityTypeNamePluralCamel()
CloudContentEntityTrait::getShortEntityTypeNameWhitespace()

3. In that case, could you please refactor / aggregate the whole K8sFormTrait trait, too?

4. +++ b/src/Traits/CloudContentEntityTrait.php
   @@ -533,4 +533,46 @@ trait CloudContentEntityTrait {
   +    return strtolower(str_replace('_', ' ', $words));
   

   strtolower() is required?

5. +++ b/src/Traits/CloudContentEntityTrait.php
   @@ -533,4 +533,46 @@ trait CloudContentEntityTrait {
   +  public static function getCamelCase($words) {
   

   The method name can be getCamelCaseWithWhitespace().

6. Please change the method from CloudContentEntityTrait::getModuleNameWhitespace() to CloudContentEntityTrait::getModuleNameWithWhitespace()

1. +++ b/src/Traits/CloudContentEntityTrait.php
   @@ -533,4 +533,46 @@ trait CloudContentEntityTrait {
   +  public static function getWithWhitespace($words) {
   

   In this case, the method can be changeUndersocreToWhiteSpace().

2. +++ b/src/Traits/CloudContentEntityTrait.php
   @@ -533,4 +533,46 @@ trait CloudContentEntityTrait {
   +  public static function getUpperCaseWithWhitespace($words) {
   

   The method name can be getCamelCaseWithWhiteSpace().

3. +++ b/src/Traits/CloudContentEntityTrait.php
   @@ -533,4 +533,46 @@ trait CloudContentEntityTrait {
   +  public static function getCamelCaseWithoutWhitespace($words) {
   

   The method name can be getCamelCaseWithoutWhiteSpace().

4. +++ b/src/Traits/CloudContentEntityTrait.php
   @@ -187,8 +187,8 @@ trait CloudContentEntityTrait {
   +  protected function getModuleNameWithWhitespace(EntityInterface $entity): string {
   +    return self::getWithWhitespace($this->getModuleName($entity));
   

   Please change the method name to getModuleNameWithWhiteSpace().

5. Please check the coding standard violations: https://www.drupal.org/pift-ci-job/1819079

@liuchanggang

Thank you for the update.

1. +++ b/modules/cloud_service_providers/k8s/src/Traits/K8sFormTrait.php
   @@ -33,9 +36,7 @@ trait K8sFormTrait {
      protected function getShortEntityTypeNameWhitespace(CloudContentEntityBase $entity) {
   
   @@ -48,7 +49,7 @@ trait K8sFormTrait {
      protected function getShortEntityTypeNameCamel(CloudContentEntityBase $entity) {
   
   @@ -62,11 +63,7 @@ trait K8sFormTrait {
      protected function getShortEntityTypeNamePluralCamel(CloudContentEntityBase $entity) {
   

   Those methods are overlapped in CloudContentEntityTrait. Could you remove K8sFormTrait by aggregating those method? In that case, please consider the getShortEntityTypeNameUnderscore() as more generic function.

2. +++ b/src/Traits/CloudContentEntityTrait.php
   @@ -533,4 +533,43 @@ trait CloudContentEntityTrait {
   +  public static function changeUndersocreToWhiteSpace($words) {
   

   On second thought, can you change the method name to convertUndersocreToWhiteSpace()?

@liuchanggang

Thank you for the update. In my comment #26,

Those methods are overlapped in CloudContentEntityTrait. Could you remove K8sFormTrait by aggregating those method? In that case, please consider the getShortEntityTypeNameUnderscore() as more generic function.

What about this refactoring? (Could you remove K8sFormTrait by aggregating those method? In that case, please consider the getShortEntityTypeNameUnderscore() as more generic function.)

@liuchanggang

Thank you for the update. It looks good to me now. We need the test cases for the additional methods but we should create another issue.

@baldwinlouie

What do you think?

@liuchanggang,

Thank you for the patch. I have the following comments.

1. +++ b/modules/cloud_service_providers/aws_cloud/src/Form/Ec2/ElasticIpAssociateForm.php
   @@ -493,7 +496,7 @@ class ElasticIpAssociateForm extends AwsDeleteForm {
   +    $cloud_name = self::convertUndersocreToWhiteSpace($module_name);
   
   +++ b/modules/cloud_service_providers/aws_cloud/src/Form/Ec2/SnapshotCreateForm.php
   @@ -159,7 +159,7 @@ class SnapshotCreateForm extends AwsCloudContentForm {
   +    $cloud_name = self::convertUndersocreToWhiteSpace($module_name);
   

   There is a typo in underscore

2. +++ b/modules/cloud_service_providers/k8s/src/Service/K8sBatchOperations.php
   @@ -1010,7 +1012,7 @@ class K8sBatchOperations {
   +          $field_camel = lcfirst(self::getCamelCaseWithoutWhiteSpace($field));
   
   @@ -1316,7 +1318,7 @@ class K8sBatchOperations {
   +        $field_camel = lcfirst(self::getCamelCaseWithoutWhiteSpace($field));
   

   Do you think lcfirst should be an option for the getCamelCaseWithoutWhiteSpace method? That way, it can be reused in other places in the future.

@liuchanggang

Thank you for the update. It looks good to me now.

@baldwinlouie

What do you think?

@yas and @liuchanggang,

Looks good to me.

@baldwinlouie

Thank you for your review.

@liuchangchang

Could you please create the patch for 8.x-1.x and 8.x-2.x?

@liuchanggang

Thank you for the update. I'll merge the patch to 8.x-1.x, 8.x-2.x and 3.x and close this issue as Fixed.