Apakah itu ide yang buruk telah membuat metode kelas yang lulus variabel kelas?

14

Inilah yang saya maksud:

class MyClass {
    int arr1[100];
    int arr2[100];
    int len = 100;

    void add(int* x1, int* x2, int size) {
        for (int i = 0; i < size; i++) {
            x1[i] += x2[i];
        }
    }
};

int main() {
    MyClass myInstance;

    // Fill the arrays...

    myInstance.add(myInstance.arr1, myInstance.arr2, myInstance.len);
}

addsudah dapat mengakses semua variabel yang dibutuhkan, karena ini adalah metode kelas, jadi apakah ini ide yang buruk? Adakah alasan mengapa saya harus atau tidak melakukan ini?

tukang kertas
sumber
13
Jika Anda merasa ingin melakukan ini, itu mengisyaratkan desain yang buruk. Properti kelas mungkin milik tempat lain.
bitsoflogic
11
Cuplikan terlalu kecil. Level abstraksi campuran ini tidak terjadi dalam cuplikan ukuran ini. Anda juga dapat memperbaikinya dengan beberapa teks yang bagus tentang pertimbangan desain.
Yosua
16
Jujur saya tidak mengerti mengapa Anda bahkan melakukan ini. Apa yang kamu dapat? Mengapa tidak hanya memiliki metode no-arg addyang beroperasi secara internal? Kenapa?
Ian Kemp
2
@IanKemp Atau punya addmetode yang mengambil argumen tetapi tidak ada sebagai bagian dari kelas. Hanya fungsi murni untuk menambahkan dua array bersamaan.
Kevin
Apakah metode add selalu menambahkan arr1 dan arr2, atau dapatkah itu digunakan untuk menambahkan sesuatu ke sesuatu?
user253751

Jawaban:

33

Mungkin ada beberapa hal dengan kelas yang akan saya lakukan secara berbeda, tetapi untuk menjawab pertanyaan langsung, jawaban saya adalah

ya, itu ide yang buruk

Alasan utama saya untuk ini adalah bahwa Anda tidak memiliki kendali atas apa yang diteruskan ke addfungsi. Tentu Anda berharap ini adalah salah satu array anggota, tetapi apa yang terjadi jika seseorang melewati dalam array yang berbeda yang memiliki ukuran lebih kecil dari 100, atau Anda memberikan panjang lebih dari 100?

Apa yang terjadi adalah Anda telah menciptakan kemungkinan buffer overrun. Dan itu adalah hal buruk di sekitar.

Untuk menjawab beberapa pertanyaan (bagi saya) yang jelas:

  1. Anda mencampur array gaya C dengan C ++. Saya bukan guru C ++, tetapi saya tahu bahwa C ++ memiliki cara penanganan array yang lebih baik (lebih aman)

  2. Jika kelas sudah memiliki variabel anggota, mengapa Anda harus meneruskannya? Ini lebih merupakan pertanyaan arsitektur.

Orang lain yang memiliki lebih banyak pengalaman C ++ (saya berhenti menggunakannya 10 atau 15 tahun yang lalu) mungkin memiliki cara yang lebih fasih dalam menjelaskan masalah, dan mungkin akan menghasilkan lebih banyak masalah juga.

Peter M
sumber
Memang, vector<int>akan lebih cocok; panjang kemudian tidak akan berguna. Atau const int len, karena array panjang variabel bukan bagian dari standar C ++ (walaupun beberapa kompiler mendukungnya, karena ini adalah fitur opsional di C).
Christophe
5
@Christophe Author menggunakan array ukuran tetap, yang harus diganti dengan std::arrayyang tidak membusuk ketika mengirimkannya ke parameter, memiliki cara yang lebih mudah untuk mendapatkan ukurannya, dapat disalin dan memiliki akses dengan batas pemeriksaan opsional, sementara tidak memiliki overhead lebih dari Array gaya C.
Cubic
1
@Cubic: setiap kali saya melihat kode yang mendeklarasikan array dengan ukuran tetap yang sewenang-wenang (seperti 100), saya cukup yakin penulisnya adalah seorang pemula yang tidak tahu tentang implikasi dari omong kosong ini, dan itu adalah ide yang baik untuk merekomendasikan dia mengubah desain ini menjadi sesuatu dengan ukuran variabel.
Doc Brown
Array baik-baik saja.
Lightness Races with Monica
... untuk mereka yang tidak mengerti apa yang saya maksud, lihat juga Zero One Infinity Rule
Doc Brown
48

