Nous avons ce code qui, une fois simplifié, ressemble à ceci:
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; }
}
Nous avons maintenant trois points de vue.
1) Il s'agit d'un bon code car la Client
propriété doit toujours être définie (c'est-à-dire non nulle), de sorte que la valeur Client == null
ne se produira jamais et que la valeur Id 0
indique un faux Id de toute façon (c'est l'opinion de l'auteur du code ;-))
2) Vous ne pouvez pas compter sur l'appelant pour savoir qu'il 0
s'agit d'une valeur fausse Id
et lorsque la Client
propriété doit toujours être définie, vous devez jeter un exception
dans la get
lorsque la Client
propriété se trouve être nulle.
3) Lorsque la Client
propriété doit toujours être définie, il vous suffit de revenir Client.Id
et de laisser le code lever une NullRef
exception lorsque la Client
propriété est nulle.
Laquelle de ces réponses est la plus correcte? Ou existe-t-il une quatrième possibilité?
la source
Réponses:
Cela sent que vous devriez limiter le nombre d'états dans lesquels votre
Room
classe peut être.Le fait même que vous vous demandiez quoi faire quand
Client
est nul est un indice queRoom
l'espace d'état est trop grand.Pour garder les choses simples, je n'autoriserais jamais la
Client
propriété d'uneRoom
instance à être nulle. Cela signifie que le code qu'il contientRoom
peut sans risque supposer que leClient
n'est jamais nul.Si pour une raison quelconque dans l'avenir
Client
devientnull
résister à l'envie de soutenir cet état. Cela augmentera la complexité de votre maintenance.Au lieu de cela, laissez le code échouer et échouer rapidement. Après tout, ce n'est pas un état pris en charge. Si l'application se met dans cet état, vous avez déjà franchi une ligne de non-retour. La seule chose raisonnable à faire est alors de fermer l'application.
Cela peut se produire (tout naturellement) à la suite d'une exception de référence nulle non gérée.
la source
null
valeur par unNullObject
(ou plutôtNullClient
) qui fonctionnerait alors avec votre code existant et vous permettrait de définir le comportement d'un client inexistant si nécessaire. La question est de savoir si le cas «sans client» fait partie de la logique métier ou non.Quelques considérations:
a) Pourquoi existe-t-il un getter spécifiquement pour ClientId alors qu'il existe déjà un getter public pour l'instance Client elle-même? Je ne vois pas pourquoi les informations sur le ClientId
long
doivent être gravées dans la pierre dans la signature de Room.b) Concernant le deuxième avis, vous pourriez introduire une constante
Invalid_Client_Id
.c) Concernant les opinions un et trois (et étant mon point principal): Une chambre doit toujours avoir un client? C'est peut-être juste de la sémantique mais cela ne sonne pas bien. Il serait peut-être plus approprié d'avoir des classes complètement séparées pour la chambre et le client et une autre classe qui les relie. Peut-être rendez-vous, réservation, occupation? (Cela dépend de ce que vous faites réellement.) Et sur cette classe, vous pouvez appliquer les contraintes "doit avoir une salle" et "doit avoir un client".
la source
Je ne suis pas d'accord avec les trois opinions. Si
Client
cela ne peut jamais arrivernull
, alors ne le permettez même pasnull
!Client
dans le constructeurArgumentNullException
dans le constructeur.Donc, votre code serait quelque chose comme:
Ceci n'est pas lié à votre code, mais vous feriez probablement mieux d'utiliser une version immuable de
Room
en créanttheClient
readonly
, puis si le client change, en créant une nouvelle salle. Cela améliorera la sécurité des threads de votre code en plus de la sécurité nulle des autres aspects de ma réponse. Voir cette discussion sur mutable vs immuablela source
ArgumentNullException
s?NullReferenceException
, il est lancé par la VM .Net "quand il y a une tentative de déréférencer une référence d'objet nul" . Vous devez lancer unArgumentNullException
, qui est levé "lorsqu'une référence null (Nothing en Visual Basic) est passée à une méthode qui ne l'accepte pas comme argument valide".readonly
). J'aurais juste édité mais je sentais qu'un commentaire servirait à mieux expliquer pourquoi (pour les futurs lecteurs). Si vous n'aviez pas déjà donné cette réponse, je donnerais exactement la même solution. L'immuabilité est également une bonne idée, mais ce n'est pas toujours aussi facile qu'on l'espère.Les deuxième et troisième options doivent être évitées - le getter ne doit pas frapper l'appelant à une exception qu'il n'a aucun contrôle.
Vous devez décider si le client peut être nul. Si tel est le cas, vous devez fournir à l'appelant un moyen de vérifier s'il est nul avant d'y accéder (par exemple, la propriété bool ClientIsNull).
Si vous décidez que le client ne peut jamais être nul, faites-en un paramètre obligatoire pour le constructeur et lancez-y l'exception si un null est passé.
Enfin, la première option contient également une odeur de code. Vous devez laisser le client gérer sa propre propriété d'identification. Il semble exagéré de coder un getter dans une classe conteneur qui appelle simplement un getter sur la classe contenue. Exposez simplement le client en tant que propriété (sinon, vous finirez par dupliquer tout ce qu'un client offre déjà ).
Si le client peut être nul, vous allez au moins donner la responsabilité à l'appelant:
la source
Si une propriété Client nulle est un état pris en charge, envisagez d'utiliser un objet NullObject .
Mais très probablement, c'est un état exceptionnel, vous devriez donc rendre impossible (ou tout simplement pas très pratique) de se retrouver avec un null
Client
:S'il n'est pas pris en charge cependant, ne perdez pas de temps sur des solutions à moitié cuites. Dans ce cas:
Vous avez "résolu" l'exception NullReferenceException ici, mais à un coût élevé! Cela forcerait tous les appelants
Room.ClientId
à vérifier 0:Des bogues étranges peuvent survenir avec d'autres développeurs (ou vous-même!) En oubliant de vérifier la valeur de retour "ErrorCode" à un moment donné.
Jouez en toute sécurité et échouez rapidement. Autoriser la levée de l'exception NullReferenceException et l'attraper "gracieusement" quelque part plus haut dans la pile des appels au lieu de permettre à l'appelant de gâcher encore plus les choses ...
Sur une note différente, si vous êtes si désireux d'exposer le
Client
et aussi deClientId
penser à TellDontAsk . Si vous en demandez trop aux objets au lieu de leur dire ce que vous voulez réaliser, vous pouvez mettre fin à tout coupler ensemble, ce qui rendra le changement plus difficile plus tard.la source
NotSupportedException
est mal utilisé, vous devriez plutôt utiliser unArgumentNullException
.À partir de c # 6.0, qui est maintenant disponible, vous devez simplement le faire,
Élever une propriété de
Client
toRoom
est une violation évidente de l'encapsulation et du principe DRY.Si vous devez accéder à l'
Id
unClient
des éléments queRoom
vous pouvez faire,Notez que l'utilisation de l' opérateur conditionnel Null ,
clientId
sera unlong?
, siClient
estnull
,clientId
seranull
, sinonclientId
aura la valeur deClient.Id
.la source
clientid
est un nullable longtemps alors?var clientId = room.Client.Id
sera à la fois sûr etlong
.