Tenemos este código que, cuando se simplifica, se ve así:
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; }
}
Ahora tenemos tres puntos de vista.
1) Este es un buen código porque la Client
propiedad siempre debe establecerse (es decir, no es nula) para Client == null
que nunca ocurra y el valor de Id 0
denota un Id falso de todos modos (esta es la opinión del escritor del código ;-))
2) No puede confiar en que la persona que llama sabe que 0
es un valor falso para Id
y cuando la Client
propiedad siempre debe establecerse, debe arrojar un exception
en get
cuando la Client
propiedad sea nula
3) Cuando la Client
propiedad siempre debe establecerse, simplemente regresa Client.Id
y deja que el código arroje una NullRef
excepción cuando la Client
propiedad resulta ser nula.
¿Cuál de estos es el más correcto? ¿O hay una cuarta posibilidad?
fuente
Respuestas:
Parece que deberías limitar la cantidad de estados en los que
Room
puede estar tu clase.El hecho mismo de que esté preguntando qué hacer cuando
Client
es nulo es una pista de queRoom
el espacio de estado es demasiado grande.Para simplificar las cosas, no permitiría que la
Client
propiedad de cualquierRoom
instancia sea nula. Eso significa que el código dentroRoom
puede asumir con seguridad queClient
nunca es nulo.Si por alguna razón en el futuro se
Client
vuelvenull
resistir el impulso de apoyar ese estado. Hacerlo aumentará su complejidad de mantenimiento.En su lugar, permita que el código falle y falle rápidamente. Después de todo, este no es un estado compatible. Si la aplicación entra en este estado, ya ha cruzado una línea de no retorno. Lo único razonable que se puede hacer es cerrar la aplicación.
Esto puede suceder (naturalmente) como resultado de una excepción de referencia nula no controlada.
fuente
null
valor con unNullObject
(o más bienNullClient
) que aún funcionaría con su código existente y le permitiría definir el comportamiento de un cliente inexistente si es necesario. La pregunta es si el caso "sin cliente" es parte de la lógica empresarial o no.Solo algunas consideraciones:
a) ¿Por qué hay un getter específicamente para el ClientId cuando ya hay un getter público para la propia instancia del Cliente? No veo por qué la información de que el ClientId es
long
tiene que ser tallada en piedra con la firma de Room.b) Con respecto a la segunda opinión, podría introducir una constante
Invalid_Client_Id
.c) Con respecto a la opinión uno y tres (y siendo mi punto principal): ¿Una habitación siempre debe tener un cliente? Tal vez sea solo semántica, pero esto no suena bien. Quizás sería más apropiado tener clases completamente separadas para Room y Client y otra clase que las vincule. Tal vez cita, reserva, ocupación? (Esto depende de lo que realmente esté haciendo). Y en esa clase puede imponer las restricciones "debe tener una habitación" y "debe tener un cliente".
fuente
No estoy de acuerdo con las tres opiniones. Si
Client
nunca puede sernull
, ¡ entonces ni siquiera lo hagas posiblenull
!Client
en el constructorArgumentNullException
en el constructor.Entonces su código sería algo como:
Esto no está relacionado con su código, pero probablemente sería mejor usar una versión inmutable de
Room
makingtheClient
readonly
, y luego, si el cliente cambia, hacer una nueva habitación. Esto mejorará la seguridad del hilo de su código además de la seguridad nula de los otros aspectos de mi respuesta. Ver esta discusión sobre mutable vs inmutablefuente
ArgumentNullException
s?NullReferenceException
, lo arroja la VM .Net "cuando hay un intento de desreferenciar una referencia de objeto nulo" . Debería lanzar unArgumentNullException
, que se arroja "cuando se pasa una referencia nula (Nothing en Visual Basic) a un método que no lo acepta como argumento válido".readonly
). Habría editado, pero sentí que un comentario serviría para explicar mejor por qué (para futuros lectores). Si aún no hubiera dado esta respuesta, estaría dando exactamente la misma solución. La inmutabilidad también es una buena idea, pero no siempre es tan fácil como uno espera.Deben evitarse las opciones segunda y tercera: el captador no debe golpear a la persona que llama con la excepción de que no tienen control.
Debe decidir si el Cliente puede ser nulo. Si es así, debe proporcionar una forma para que la persona que llama compruebe si es nulo antes de acceder (p. Ej., Propiedad bool ClientIsNull).
Si decide que el Cliente nunca puede ser nulo, conviértalo en un parámetro obligatorio para el constructor y arroje la excepción allí si se pasa un nulo.
Finalmente, la primera opción también contiene un código de olor. Debe dejar que el Cliente se ocupe de su propia propiedad de identificación. Parece excesivo codificar un captador en una clase de contenedor que simplemente llama a un captador en la clase contenida. Simplemente exponga al Cliente como una propiedad (de lo contrario, terminará duplicando todo lo que un Cliente ya ofrece ).
Si el Cliente puede ser nulo, al menos le dará responsabilidad a la persona que llama:
fuente
Si una propiedad de cliente nulo es un estado admitido, considere usar un objeto nulo .
Pero lo más probable es que este sea un estado excepcional, por lo que debe hacer que sea imposible (o simplemente no muy conveniente) terminar con un valor nulo
Client
:Sin embargo, si no es compatible, no pierda el tiempo con soluciones medio horneadas. En este caso:
¡Has 'resuelto' la NullReferenceException aquí pero a un gran costo! Esto obligaría a todas las personas que llaman
Room.ClientId
a verificar 0:Pueden surgir errores extraños con otros desarrolladores (¡o con usted mismo!) Olvidando verificar el valor de retorno de "ErrorCode" en algún momento.
Juega seguro y falla rápido. Permita que se arroje la NullReferenceException y "con gracia" la atrape en algún lugar más arriba de la pila de llamadas en lugar de permitir que la persona que llama arruine las cosas aún más ...
En una nota diferente, si está tan interesado en exponer el
Client
y tambiénClientId
pensar en TellDontAsk . Si le pides demasiado a los objetos en lugar de decirles lo que quieres lograr, puedes terminar de acoplar todo, haciendo que el cambio sea más difícil más adelante.fuente
NotSupportedException
está siendo mal utilizado, deberías estar usando unArgumentNullException
lugar.A partir de c # 6.0, que ahora se publica, solo debe hacer esto,
Elevar una propiedad de
Client
aRoom
es una violación evidente de la encapsulación y el principio DRY.Si necesita acceder al
Id
de unClient
de lasRoom
que puede hacer,Tenga en cuenta el uso del operador condicional nulo ,
clientId
será unalong?
, siClient
esnull
,clientId
seránull
, de lo contrarioclientId
tendrá el valor deClient.Id
.fuente
clientid
es un largo anulable entonces?var clientId = room.Client.Id
será a la vez seguro ylong
.