Czy to zły pomysł, aby stworzyć metodę klasową, która jest przekazywana do zmiennych klasowych?

Oto co mam na myśli:

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

addmoże już uzyskać dostęp do wszystkich potrzebnych zmiennych, ponieważ jest to metoda klasowa, więc czy to zły pomysł? Czy istnieją powody, dla których powinienem lub nie powinienem tego robić?

Z klasą mogą być rzeczy, które zrobiłbym inaczej, ale odpowiadając na bezpośrednie pytanie, moja odpowiedź byłaby

tak, to zły pomysł

Moim głównym powodem jest to, że nie masz kontroli nad tym, co jest przekazywane do addfunkcji. Pewnie masz nadzieję, że jest to jedna z tablic składowych, ale co się stanie, jeśli ktoś przejdzie w innej tablicy, która ma mniejszy rozmiar niż 100, lub jeśli przejdziesz przez długość większą niż 100?

Dzieje się tak, ponieważ stworzyłeś możliwość przekroczenia bufora. I to wszystko jest złe.

Aby odpowiedzieć na kilka (dla mnie) oczywistych pytań:

1. Mieszasz tablice w stylu C z C ++. Nie jestem guru C ++, ale wiem, że C ++ ma lepsze (bezpieczniejsze) sposoby obsługi tablic

2. Jeśli klasa ma już zmienne składowe, dlaczego musisz je przekazać? To bardziej pytanie architektoniczne.

Inne osoby z większym doświadczeniem w C ++ (przestałem go używać 10 lub 15 lat temu) mogą mieć bardziej wymowne sposoby wyjaśniania problemów i prawdopodobnie wymyślą również więcej problemów.

Wywołanie metody klasy z niektórymi zmiennymi klasy niekoniecznie jest złe. Ale robienie tego spoza klasy jest bardzo złym pomysłem i sugeruje zasadniczą wadę w twoim projekcie OO, a mianowicie brak odpowiedniej enkapsulacji :

• Każdy kod korzystający z twojej klasy musiałby wiedzieć, że lenjest to długość tablicy i konsekwentnie go używać. Jest to sprzeczne z zasadą najmniejszej wiedzy . Taka zależność od wewnętrznych szczegółów klasy jest wyjątkowo podatna na błędy i ryzykowna.
• Utrudniłoby to ewolucję klasy (np. Gdybyś chciał zmienić starsze tablice i lenna bardziej nowoczesny std::vector<int>), ponieważ wymagałoby to również zmiany całego kodu za pomocą klasy.
• Każda część kodu może siać spustoszenie w twoich MyClassobiektach, niszcząc niektóre publiczne zmienne bez przestrzegania reguł (tzw. Niezmienniki klas )
• Wreszcie metoda jest w rzeczywistości niezależna od klasy, ponieważ działa tylko z parametrami i nie zależy od żadnego innego elementu klasy. Tego rodzaju metoda mogłaby równie dobrze być niezależną funkcją poza klasą. Lub przynajmniej metoda statyczna.

Powinieneś refaktoryzować swój kod i:

• wprowadź zmienne klas privatelub protected, chyba że istnieje dobry powód, aby tego nie robić.
• zaprojektuj metody publiczne jako działania do wykonania na klasie (np object.action(additional parameters for the action).:).
• Jeśli po tym refaktoryzacji nadal będziesz mieć metody klasy, które należy wywołać za pomocą zmiennych klasy, uczyń je chronionymi lub prywatnymi po sprawdzeniu, że są to funkcje narzędziowe obsługujące metody publiczne.

Jeśli celem tego projektu jest chęć ponownego użycia tej metody dla danych, które nie pochodzą z tego wystąpienia klasy, możesz zadeklarować ją jako staticmetodę .

Metoda statyczna nie należy do żadnej konkretnej instancji klasy. Jest to raczej metoda samej klasy. Ponieważ metody statyczne nie są uruchamiane w kontekście żadnej konkretnej instancji klasy, mogą one uzyskiwać dostęp tylko do zmiennych składowych, które są również zadeklarowane jako statyczne. Biorąc pod uwagę, że twoja metoda addnie odwołuje się do żadnej ze zmiennych składowych zadeklarowanych w MyClass, jest dobrym kandydatem do uzyskania deklaracji jako statyczna.

Jednak problemy bezpieczeństwa wymienione w innych odpowiedziach są ważne: Nie sprawdzasz, czy obie tablice są co najmniej lendługie. Jeśli chcesz pisać nowoczesny i solidny C ++, powinieneś unikać używania tablic. W std::vectormiarę możliwości używaj klasy . W przeciwieństwie do zwykłych tablic, wektory znają swój rozmiar. Więc nie musisz sam śledzić ich wielkości. Większość (nie wszystkie!) Ich metod wykonuje również automatyczne sprawdzanie granic, co gwarantuje, że otrzymasz wyjątek, gdy czytasz lub piszesz poza końcem. Regularny dostęp do tablicy nie wykonuje sprawdzania ograniczeń, co w najlepszym wypadku skutkuje segfaultem, a w gorszym przypadku może spowodować podatność na przepełnienie bufora .

Metoda w klasie idealnie manipuluje enkapsulowanymi danymi wewnątrz klasy. W twoim przykładzie nie ma powodu, aby metoda add miała jakiekolwiek parametry, wystarczy użyć właściwości klasy.

Przekazywanie (części) obiektu do funkcji składowej jest w porządku. Wdrażając swoje funkcje, zawsze musisz mieć świadomość możliwego aliasingu i jego konsekwencji.

Oczywiście umowa może pozostawiać niektóre aliasy niezdefiniowane, nawet jeśli język to obsługuje.

Wybitne przykłady:

• Samodzielne przydzielanie. Powinien to być, być może kosztowny, brak możliwości działania. Nie pesymalizuj powszechnego przypadku.
  Z drugiej strony zadanie samodzielnego poruszania się, chociaż nie powinno eksplodować w twoją twarz, nie musi być przeszkodą.
• Wstawianie kopii elementu w kontenerze do kontenera. Coś jak v.push_back(v[2]);.
• std::copy() zakłada, że ​​miejsce docelowe nie zaczyna się w źródle.

Ale twój przykład ma inne problemy:

• Funkcja członka nie korzysta z żadnych swoich uprawnień ani się do niej nie odwołuje this. Działa jak darmowa funkcja użyteczności, która po prostu znajduje się w niewłaściwym miejscu.
• Twoja klasa nie próbuje przedstawiać żadnej abstrakcji . Zgodnie z tym, nie próbuje on zamaskować swoich wnętrzności, ani utrzymywać żadnych niezmienników . To głupia torba z dowolnymi elementami.

Podsumowując: Tak, ten przykład jest zły. Ale nie dlatego, że funkcja członka otrzymuje przekazywane elementy członkowskie.

W szczególności zrobienie tego jest złym pomysłem z kilku dobrych powodów, ale nie jestem pewien, czy wszystkie z nich są integralną częścią twojego pytania:

• Posiadanie zmiennych klas (zmiennych rzeczywistych, a nie stałych) jest czymś, czego w ogóle można uniknąć, i powinno poważnie zastanowić się, czy jest to absolutnie potrzebne.
• Pozostawienie dostępu do zapisu do tych zmiennych klas całkowicie otwartych dla wszystkich jest prawie ogólnie złe.
• Twoja metoda klasowa nie ma związku z twoją klasą. W rzeczywistości jest to funkcja, która dodaje wartości jednej tablicy do wartości innej tablicy. Ten rodzaj funkcji powinien być funkcją w języku, który ma funkcje, i powinien być statyczną metodą „fałszywej klasy” (tj. Zbioru funkcji bez zachowania danych lub obiektów) w językach, które nie mają funkcji.
• Alternatywnie, usuń te parametry i zmień je w metodę prawdziwej klasy, ale nadal byłoby złe z wyżej wymienionych powodów.
• Dlaczego tablice int? vector<int>ułatwiłoby ci życie.
• Jeśli ten przykład jest naprawdę reprezentatywny dla twojego projektu, rozważ umieszczenie danych w obiektach, a nie w klasach, a także zaimplementowanie konstruktorów dla tych obiektów w sposób, który nie wymaga zmiany wartości danych obiektu po utworzeniu.

Jest to klasyczne podejście „C z klasami” do C ++. W rzeczywistości nie tak napisałby każdy doświadczony programista C ++. Po pierwsze, używanie surowych tablic C jest ogólnie odradzane, chyba że implementujesz bibliotekę kontenerów.

Coś takiego byłoby bardziej odpowiednie:

// 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
    }
}