Como implementar uma propriedade na classe A que se refere a uma propriedade de um objeto filho da classe A

9

Temos este código que, quando simplificado, fica assim:

public class Room
{
    public Client Client { get; set; }

    public long ClientId
    {
        get
        {
            return Client == null ? 0 : Client.Id;
        }
    }
}

public class Client 
{
    public long Id { get; set; }
}

Agora temos três pontos de vista.

1) Este é um bom código porque a Clientpropriedade deve sempre ser configurada (ou seja, não nula), para Client == nullque nunca ocorra e o valor do Id 0denote um ID falso de qualquer maneira (essa é a opinião do autor do código ;-))

2) Você não pode confiar no chamador para saber que 0é um valor falso para Ide quando a Clientpropriedade sempre deve ser definida, você deve lançar um exceptionno getquando a Clientpropriedade for nula

3) Quando a Clientpropriedade sempre deve ser definida, basta retornar Client.Ide deixar o código lançar uma NullRefexceção quando a Clientpropriedade for nula.

Qual destes está mais correto? Ou existe uma quarta possibilidade?

Michel
fonte
5
Não é uma resposta, mas apenas uma observação: nunca lance exceções de getters de propriedades.
Brandon
3
Para elaborar o ponto de Brandon: stackoverflow.com/a/1488488/569777
MetaFight
11
Claro: a idéia geral ( msdn.microsoft.com/en-us/library/… , stackoverflow.com/questions/1488472/… , entre outros) é que as propriedades façam o mínimo possível, sejam leves e representem a limpeza , estado público do seu objeto, com indexadores sendo uma pequena exceção. Também é um comportamento inesperado ver exceções em uma janela de inspeção de uma propriedade durante a depuração.
Brandon
11
Você não deve (precisa) escrever o código que lança um getter de propriedade. Isso não significa que você deve ocultar e engolir quaisquer exceções que ocorram devido a um objeto entrar em um estado não suportado.
Ben Ben
4
Por que você não pode simplesmente fazer o Room.Client.Id? Por que você deseja agrupar isso no Room.ClientId? Se é para procurar um cliente, por que não algo como Room.HasClient {get {return client == null; }} isso é mais direto e ainda permite que as pessoas façam o Room.Client.Id normal quando realmente precisam do ID?
James

Respostas:

25

Parece que você deve limitar o número de estados em que sua Roomclasse pode estar.

O fato de você estar perguntando sobre o que fazer quando Clienté nulo é uma dica de que Roomo espaço de estado é muito grande.

Para simplificar, não permitiria que a Clientpropriedade de nenhuma Roominstância fosse nula. Isso significa que o código dentro Roompode assumir com segurança que Clientnunca é nulo.

Se por algum motivo no futuro Clienttorna-se nullresistir ao desejo de apoiar esse estado. Fazer isso aumentará sua complexidade de manutenção.

Em vez disso, permita que o código falhe e falhe rapidamente. Afinal, este não é um estado suportado. Se o aplicativo entrar nesse estado, você já cruzou uma linha sem retorno. A única coisa razoável a fazer é fechar o aplicativo.

Isso pode acontecer (naturalmente) como resultado de uma exceção de referência nula não tratada.

MetaFight
fonte
2
Agradável. Gosto do pensamento "Para simplificar as coisas, não permitiria que a propriedade Client de qualquer instância da Sala fosse nula." Isso é realmente melhor do que questionar o que fazer quando é nulo
Michel
@ Michel, se você decidir posteriormente alterar esse requisito, poderá substituir o nullvalor por um NullObject(ou melhor NullClient) que ainda funcionaria com o código existente e permitir definir o comportamento para um cliente inexistente, se necessário. A questão é se o caso "sem cliente" faz parte da lógica de negócios ou não.
Nulo
3
Totalmente correto. Não escreva um único caractere de código para suportar um estado não suportado. Apenas deixe jogar um NullRef.
Ben Ben
@Ben Discordo; As diretrizes da Microsoft declaram explicitamente que os getters de propriedade não devem lançar exceções. E permitir que referências nulas sejam lançadas quando uma exceção mais significativa pode ser lançada é apenas preguiçoso.
Andy
11
@A compreensão do motivo da diretriz permitirá que você saiba quando ela não se aplica.
Ben
21

Apenas algumas considerações:

a) Por que existe um getter específico para o ClientId quando já existe um getter público para a própria instância do Client? Não vejo por que as informações do ClientId precisam longser gravadas em pedra na assinatura do Room.

b) Em relação à segunda opinião, você pode introduzir uma constante Invalid_Client_Id.

c) Em relação à opinião um e três (e sendo meu ponto principal): Uma sala deve sempre ter um cliente? Talvez seja apenas semântica, mas isso não parece certo. Talvez seja mais apropriado ter classes completamente separadas para Room e Client e outra classe que as una. Talvez nomeação, reserva, ocupação? (Isso depende do que você está realmente fazendo.) E nessa classe, você pode impor as restrições "deve ter uma sala" e "deve ter um cliente".

VolkerK
fonte
5
+1 para "Eu não vejo por exemplo, por que a informação que o ClientId é uma longa tem de ser esculpida em pedra para a assinatura do Quarto"
Michel
Seu ingles é bom. ;) Só de ler esta resposta, eu não saberia que sua língua nativa não é o inglês sem você dizer isso.
precisa saber é o seguinte
Os pontos B e C são bons; RE: a, seu método de conveniência, eo fato de que a sala tem uma referência ao cliente pode ser apenas um detalhe de implementação (pode-se argumentar que o Cliente não deve ser visível publicamente)
Andy
17

Não concordo com as três opiniões. Se Clientnunca pode ser null, nem torne possível que sejanull !

  1. Defina o valor de Clientno construtor
  2. Jogue um ArgumentNullExceptionno construtor.

Portanto, seu código seria algo como:

public class Room
{
    private Client theClient;

    public Room(Client client) {
        if(client == null) throw new ArgumentNullException();
        this.theClient = client;
    }

    public Client Client { 
        get { return theClient; }
        set 
        {
            if (value == null) 
                throw new ArgumentNullException();
            theClient = value;
        }
    }

    public long ClientId
    {
        get
        {
            return theClient.Id;
        }
    }
}
public class Client 
{
    public long Id { get; set; }
}

Isso não tem relação com o seu código, mas você provavelmente usaria uma versão imutável de Roommaking theClient readonly, e se o cliente mudar, criando uma nova sala. Isso melhorará a segurança do encadeamento do seu código, além da segurança nula dos outros aspectos da minha resposta. Veja esta discussão sobre mutável vs imutável

