Apakah ini praktik yang baik untuk menghindari konstanta dengan menggunakan getter?

25

Apakah ini praktik yang baik untuk mengganti konstanta yang digunakan di luar kelas oleh getter?

Sebagai contoh, apakah lebih baik menggunakan if User.getRole().getCode() == Role.CODE_ADMINatau if User.getRole().isCodeAdmin()?

Itu akan mengarah ke kelas ini:

class Role {
    constant CODE_ADMIN = "admin"
    constant CODE_USER = "user"

    private code

    getRoleCode() {
       return Role.code
    }

    isCodeAdmin () {
       return Role.code == Role.CODE_ADMIN
    }

    isCodeUser () {
       return Role.code == Role.CODE_USER
    }
}
pergi ke
sumber
15
Saya lebih suka menggunakan sesuatu seperti User.HasRole(Role.Admin).
CodesInChaos
4
Periksa prinsip Tell don't Ask .
Andy
Saya mempertanyakan premis: User.getRole().getCode()sudah merupakan pembacaan yang tidak menyenangkan, membandingkan Kode dengan Peran membuatnya lebih canggung lagi.
msw

Jawaban:

47

Pertama, harap dicatat bahwa melakukan sesuatu seperti entity.underlyingEntity.underlyingEntity.method()dianggap sebagai bau kode menurut Hukum Demeter . Dengan cara ini, Anda memaparkan banyak detail implementasi kepada konsumen. Dan setiap kebutuhan ekstensi atau modifikasi sistem seperti itu akan sangat menyakitkan.

Karena itu, saya sarankan Anda untuk memiliki HasRoleatau IsAdminmetode pada Userper CodesInChaos komentar. Dengan cara ini, cara bagaimana peran diterapkan pada pengguna tetap detail implementasi untuk konsumen. Dan juga rasanya lebih alami untuk bertanya kepada pengguna apa perannya alih-alih bertanya kepadanya tentang detail perannya dan kemudian memutuskan berdasarkan itu.


Harap hindari juga penggunaan stringkecuali jika diperlukan. nameadalah contoh stringvariabel yang baik karena isinya tidak diketahui sebelumnya. Di sisi lain, sesuatu seperti di rolemana Anda memiliki dua nilai berbeda yang terkenal pada waktu kompilasi, Anda sebaiknya menggunakan pengetikan yang kuat. Di situlah jenis enumerasi berperan ...

Membandingkan

public bool HasRole(string role)

dengan

public enum Role { Admin, User }

public bool HasRole(Role role)

Kasus kedua memberi saya lebih banyak ide tentang apa yang harus saya sampaikan. Itu juga mencegah saya dari keliru memasukkan tidak valid stringkalau-kalau saya tidak tahu tentang konstanta peran Anda.


Berikutnya adalah keputusan tentang bagaimana peran akan terlihat. Anda bisa menggunakan enum yang disimpan langsung di pengguna:

public enum Role
{
    Admin,
    User
}

public class User
{
    private Role _role;

    public bool HasRole(Role role)
    {
        return _role == role;
    }

    // or
    public bool IsAdmin()
    {
        return _role == Role.Admin;
    }
}

Di sisi lain, jika Anda ingin peran Anda memiliki perilaku itu sendiri, itu pasti harus menyembunyikan rincian bagaimana jenisnya diputuskan:

public enum RoleType
{
    User,
    Admin
}

public class Role
{
    private RoleType _roleType;

    public bool IsAdmin()
    {
        return _roleType == RoleType.Admin;
    }

    public bool IsUser()
    {
        return _roleType == RoleType.User;
    }

    // more role-specific logic...
}

public class User
{
    private Role _role;

    public bool IsAdmin()
    {
        return _role.IsAdmin();
    }

    public bool IsUser()
    {
        return _role.IsUser();
    }
}

Namun ini sangat verbose dan kompleksitas akan meningkat dengan setiap penambahan peran - itu biasanya bagaimana kode berakhir ketika Anda mencoba untuk sepenuhnya mematuhi Hukum Demeter. Anda harus meningkatkan desain, berdasarkan persyaratan nyata sistem yang dimodelkan.

Menurut pertanyaan Anda, saya kira Anda sebaiknya pergi dengan opsi pertama dengan enum langsung User. Jika Anda membutuhkan lebih banyak logika pada Role, opsi kedua harus dianggap sebagai titik awal.

