Skip to content
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.

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

Refactoring Suggestion - Address Class #142

Open
lborja04 opened this issue Jan 9, 2024 · 0 comments · May be fixed by #164
Open

Refactoring Suggestion - Address Class #142

lborja04 opened this issue Jan 9, 2024 · 0 comments · May be fixed by #164

Comments

@lborja04
Copy link

lborja04 commented Jan 9, 2024

THIS ISSUE IS POSTED AS A COLLEGE ASSIGNMENT ON BAD CODING SMELLS AND REFACTORING TECHNIQUES. PLEASE CLOSE THE ISSUE IF IT IS OF NO USE.

Hi shashirajraja,

I hope this message finds you well. After reviewing the Address class in your codebase, I wanted to propose a potential improvement and suggest incorporating the "Remove Setting Method" refactoring technique.

Why Refactor?

Encapsulation and Cohesion: The current class primarily acts as a data holder, and adding meaningful behavior to the class can enhance encapsulation and cohesion.
Maintainability: By introducing constructor-based initialization and removing setter methods, you improve the class's predictability and make it easier to maintain.
Avoiding Redundancy: Implementing methods like equals(), hashCode(), and providing a custom toString() can contribute to a cleaner, more organized design.
Refactoring Proposal:

import java.util.Objects;

public class Address {
    private final String addressLine1;
    private final String addressLine2;
    private final String city;
    private final String state;
    private final String country;
    private final long pinCode;
    private final String phone;

    public Address(String addressLine1, String addressLine2, String city, String state, String country, long pinCode, String phone) {
        this.addressLine1 = addressLine1;
        this.addressLine2 = addressLine2;
        this.city = city;
        this.state = state;
        this.country = country;
        this.pinCode = pinCode;
        this.phone = phone;
    }

    // Additional methods for better functionality
    public String getAddressSummary() {
        return String.format("%s, %s, %s, %s, %s", addressLine1, addressLine2, city, state, country);
    }

    // Implementing equals() and hashCode() for better object comparison
    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Address address = (Address) o;
        return pinCode == address.pinCode &&
                Objects.equals(addressLine1, address.addressLine1) &&
                Objects.equals(addressLine2, address.addressLine2) &&
                Objects.equals(city, address.city) &&
                Objects.equals(state, address.state) &&
                Objects.equals(country, address.country) &&
                Objects.equals(phone, address.phone);
    }

    @Override
    public int hashCode() {
        return Objects.hash(addressLine1, addressLine2, city, state, country, pinCode, phone);
    }

    // Additional methods as needed

    @Override
    public String toString() {
        return "Address{" +
                "addressLine1='" + addressLine1 + '\'' +
                ", addressLine2='" + addressLine2 + '\'' +
                ", city='" + city + '\'' +
                ", state='" + state + '\'' +
                ", country='" + country + '\'' +
                ", pinCode=" + pinCode +
                ", phone='" + phone + '\'' +
                '}';
    }
}

By removing the setter methods, the class becomes immutable, which has several advantages, including improved thread safety and reasoning about the state of objects.

Feel free to incorporate this suggestion and reach out if you have any questions or need further clarification.

@BruceHawk BruceHawk linked a pull request Jul 17, 2024 that will close this issue
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant