É uma má ideia fazer um método de classe que seja passado a variáveis de classe?

14

Veja o que quero dizer:

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

add já pode acessar todas as variáveis de que precisa, já que é um método de classe, então é uma má ideia? Existem razões pelas quais devo ou não fazer isso?

    
por swimingduck 09.12.2018 / 18:10
fonte

7 respostas

33

Pode haver coisas com a classe que eu faria diferente, mas para responder a pergunta direta, minha resposta seria

sim, é uma má ideia

Meu principal motivo para isso é que você não tem controle sobre o que é passado para a função add . Claro que você espera que seja um dos arrays de membros, mas o que acontece se alguém passar em um array diferente que tenha um tamanho menor que 100, ou você passar um comprimento maior que 100?

O que acontece é que você criou a possibilidade de um buffer saturado. E isso é uma coisa ruim ao redor.

Para responder mais algumas perguntas óbvias:

  1. Você está misturando matrizes de estilo C com C ++. Eu não sou nenhum guru C ++, mas eu faço saiba que o C ++ tem formas melhores (mais seguras) de lidar com matrizes

  2. Se o classe já tem as variáveis de membro, por que você precisa passá-las em? Esta é mais uma questão arquitetônica.

Outras pessoas com mais experiência em C ++ (parei de usá-lo há 10 ou 15 anos) podem ter formas mais eloqüentes de explicar os problemas e, provavelmente, apresentarão mais problemas também.

    
por 09.12.2018 / 18:55
fonte
48

Chamar um método de classe com algumas variáveis de classe não é necessariamente ruim. Mas fazê-lo de fora da classe é uma idéia muito ruim e sugere uma falha fundamental em seu design OO, ou seja, a ausência de um bom encapsulation :

  • Qualquer código que usa sua classe precisaria saber que len é o tamanho do array e usá-lo consistentemente. Isso vai contra o princípio do menor conhecimento . Tal dependência nos detalhes internos da classe é extremamente propensa a erros e arriscada.
  • Isso tornaria a evolução da classe muito difícil (por exemplo, se um dia você quiser alterar as matrizes herdadas e len para uma std::vector<int> mais moderna), pois isso exigiria que você também alterasse todas as código usando sua classe.
  • Qualquer parte do código pode causar estragos em seus objetos MyClass corrompendo algumas variáveis públicas sem respeitar as regras (chamadas de invariants de classe )
  • Finalmente, o método é, na realidade, independente da classe, pois funciona apenas com os parâmetros e não depende de nenhum outro elemento de classe. Esse tipo de método poderia muito bem ser uma função independente fora da classe. Ou pelo menos um método estático.

Você deve refatorar seu código e:

  • faça suas variáveis de classe private ou protected , a menos que haja uma boa razão para não fazer isso.
  • projete seus métodos públicos como ações a serem executadas na turma (por exemplo, object.action(additional parameters for the action) ).
  • Se, após essa refatoração, você ainda tiver alguns métodos de classe que precisem ser chamados com variáveis de classe, torne-os protegidos ou privados depois de verificar se são funções de utilitário que suportam os métodos públicos.
por 09.12.2018 / 19:54
fonte
7

Quando a intenção desse design é que você queira reutilizar esse método para dados que não são provenientes dessa instância de classe, convém declará-lo como static method .

Um método estático não pertence a nenhuma instância particular de uma classe. É um método da própria classe. Como os métodos estáticos não são executados no contexto de nenhuma instância específica da classe, eles só podem acessar variáveis de membros que também são declaradas como estáticas. Considerando que seu método add não se refere a nenhuma das variáveis de membro declaradas em MyClass , é um bom candidato para ser declarado como estático.

No entanto, os problemas de segurança mencionados pelas outras respostas são válidos: Você não está verificando se os dois arrays são pelo menos len long. Se você quiser escrever C ++ moderno e robusto, evite o uso de matrizes. Use a classe std::vector em vez disso, sempre que possível. Ao contrário dos arrays regulares, os vetores conhecem seu próprio tamanho. Então você não precisa acompanhar o tamanho deles mesmos. A maioria (não todos!) De seus métodos também faz a verificação automática de limites, o que garante que você receba uma exceção quando ler ou escrever após o final. O acesso regular à matriz não faz a verificação vinculada, o que resulta em um segfault na melhor das hipóteses e pode causar uma vulnerabilidade de estouro de buffer explorável em pior.

    
por 10.12.2018 / 12:58
fonte
1

Um método em uma classe idealmente manipula dados encapsulados dentro da classe. No seu exemplo, não há motivos para o método add ter parâmetros, basta usar as propriedades da classe.

    
por 09.12.2018 / 18:55
fonte
0

Passar (parte de) um objeto para uma função-membro é bom. Ao implementar suas funções, você sempre tem que estar ciente do possível aliasing e das conseqüências disso.

É claro que o contrato pode deixar alguns tipos de aliasing indefinidos, mesmo que o idioma o suporte.

Exemplos proeminentes:

  • Auto-designação. Este deve ser um, possivelmente caro, não operacional. Não pessimize o caso comum para isso.
    Por outro lado, a atribuição de movimento automático, embora não deva explodir na sua cara, não precisa ser um não operacional.
  • Inserindo uma cópia de um elemento em um contêiner no contêiner. Algo como v.push_back(v[2]); .
  • std::copy() assume que o destino não começa dentro da origem.

Mas seu exemplo tem problemas diferentes:

  • A função-membro não usa nenhum privilégio, nem se refere a this . Ele age como uma função de utilidade gratuita que está simplesmente no lugar errado.
  • Sua classe não tenta apresentar nenhum tipo de abstração . De acordo com isso, ele não tenta encapsular suas entranhas, nem manter quaisquer invariantes . É um saco idiota de elementos arbitrários.

Em resumo: Sim, este exemplo é ruim. Mas não porque a função-membro é passada pelos objetos-membro.

    
por 10.12.2018 / 13:14
fonte
0

Especificamente, fazer isso é uma má idéia por vários bons motivos, mas não tenho certeza se todos eles são essenciais para sua pergunta:

  • Ter variáveis de classe (variáveis reais, não constantes) é algo a ser evitado, se possível, e deve desencadear uma reflexão séria sobre se você realmente precisa dela.
  • Deixar o acesso de gravação a essas variáveis de classe completamente abertas para todos é quase universalmente ruim.
  • Seu método de aula acaba sendo não relacionado à sua turma. O que realmente é é uma função que adiciona os valores de uma matriz aos valores de outra matriz. Esse tipo de função deve ser uma função em uma linguagem que tenha funções e deve ser um método estático de uma "classe falsa" (ou seja, uma coleção de funções sem dados ou comportamento de objetos) em linguagens que não possuem funções. / li>
  • Como alternativa, remova esses parâmetros e transforme-os em um método de classe real, mas isso ainda seria ruim pelos motivos mencionados anteriormente.
  • Por que matrizes int? vector<int> facilitaria sua vida.
  • Se este exemplo for realmente representativo de seu design, considere colocar seus dados em objetos e não em classes e também implementar construtores para esses objetos de uma maneira que não exija a alteração do valor dos dados do objeto após a criação.
por 10.12.2018 / 14:17
fonte
0

Esta é uma abordagem clássica de "C com classes" para o C ++. Na realidade, isso não é o que qualquer programador experiente de C ++ escreveria. Por um lado, usar arrays C brutos é praticamente desencorajado, a menos que você esteja implementando uma biblioteca de contêineres.

Algo como isso seria mais apropriado:

// 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
    }
}
    
por 11.12.2018 / 06:08
fonte