Zdeněk Jelínek
sumber
10
Saya berharap ini adalah cara orang melakukannya di mana-mana. Memiliki pengambil pada atribut privatr hanya demi memeriksa apakah itu sama dengan sesuatu yang lain adalah praktik yang mengerikan.
Andy
2
Re "instance.property.property.method () ..." Bukankah itu cairannya ?
Peter Mortensen
2
@PeterMortensen Ini berbeda dari antarmuka yang lancar. Antarmuka yang lancar pada tipe Xtentu akan memungkinkan Anda untuk membuat serangkaian panggilan fungsi seperti X.foo().bar().fee().... Dalam antarmuka yang lancar ini, foo, bar, dan biaya semuanya akan menjadi fungsi di dalam kelas yang Xmengembalikan objek bertipe X. Tetapi untuk 'instance.property.property.method () yang disebutkan dalam contoh ini, dua propertypanggilan sebenarnya akan berada di kelas yang terpisah. Masalahnya adalah Anda melewati beberapa lapis abstraksi untuk mendapatkan detail tingkat rendah.
Shaz
10
Jawaban yang bagus, tetapi Hukum Demeter bukan latihan menghitung titik. instance.property.property.method()belum tentu merupakan pelanggaran atau bahkan bau kode; itu tergantung pada apakah Anda bekerja dengan objek atau struktur data. Node.Parent.RightChild.Remove()mungkin bukan pelanggaran LoD (walaupun bau karena alasan lain). var role = User.Role; var roleCode = role.Code; var isAdmin = roleCode == ADMIN;hampir pasti merupakan pelanggaran, meskipun pada kenyataannya saya selalu "hanya menggunakan satu titik".
Carl Leth
1
Saya mengerti bahwa instance.property.property.method()ini merupakan pelanggaran terhadap LoD, tetapi OP memiliki instance.method().method()yang seharusnya baik-baik saja. Dalam contoh terakhir Anda, ada begitu banyak kode boilerplate Useryang hanya berfungsi sebagai fasad untuk Role.
Bergi
9

Tampaknya bodoh memiliki fungsi untuk memeriksa apakah kode yang disimpan adalah kode admin. Yang benar-benar ingin Anda ketahui adalah apakah orang itu adalah admin. Jadi jika Anda tidak ingin mengekspos konstanta, maka Anda juga tidak boleh mengekspos bahwa ada kode, dan memanggil metode isAdmin () dan isUser ().

Yang mengatakan, "jika User.getRole (). GetCode () == Peran.CODE_ADMIN" benar-benar segelintir hanya untuk memeriksa apakah pengguna adalah admin. Berapa banyak hal yang harus diingat pengembang untuk menulis baris itu? Dia harus ingat bahwa pengguna memiliki peran, peran memiliki kode, dan kelas Peran memiliki konstanta untuk kode. Itu banyak informasi yang murni tentang implementasinya.

gnasher729
sumber
3
Ini lebih buruk: Bahwa pengguna selalu memiliki satu peran, tidak lebih dan tidak kurang.
Deduplicator
5

Selain yang telah diposkan orang lain, Anda harus ingat bahwa menggunakan konstanta secara langsung memiliki kelemahan lain: jika ada perubahan cara Anda menangani hak pengguna, semua tempat itu perlu diubah juga.

Dan itu membuatnya mengerikan untuk ditingkatkan. Mungkin Anda ingin memiliki tipe pengguna super pada satu titik, yang jelas juga memiliki hak admin. Dengan enkapsulasi, ini pada dasarnya adalah satu baris untuk ditambahkan.

Tidak hanya pendek dan bersih, tetapi juga mudah digunakan dan dimengerti. Dan - mungkin yang terpenting - sulit untuk salah.

Eiko
sumber
2

Sementara saya sebagian besar setuju dengan saran untuk menghindari konstanta dan memiliki metode isFoo()dll, satu contoh tandingan mungkin.

Jika ada ratusan konstanta ini, dan panggilannya jarang digunakan, mungkin tidak ada gunanya untuk menulis ratusan metode isConstant1, isConstant2,. Dalam kasus khusus dan tidak biasa ini, menggunakan konstanta adalah masuk akal.

Perhatikan bahwa menggunakan enum atau hasRole()menghindari kebutuhan untuk menulis ratusan metode, jadi ini adalah yang terbaik dari semua yang ada di dunia.

pengguna949300
sumber
2

Saya tidak berpikir salah satu opsi yang Anda berikan pada dasarnya salah.

Saya melihat Anda belum menyarankan satu hal yang saya sebut benar salah: pengkodean kode peran dalam fungsi di luar kelas Peran. Itu adalah:

if (user.getRole().equals("Administrator")) ...

Saya katakan pasti salah. Saya telah melihat program yang melakukan ini dan kemudian mendapatkan kesalahan misterius karena seseorang salah mengeja string. Saya ingat suatu kali seorang programmer menulis "stok" ketika fungsi sedang memeriksa "Saham".

Jika ada 100 peran yang berbeda, saya akan sangat enggan untuk menulis 100 fungsi untuk memeriksa setiap kemungkinan. Anda mungkin akan membuatnya dengan menulis fungsi pertama dan kemudian menyalin dan menempelnya 99 kali, dan seberapa besar Anda ingin bertaruh bahwa dalam salah satu dari 99 salinan itu Anda akan lupa untuk memperbarui tes atau Anda akan mendapatkannya dengan satu ketika Anda berlari melalui daftar, jadi sekarang Anda miliki

public bool isActuary() { return code==Role.ACTUARY; }
public bool isAccountant() { return code==Role.ACTUARY; }
... etc ...

Secara pribadi, saya juga menghindari rantai panggilan. Saya lebih suka menulis

if (user.getRole().equals(Role.FOOBATER))

kemudian

if (user.getRole().getRoleCode()==Role.FOOBATER_CODE)

dan pada saat itu mengapa note menulis:

if (user.hasRole(Role.FOOBATER))

Kemudian biarkan kelas Pengguna tahu cara memeriksa peran.

Jay
sumber