Cyclometic複雜性巨大 - 克隆對象更好的方法？

Question

我的度量工具表示copyTo（）方法的圈複雜度爲44.這非常大。 如何使用更好更快的方法創建地址對象的克隆？注意：我沒有可用的Cloneable界面（移動應用程序）。

這裏是類：

class Address 
{ 
    Address() 
    { 
     init(); 
    } 

    String company1; 
    String company2; 
    String degree; 
    String firstName; 
    String lastName; 
    String salutation; 
    String toPerson; 
    String poBox; 
    String country; 
    String postalCode; 
    String place; 
    String street; 
    String telephone1; 
    String telephone2; 
    String mobilePhone; 
    String privatePhone; 
    String fax; 
    String eMail; 
    String homepage; 
    String socialSecurityNumber; 
    String bank1Name; 
    String bank1BCN; 
    String bank1AccountNr; 
    String bank2Name; 
    String bank2BCN; 
    String bank2AccountNr; 
    String free01Name; 
    String free01Value; 
    String free02Name; 
    String free02Value; 
    String free03Name; 
    String free03Value; 
    String free04Name; 
    String free04Value; 
    String free05Name; 
    String free05Value; 
    String comment; 
    String cityCode; 
    GpsPosition position; 
    int recId; 
    int recStatus; 
    DateTime recCreated; 
    String recCreatedBy; 
    String recCreatedByProg; 
    DateTime recChanged; 
    String recChangedBy; 
    String recChangedByProg; 
    AddressPropertyUsage usage; 

    /* Generates a duplicate of the Address object */ 
    Address clone() 
    { 
     Address clonedObject = new Address(); 

     copyTo(clonedObject); 

     return clonedObject; 
    } 