durron597
fonte
11
Estes não deveriam ser ArgumentNullExceptions?
Mathieu Guindon
4
Não. O código do programa nunca deve gerar a NullReferenceException, é lançado pela .Net VM "quando houver uma tentativa de desreferenciar uma referência de objeto nulo" . Você deve lançar um ArgumentNullException, que é lançado "quando uma referência nula (Nada no Visual Basic) é passada para um método que não o aceita como argumento válido".
Pharap
2
@Pharap Eu sou um programador Java tentando escrever código C #, achei que cometeria erros assim.
durron597
@ durron597 Eu deveria ter adivinhado que, pelo uso do termo 'final' (neste caso, a palavra-chave C # correspondente é readonly). Eu teria acabado de editar, mas senti que um comentário serviria para explicar melhor o porquê (para futuros leitores). Se você ainda não tivesse dado essa resposta, eu daria exatamente a mesma solução. Imutabilidade também é uma boa ideia, mas nem sempre é tão fácil quanto se espera.
Pharap
2
As propriedades geralmente não devem gerar exceções; em vez de um setter que lança ArgumentNullException, forneça um método SetClient. Alguém postou o link para uma resposta sobre isso na pergunta.
Andy
5

As segunda e terceira opções devem ser evitadas - o atendente não deve bater no chamador com uma exceção sobre a qual eles não têm controle.

Você deve decidir se o Cliente pode ser nulo. Nesse caso, você deve fornecer uma maneira para um chamador verificar se é nulo antes de acessá-lo (por exemplo, propriedade booleana ClientIsNull).

Se você decidir que o Cliente nunca pode ser nulo, torne-o um parâmetro obrigatório para o construtor e lance a exceção se um nulo for passado.

Finalmente, a primeira opção também contém um cheiro de código. Você deve deixar o Cliente lidar com sua própria propriedade de ID. Parece um exagero codificar um getter em uma classe de contêiner que simplesmente chama um getter na classe contida. Apenas exponha o cliente como uma propriedade (caso contrário, você duplicará tudo o que o cliente já oferece ).

long clientId = room.Client.Id;

Se o Cliente puder ser nulo, você pelo menos dará responsabilidade ao chamador:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}
Kent A.
fonte
11
Eu acho que "você vai acabar duplicando tudo o que um cliente já oferece" precisa estar em negrito ou pelo menos em itálico.
Pharap
2

Se uma propriedade nula do cliente for um estado suportado, considere usar um NullObject .

Mas, provavelmente, esse é um estado excepcional, portanto, você deve tornar impossível (ou não muito conveniente) acabar com um valor nulo Client:

public Client Client { get; private set; }

public Room(Client client)
{
    Client = client;
}

public void SetClient(Client client)
{
    if (client == null) throw new ArgumentNullException();
    Client = client;
}

Se não for suportado, no entanto, não perca tempo com soluções meio cozidas. Nesse caso:

return Client == null ? 0 : Client.Id;

Você 'resolveu' a NullReferenceException aqui, mas a um grande custo! Isso forçaria todos os chamadores de Room.ClientIda verificarem 0:

var aRoom = new Room(aClient);
var clientId = aRoom.ClientId;
if (clientId == 0) RestartRoombookingWizard();
else ContinueRoombooking(clientid);

Bugs estranhos podem surgir com outros desenvolvedores (ou você mesmo!) Esquecendo de verificar o valor de retorno "ErrorCode" em algum momento.
Jogue pelo seguro e falhe rápido. Permita que a NullReferenceException seja lançada e "graciosamente" a pegue em algum lugar mais alto na pilha de chamadas, em vez de permitir que o chamador estrague ainda mais as coisas ...

Em outra nota, se você deseja expor o Cliente também ClientIdpensar sobre o TellDontAsk . Se você pedir demais aos objetos, em vez de dizer a eles o que deseja alcançar, poderá acabar acoplando tudo, dificultando as mudanças mais tarde.

Laoujin
fonte
Isso NotSupportedExceptionestá sendo usado de maneira incorreta; você deve usar um ArgumentNullException.
Pharap
1

A partir do c # 6.0, agora lançado, você deve fazer isso,

public class Room
{
    public Room(Client client)
    {
        this.Client = client;
    }

    public Client Client { get; }
}

public class Client
{
    public Client(long id)
    {
        this.Id = id;
    }

    public long Id { get; }
}

Criar uma propriedade de Clientto Roomé uma violação óbvia do encapsulamento e do princípio DRY.

Se você precisar acessar o Idde um Clientde um, Roomvocê pode fazer,

var clientId = room.Client?.Id;

Observe o uso do operador condicional nulo , clientIdserá um long?, se Clienté null, clientIdserá null, caso contrário clientIdterá o valor de Client.Id.

Jodrell
fonte
mas o tipo de clientidé um longo anulável, então?
Michel Michel
@ Michel bem, se uma sala precisar ter um cliente, faça uma verificação nula no ctor. Então var clientId = room.Client.Idserá seguro e long.
Jodrell