1

私のメトリック ツールによると、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);
    }
4

4 に答える 4

3

あなたはそれらのsomeString.substring(0)ものをたくさん持っています（私が正しく数えた場合、43回の出現）。Java では文字列は不変であるため、文字列を割り当てるだけで十分であり、null のチェックは廃止されます。

于 2012-04-25T12:03:57.863 に答える
2

クラスAddressに含まれるフィールドが多すぎます。BankInfo理想的には、オブジェクトを、RecInfo、PhoneInfo、などの多くの小さなオブジェクトに分割する必要がありますFreeValues。そして、Addressクラスにこれらのオブジェクトへの変数を含めます。

于 2012-04-25T12:07:56.183 に答える
1

.substring(0)通話で何を達成しようとしていますか?

これにより、元のオブジェクトと同じ基になる char 配列を持つ別のオブジェクトが作成されます。これを使用したくなるかもしれません。しかし、とにかく別のオブジェクトが必要なのはなぜですか? 文字列は不変であるため、 の両方のインスタンスが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のようです。Addressクラスが住所のみを表し、銀行の詳細がAccountクラスに移動し、最後の変更の詳細がクラスに表示されるなどの場合、コードはより明確になりますAuditDetails。)

于 2012-04-25T12:04:24.510 に答える
0

文字列をコピーしようとするべきではなく、そうしないことで条件を取り除くことができると言った他のすべての人に同意します。ただし、これを追加する価値はあります。条件付きコピーを行う必要があった場合 (たとえば、それらが文字列ではなく、他の種類のオブジェクトである場合)、繰り返されるロジックを除外することをお勧めします。

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);
  // ...
}

疑念を避けるために、上記はこの特定のケースでは正しい解決策ではありません.同じコードを何度も書く場合は、一度だけ書いて繰り返し使用する方法を探す必要があります。

于 2012-04-25T12:11:34.997 に答える