Memanggil metode kelas dengan beberapa variabel kelas belum tentu buruk. Tetapi melakukannya dari luar kelas adalah ide yang sangat buruk dan menyarankan cacat mendasar dalam desain OO Anda, yaitu tidak adanya enkapsulasi yang tepat :

  • Kode apa pun yang menggunakan kelas Anda harus tahu bahwa itu lenadalah panjang dari array, dan menggunakannya secara konsisten. Ini bertentangan dengan prinsip ilmu yang paling sedikit . Ketergantungan seperti itu pada detail bagian dalam kelas sangat rawan kesalahan dan berisiko.
  • Ini akan membuat evolusi kelas menjadi sangat sulit (mis. Jika suatu hari, Anda ingin mengubah array legacy dan lenke yang lebih modernstd::vector<int> ), karena itu mengharuskan Anda untuk juga mengubah semua kode menggunakan kelas Anda.
  • Bagian mana pun dari kode dapat menimbulkan kerusakan pada MyClassobjek Anda dengan merusak beberapa variabel publik tanpa mematuhi aturan (disebut invarian kelas )
  • Akhirnya, metode ini sebenarnya independen dari kelas, karena hanya berfungsi dengan parameter dan tidak bergantung pada elemen kelas lainnya. Metode semacam ini bisa menjadi fungsi independen di luar kelas. Atau setidaknya metode statis.

