Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

It looks rather hacky because I have the feeling there must be someplace down the line where you can more easily determine whether or not the properties are filled in.

Nevertheless: right now your code will only work for strings while it is probably realistic to say that there might be other fields in the future as well (Id, DateOfBirth, etc) which would require a code change. Therefore I would suggest making your code a little less string-specific:

public bool isEmpty()
{
 var properties = this.GetType().GetProperties(BindingFlags.Public | BindingFlags.Instance);
 foreach (var prop in properties)
 {
 var value = prop.GetValue(this, null);
 
 if (value != default(prop.PropertyType))
 {
 return false;
 }
 
 var stringValue = value as string;
 if(stringValue != null)
 {
 if(stringValue != string.IsNullOrWhiteSpace(value))
 {
 return false;
 }
 }
 }
 return true;
}
  • Using default so it words with primitive types (they won't be able to be 0 with this)
  • If you do want to be able to be 0, make them nullable types instead
  • as combined with != null as combined with != null
  • IsNullOrWhiteSpace(string) instead of comparison to empty quotes
  • Removed t since it was only used once

It looks rather hacky because I have the feeling there must be someplace down the line where you can more easily determine whether or not the properties are filled in.

Nevertheless: right now your code will only work for strings while it is probably realistic to say that there might be other fields in the future as well (Id, DateOfBirth, etc) which would require a code change. Therefore I would suggest making your code a little less string-specific:

public bool isEmpty()
{
 var properties = this.GetType().GetProperties(BindingFlags.Public | BindingFlags.Instance);
 foreach (var prop in properties)
 {
 var value = prop.GetValue(this, null);
 
 if (value != default(prop.PropertyType))
 {
 return false;
 }
 
 var stringValue = value as string;
 if(stringValue != null)
 {
 if(stringValue != string.IsNullOrWhiteSpace(value))
 {
 return false;
 }
 }
 }
 return true;
}
  • Using default so it words with primitive types (they won't be able to be 0 with this)
  • If you do want to be able to be 0, make them nullable types instead
  • as combined with != null
  • IsNullOrWhiteSpace(string) instead of comparison to empty quotes
  • Removed t since it was only used once

It looks rather hacky because I have the feeling there must be someplace down the line where you can more easily determine whether or not the properties are filled in.

Nevertheless: right now your code will only work for strings while it is probably realistic to say that there might be other fields in the future as well (Id, DateOfBirth, etc) which would require a code change. Therefore I would suggest making your code a little less string-specific:

public bool isEmpty()
{
 var properties = this.GetType().GetProperties(BindingFlags.Public | BindingFlags.Instance);
 foreach (var prop in properties)
 {
 var value = prop.GetValue(this, null);
 
 if (value != default(prop.PropertyType))
 {
 return false;
 }
 
 var stringValue = value as string;
 if(stringValue != null)
 {
 if(stringValue != string.IsNullOrWhiteSpace(value))
 {
 return false;
 }
 }
 }
 return true;
}
  • Using default so it words with primitive types (they won't be able to be 0 with this)
  • If you do want to be able to be 0, make them nullable types instead
  • as combined with != null
  • IsNullOrWhiteSpace(string) instead of comparison to empty quotes
  • Removed t since it was only used once
Source Link
Jeroen Vannevel
  • 11.6k
  • 2
  • 39
  • 79

It looks rather hacky because I have the feeling there must be someplace down the line where you can more easily determine whether or not the properties are filled in.

Nevertheless: right now your code will only work for strings while it is probably realistic to say that there might be other fields in the future as well (Id, DateOfBirth, etc) which would require a code change. Therefore I would suggest making your code a little less string-specific:

public bool isEmpty()
{
 var properties = this.GetType().GetProperties(BindingFlags.Public | BindingFlags.Instance);
 foreach (var prop in properties)
 {
 var value = prop.GetValue(this, null);
 
 if (value != default(prop.PropertyType))
 {
 return false;
 }
 
 var stringValue = value as string;
 if(stringValue != null)
 {
 if(stringValue != string.IsNullOrWhiteSpace(value))
 {
 return false;
 }
 }
 }
 return true;
}
  • Using default so it words with primitive types (they won't be able to be 0 with this)
  • If you do want to be able to be 0, make them nullable types instead
  • as combined with != null
  • IsNullOrWhiteSpace(string) instead of comparison to empty quotes
  • Removed t since it was only used once
lang-cs

AltStyle によって変換されたページ (->オリジナル) /