Skip to main content
Added emphasis, as suggested by a commenter
Source Link
Kent A.
  • 880
  • 4
  • 9

The second and third options should be avoided - the getter should not smack the caller with an exception they have no control over.

You should decide whether Client can ever be null. If so, you should provide a way for a caller to check whether it is null before accessing it (e.g., bool ClientIsNull property).

If you decide that Client can never be null, then make it a required parameter to the constructor and throw the exception there if a null is passed in.

Finally, the first option also contains a code smell. You should let Client deal with its own ID property. It seems like overkill to code a getter in a container class that simply calls a getter on the contained class. Just expose the Client as a property (otherwise, you'll end up duplicating everything a client already offersyou'll end up duplicating everything a Client already offers).

long clientId = room.Client.Id;

If Client can be null, then you'll at least give responsibility to the caller:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}

The second and third options should be avoided - the getter should not smack the caller with an exception they have no control over.

You should decide whether Client can ever be null. If so, you should provide a way for a caller to check whether it is null before accessing it (e.g., bool ClientIsNull property).

If you decide that Client can never be null, then make it a required parameter to the constructor and throw the exception there if a null is passed in.

Finally, the first option also contains a code smell. You should let Client deal with its own ID property. It seems like overkill to code a getter in a container class that simply calls a getter on the contained class. Just expose the Client as a property (otherwise, you'll end up duplicating everything a client already offers).

long clientId = room.Client.Id;

If Client can be null, then you'll at least give responsibility to the caller:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}

The second and third options should be avoided - the getter should not smack the caller with an exception they have no control over.

You should decide whether Client can ever be null. If so, you should provide a way for a caller to check whether it is null before accessing it (e.g., bool ClientIsNull property).

If you decide that Client can never be null, then make it a required parameter to the constructor and throw the exception there if a null is passed in.

Finally, the first option also contains a code smell. You should let Client deal with its own ID property. It seems like overkill to code a getter in a container class that simply calls a getter on the contained class. Just expose the Client as a property (otherwise, you'll end up duplicating everything a Client already offers).

long clientId = room.Client.Id;

If Client can be null, then you'll at least give responsibility to the caller:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}
added 38 characters in body
Source Link
Kent A.
  • 880
  • 4
  • 9

The second and third options should be avoided - the getter should not smack the caller with an exception they have no control over.

You should decide whether Client can ever be null. If so, you should provide a way for a caller to check whether it is null before accessing it (e.g., bool ClientIsNull property).

If you decide that Client can never be null, then make it a required parameter to the constructor and throw the exception there if a null is passed in.

Finally, you might want tothe first option also contains a code smell. You should let Client deal with its own ID property. It seems like overkill to code a getter in a container class that simply calls a getter on the contained class. Just expose the Client as a property (otherwise, you'll end up duplicating everything a client already offers).

long clientId = room.Client.Id;

If Client can be null, then you'll at least give responsibility to the caller:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}

The second and third options should be avoided - the getter should not smack the caller with an exception they have no control over.

You should decide whether Client can ever be null. If so, you should provide a way for a caller to check whether it is null before accessing it (e.g., bool ClientIsNull property).

If you decide that Client can never be null, then make it a required parameter to the constructor and throw the exception there if a null is passed in.

Finally, you might want to let Client deal with its own ID property. It seems like overkill to code a getter in a container class that simply calls a getter on the contained class. Just expose the Client as a property (otherwise, you'll end up duplicating everything a client already offers).

long clientId = room.Client.Id;

If Client can be null, then you'll at least give responsibility to the caller:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}

The second and third options should be avoided - the getter should not smack the caller with an exception they have no control over.

You should decide whether Client can ever be null. If so, you should provide a way for a caller to check whether it is null before accessing it (e.g., bool ClientIsNull property).

If you decide that Client can never be null, then make it a required parameter to the constructor and throw the exception there if a null is passed in.

Finally, the first option also contains a code smell. You should let Client deal with its own ID property. It seems like overkill to code a getter in a container class that simply calls a getter on the contained class. Just expose the Client as a property (otherwise, you'll end up duplicating everything a client already offers).

long clientId = room.Client.Id;

If Client can be null, then you'll at least give responsibility to the caller:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}
Source Link
Kent A.
  • 880
  • 4
  • 9

The second and third options should be avoided - the getter should not smack the caller with an exception they have no control over.

You should decide whether Client can ever be null. If so, you should provide a way for a caller to check whether it is null before accessing it (e.g., bool ClientIsNull property).

If you decide that Client can never be null, then make it a required parameter to the constructor and throw the exception there if a null is passed in.

Finally, you might want to let Client deal with its own ID property. It seems like overkill to code a getter in a container class that simply calls a getter on the contained class. Just expose the Client as a property (otherwise, you'll end up duplicating everything a client already offers).

long clientId = room.Client.Id;

If Client can be null, then you'll at least give responsibility to the caller:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}