    void copyTo(Address destination) 
    { 
     if (destination.usage == null) 
      destination.usage = new AddressPropertyUsage(this.usage.value); 
     else 
      destination.usage.value = this.usage.value; 

     destination.recId = this.recId; 
     destination.recStatus = this.recStatus; 
     destination.recChanged = this.recChanged.clone(); 

     if (this.recChangedBy == null) 
      destination.recChangedBy = null; 
     else 
      destination.recChangedBy = this.recChangedBy.substring(0); 

     if (this.recChangedByProg == null) 
      destination.recChangedByProg = null; 
     else 
      destination.recChangedByProg = this.recChangedByProg.substring(0); 

     destination.recCreated = this.recCreated.clone(); 

     if (this.recCreatedBy == null) 
      destination.recCreatedBy = null; 
     else 
      destination.recCreatedBy = this.recCreatedBy.substring(0); 

     if (this.recCreatedByProg == null) 
      destination.recCreatedByProg = null; 
     else 
      destination.recCreatedByProg = this.recCreatedByProg.substring(0); 

     if (this.company1 == null) 
      destination.company1 = null; 
     else 
      destination.company1 = this.company1.substring(0); 

     if (this.company2 == null) 
      destination.company2 = null; 
     else 
      destination.company2 = this.company2.substring(0); 

     if (this.degree == null) 
      destination.degree = null; 
     else 
      destination.degree = this.degree.substring(0); 

     if (this.firstName == null) 
      destination.firstName = null; 
     else 
      destination.firstName = this.firstName.substring(0); 

     if (this.lastName == null) 
      destination.lastName = null; 
     else 
      destination.lastName = this.lastName.substring(0); 

     if (this.street == null) 
      destination.street = null; 
     else 
      destination.street = this.street.substring(0); 

     if (this.postalCode == null) 
      destination.postalCode = null; 
     else 
      destination.postalCode = this.postalCode.substring(0); 

     if (this.place == null) 
      destination.place = null; 
     else 
      destination.place = this.place.substring(0); 

     if (this.country == null) 
      destination.country = null; 
     else 
      destination.country = this.country.substring(0); 

     if (this.poBox == null) 
      destination.poBox = null; 
     else 
      destination.poBox = this.poBox.substring(0); 

     if (this.telephone1 == null) 
      destination.telephone1 = null; 
     else 
      destination.telephone1 = this.telephone1.substring(0); 

     if (this.telephone2 == null) 
      destination.telephone2 = null; 
     else 
      destination.telephone2 = this.telephone2.substring(0); 

     if (this.mobilePhone == null) 
      destination.mobilePhone = null; 
     else 
      destination.mobilePhone = this.mobilePhone.substring(0); 

     if (this.fax == null) 
      destination.fax = null; 
     else 
      destination.fax = this.fax.substring(0); 

     if (this.eMail == null) 
      destination.eMail = null; 
     else 
      destination.eMail = this.eMail.substring(0); 

     if (this.homepage == null) 
      destination.homepage = null; 
     else 
      destination.homepage = this.homepage.substring(0); 

     if (this.salutation == null) 
      destination.salutation = null; 
     else 
      destination.salutation = this.salutation.substring(0); 

     if (this.toPerson == null) 
      destination.toPerson = null; 
     else 
      destination.toPerson = this.toPerson.substring(0); 

     if (this.privatePhone == null) 
      destination.privatePhone = null; 
     else 
      destination.privatePhone = this.privatePhone.substring(0); 

     if (this.comment == null) 
      destination.comment = null; 
     else 
      destination.comment = this.comment.substring(0); 

     if (this.cityCode == null) 
      destination.cityCode = null; 
     else 
      destination.cityCode = this.cityCode.substring(0); 

     this.position.copyTo(destination.position); 

     if (this.bank1AccountNr == null) 
      destination.bank1AccountNr = null; 
     else 
      destination.bank1AccountNr = this.bank1AccountNr.substring(0); 

     if (this.bank1BCN == null) 
      destination.bank1BCN = null; 
     else 
      destination.bank1BCN = this.bank1BCN.substring(0); 

     if (this.bank1Name == null) 
      destination.bank1Name = null; 
     else 
      destination.bank1Name = this.bank1Name.substring(0); 

     if (this.bank2AccountNr == null) 
      destination.bank2AccountNr = null; 
     else 
      destination.bank2AccountNr = this.bank2AccountNr.substring(0); 

     if (this.bank2BCN == null) 
      destination.bank2BCN = null; 
     else 
      destination.bank2BCN = this.bank2BCN.substring(0); 

     if (this.bank2Name == null) 
      destination.bank2Name = null; 
     else 
      destination.bank2Name = this.bank2Name.substring(0); 

     if (this.socialSecurityNumber == null) 
      destination.socialSecurityNumber = null; 
     else 
      destination.socialSecurityNumber = this.socialSecurityNumber.substring(0); 

     if (this.free01Name == null) 
      destination.free01Name = null; 
     else 
      destination.free01Name = this.free01Name.substring(0); 

     if (this.free01Value == null) 
      destination.free01Value = null; 
     else 
      destination.free01Value = this.free01Value.substring(0); 

     if (this.free02Name == null) 
      destination.free02Name = null; 
     else 
      destination.free02Name = this.free02Name.substring(0); 

     if (this.free02Value == null) 
      destination.free02Value = null; 
     else 
      destination.free02Value = this.free02Value.substring(0); 

     if (this.free03Name == null) 
      destination.free03Name = null; 
     else 
      destination.free03Name = this.free03Name.substring(0); 

     if (this.free03Value == null) 
      destination.free03Value = null; 
     else 
      destination.free03Value = this.free03Value.substring(0); 

     if (this.free04Name == null) 
      destination.free04Name = null; 
     else 
      destination.free04Name = this.free04Name.substring(0); 

     if (this.free04Value == null) 
      destination.free04Value = null; 
     else 
      destination.free04Value = this.free04Value.substring(0); 

     if (this.free05Name == null) 
      destination.free05Name = null; 
     else 
      destination.free05Name = this.free05Name.substring(0); 

     if (this.free05Value == null) 
      destination.free05Value = null; 
     else 
      destination.free05Value = this.free05Value.substring(0); 
    } 