Anda harus memperbaiki kode Anda, dan:

  • buat variabel kelas Anda privateatauprotected , kecuali ada alasan bagus untuk tidak melakukannya.
  • rancang metode publik Anda sebagai tindakan yang akan dilakukan di kelas (misalnya: object.action(additional parameters for the action) .
  • Jika setelah refactoring ini, Anda masih memiliki beberapa metode kelas yang perlu dipanggil dengan variabel kelas, menjadikannya dilindungi atau pribadi setelah memverifikasi bahwa mereka adalah fungsi utilitas yang mendukung metode publik.
Christophe
sumber
7

Ketika maksud dari desain ini adalah bahwa Anda ingin dapat menggunakan kembali metode ini untuk data yang tidak berasal dari instance kelas ini, maka Anda mungkin ingin mendeklarasikannya sebagai staticmetode .

Metode statis bukan milik instance partikel dari kelas. Ini lebih merupakan metode kelas itu sendiri. Karena metode statis tidak dijalankan dalam konteks instance kelas apa pun, mereka hanya dapat mengakses variabel anggota yang juga dinyatakan sebagai statis. Menimbang bahwa metode addAnda tidak merujuk ke variabel anggota mana pun yang dideklarasikanMyClass , itu adalah kandidat yang baik untuk dinyatakan sebagai statis.

Namun, masalah keamanan yang disebutkan oleh jawaban lain valid: Anda tidak memeriksa apakah kedua array setidaknya len panjang. Jika Anda ingin menulis C ++ yang modern dan kuat, maka Anda harus menghindari penggunaan array. Gunakan kelas std::vectorsebagai gantinya bila memungkinkan. Berlawanan dengan array reguler, vektor tahu ukurannya sendiri. Jadi, Anda tidak perlu melacak ukurannya sendiri. Sebagian besar (tidak semua!) Metode mereka juga melakukan pengecekan terikat otomatis, yang menjamin bahwa Anda mendapatkan pengecualian ketika Anda membaca atau menulis melewati akhir. Akses array reguler tidak melakukan pengecekan terikat, yang menghasilkan segfault yang terbaik dan dapat menyebabkan kerentanan buffer overflow yang dapat dieksploitasi lebih buruk.

Philipp
sumber
1

Sebuah metode di kelas idealnya memanipulasi data yang dienkapsulasi di dalam kelas. Dalam contoh Anda, tidak ada alasan untuk metode add untuk memiliki parameter apa pun, cukup gunakan properti kelas.

Rik D
sumber
0

Melewati (bagian dari) objek ke fungsi anggota tidak masalah. Saat menerapkan fungsi Anda, Anda harus selalu waspada terhadap kemungkinan alias , dan konsekuensinya.

Tentu saja, kontrak mungkin meninggalkan beberapa jenis aliasing tidak terdefinisi bahkan jika bahasa mendukungnya.

Contoh yang menonjol:

  • Penugasan diri. Ini harus menjadi, mungkin mahal, no-op. Jangan pesimis kasus umum untuk itu.
    Penugasan-gerak-sendiri di sisi lain, meskipun seharusnya tidak meledak di wajah Anda, tidak perlu menjadi larangan.
  • Memasukkan salinan elemen dalam wadah ke dalam wadah. Sesuatu sepertiv.push_back(v[2]); .
  • std::copy() mengasumsikan tujuan tidak dimulai di dalam sumber.

Tetapi contoh Anda memiliki masalah yang berbeda:

  • Fungsi anggota tidak menggunakan hak istimewanya, atau merujuk this . Kerjanya seperti fungsi utilitas gratis yang hanya di tempat yang salah.
  • Kelas Anda tidak mencoba menampilkan abstraksi apa pun . Sejalan dengan itu, ia tidak mencoba untuk merangkum jeroan, atau mempertahankan invarian . Ini adalah tas bodoh dari unsur-unsur yang sewenang-wenang.

Singkatnya: Ya, contoh ini buruk. Tetapi bukan karena fungsi anggota dilewatkan anggota-objek.

Deduplicator
sumber
0

Secara khusus melakukan ini adalah ide yang buruk karena beberapa alasan bagus, tetapi saya tidak yakin semuanya merupakan bagian integral dari pertanyaan Anda:

  • Memiliki variabel kelas (variabel aktual, bukan konstanta) adalah sesuatu yang harus dihindari jika mungkin, dan harus memicu refleksi serius pada apakah Anda benar-benar membutuhkannya.
  • Membiarkan akses tulis ke variabel kelas ini benar-benar terbuka untuk semua orang hampir secara universal buruk.
  • Metode kelas Anda pada akhirnya tidak berhubungan dengan kelas Anda. Apa itu sebenarnya adalah fungsi yang menambahkan nilai-nilai dari satu array ke nilai-nilai array lain. Fungsi semacam ini harus merupakan fungsi dalam bahasa yang memiliki fungsi, dan harus menjadi metode statis dari "kelas palsu" (yaitu kumpulan fungsi tanpa data atau perilaku objek) dalam bahasa yang tidak memiliki fungsi.
  • Atau, hapus parameter ini dan mengubahnya menjadi metode kelas nyata, tetapi akan tetap buruk karena alasan yang disebutkan sebelumnya.
  • Mengapa array int? vector<int>akan membuat hidup Anda lebih mudah.
  • Jika contoh ini benar-benar mewakili desain Anda, pertimbangkan untuk meletakkan data Anda di objek dan bukan di kelas dan juga mengimplementasikan konstruktor untuk objek ini dengan cara yang tidak memerlukan perubahan nilai data objek setelah pembuatan.
Kafein
sumber
0

Ini adalah pendekatan "C dengan kelas" klasik ke C ++. Pada kenyataannya, ini bukan apa yang akan ditulis oleh programmer C ++ berpengalaman. Pertama, menggunakan array C mentah cukup banyak dicegah secara universal, kecuali jika Anda menerapkan perpustakaan kontainer.

Sesuatu seperti ini akan lebih sesuai:

// Don't forget to compile with -std=c++17
#include <iostream>
using std::cout; // This style is less common, but I prefer it
using std::endl; // I'm sure it'll incite some strongly opinionated comments

#include <array>
using std::array;

#include <algorithm>

#include <vector>
using std::vector;


class MyClass {
public: // Begin lazy for brevity; don't do this
    std::array<int, 5> arr1 = {1, 2, 3, 4, 5};
    std::array<int, 5> arr2 = {10, 10, 10, 10, 10};
};

void elementwise_add_assign(
    std::array<int, 5>& lhs,
    const std::array<int, 5>& rhs
) {
    std::transform(
        lhs.begin(), lhs.end(), rhs.begin(),
        lhs.begin(),
        [](auto& l, const auto& r) -> int {
            return l + r;
        });
}

int main() {
    MyClass foo{};

    elementwise_add_assign(foo.arr1, foo.arr2);

    for(auto const& value: foo.arr1) {
        cout << value << endl; // arr1 values have been added to
    }

    for(auto const& value: foo.arr2) {
        cout << value << endl; // arr2 values remain unaltered
    }
}
Alexander - Pasang kembali Monica
sumber