    private void init() 
    { 
     this.company1 = StringHelper.emptyString; 
     this.company2 = StringHelper.emptyString; 
     this.degree = StringHelper.emptyString; 
     this.firstName = StringHelper.emptyString; 
     this.lastName = StringHelper.emptyString; 
     this.salutation = StringHelper.emptyString; 
     this.toPerson = StringHelper.emptyString; 
     this.poBox = StringHelper.emptyString; 
     this.country = StringHelper.emptyString; 
     this.postalCode = StringHelper.emptyString; 
     this.place = StringHelper.emptyString; 
     this.street = StringHelper.emptyString; 
     this.telephone1 = StringHelper.emptyString; 
     this.telephone2 = StringHelper.emptyString; 
     this.mobilePhone = StringHelper.emptyString; 
     this.privatePhone = StringHelper.emptyString; 
     this.fax = StringHelper.emptyString; 
     this.eMail = StringHelper.emptyString; 
     this.homepage = StringHelper.emptyString; 
     this.socialSecurityNumber = StringHelper.emptyString; 
     this.bank1Name = StringHelper.emptyString; 
     this.bank1BCN = StringHelper.emptyString; 
     this.bank1AccountNr = StringHelper.emptyString; 
     this.bank2Name = StringHelper.emptyString; 
     this.bank2BCN = StringHelper.emptyString; 
     this.bank2AccountNr = StringHelper.emptyString; 
     this.free01Name = StringHelper.emptyString; 
     this.free01Value = StringHelper.emptyString; 
     this.free02Name = StringHelper.emptyString; 
     this.free02Value = StringHelper.emptyString; 
     this.free03Name = StringHelper.emptyString; 
     this.free03Value = StringHelper.emptyString; 
     this.free04Name = StringHelper.emptyString; 
     this.free04Value = StringHelper.emptyString; 
     this.free05Name = StringHelper.emptyString; 
     this.free05Value = StringHelper.emptyString; 
     this.comment = StringHelper.emptyString; 
     this.cityCode = StringHelper.emptyString; 
     this.position = new GpsPosition(); 

     this.recId = 0; 
     this.recStatus = -1; 
     this.recCreated = DateTime.UNKNOWN_PC.clone(); 
     this.recCreatedBy = StringHelper.emptyString; 
     this.recCreatedByProg = StringHelper.emptyString; 
     this.recChanged = DateTime.UNKNOWN_PC.clone(); 
     this.recChangedBy = StringHelper.emptyString; 
     this.recChangedByProg = StringHelper.emptyString; 
     this.usage = new AddressPropertyUsage(AddressPropertyUsage.VERSION3_ALL); 
    } 

Answer 1

您試圖通過.substring(0)來電實現什麼？

這創建了另一個對象，它具有與原始字體相同的底層字符數組，我可以看到它可能會誘使您使用它。但是爲什麼你還需要另一個物體呢？字符串是不可變的，因此Address的兩個實例都可以直接使用相同的String。

而一旦這不礙事，你不需要任何的if陳述的，可以直接指定值，例如：

// No need to create new instances of String... 
// if (this.recChangedBy == null) 
//  destination.recChangedBy = null; 
// else 
//  destination.recChangedBy = this.recChangedBy.substring(0); 

// so above is replaced by: 
destination.recChangedBy = this.recChangedBy; 

瞧，大部分/所有的條件已經消失，圈複雜度已經降低。

---

（在不同的主題，如何經常你的地址修改？我會很誘惑，使這是一個不可變對象，並設置所有的值在構造函數中，這恕我直言，使其更容易推理並使用特別是相對於構建一個空的地址

也沒有調用私有init方法的構造器 - 。只是使該方法的身體構造

而且我不認爲地址真的有44個字段，這看起來像是一個God Object。你的代碼很可能是cl earer如果Address類只表示的地址，銀行信息，請到一個Account類，最後一個變化細節出現在AuditDetails類等）

Answer 2

你有很多的那些東西someString.substring(0)（如果43個OCCURENCES我正確計算）。由於字符串在Java中是不可變的，只需分配它們就足夠了，因此對null的檢查將會過時。

Answer 3

你Address類包含了太多的領域。理想情況下，你應該把你的對象分成許多小對象，如：BankInfo,RecInfo,PhoneInfo,FreeValues等。然後讓你的Address類包含這些對象的變量。

Answer 4

我同意所有其他人說你不應該試圖複製字符串，並且不這樣做可以讓你擺脫條件。但值得補充的是：如果確實需要做那個條件複製 - 例如如果那些東西是不是字符串而是其他類型的對象 - 那麼你會得到很好的建議，以分解出重複的邏輯：

String copyUnlessNull(String s) { 
    if (s==null) return null; 
    return s.substring(0); 
} 

void copyTo(Address destination) { 
    // ... 
    destination.recChangedBy  = copyUnlessNull(this.recChangedBy); 
    destination.recChangedByProg = copyUnlessNull(this.recChangedByProg); 
    // ... 
} 

爲免生疑問，上面是不權在這種特殊情況下解決方案 - 你應該根本不用打擾substring（）的東西，然後你不需要條件邏輯 - 但是每當你發現自己一遍又一遍地寫同樣的代碼時，你應該尋找一種只寫一次並反覆使用的方法。