patterncsharpMinor
Improve performance by excluding multiple conditions and List
Viewed 0 times
excludingimproveperformancemultipleconditionsandlist
Problem
This method works correctly and verifies whether a user has permissions to view or edit the first parameter (value). I think it is not the best implementation, because there are too many conditions and the
List object isn't necessary. How to improve it?protected override Exception GetValidationException( object value, string name )
{
List entities = value is int ? new List( 1 ) { EntityCache.Entity.GetEntity( (int) value ) } :
value is Guid ? new List( 1 ) { EntityCache.Entity.GetEntityByGuid( (Guid) value ) } :
value is int[] ? (value as int[]).Select( i => EntityCache.Entity.GetEntity( i ) ).ToList() :
value is Entity ? new List( 1 ) { value as Entity } :
value is Entity[] ? (value as Entity[]).ToList() :
null;
if( entities == null )
throw new ArgumentException( string.Format( RI.UnknownArgumentType, name, value, value.GetType().Name ) );
bool hasPermission = (from one in entities
select EntityPermissionCache.PermissionChecker( one.Id, one.EntityOrganizationId ) &&
_checkEditInsteadOfView ? EntityTypePermissionCache.PermissionChecker( true, one.TypeId ) : true)
.All( a => a );
return hasPermission ? null : new AccessViolationException( RI.AccessDenied );
}Solution
I think the assignment of your
That said, my first thought was why on Earth would you want to return an
Is it valid for a function to return exception objects?
Absolutely. Here is a short list of examples where this may be appropriate:
Fine then. But there's a flaw in your API - you're taking in an
If I understand what you're trying to do here, I believe you'd be better off with an overloaded method:
...Or I'd find a way to make something like this work:
entities (bad name) is uber-abusing ternary expressions.That said, my first thought was why on Earth would you want to return an
Exception? So I googled up a bit and found this:Is it valid for a function to return exception objects?
Absolutely. Here is a short list of examples where this may be appropriate:
- An exception factory.
- A context object that reports if there was a previous error as a ready to use exception.
- A function that keeps a previously caught exception.
- A third-party API that creates an exception of an inner type.
Fine then. But there's a flaw in your API - you're taking in an
Object for what seems to be an Entity.If I understand what you're trying to do here, I believe you'd be better off with an overloaded method:
protected override Exception GetValidationException(int value, string name)
protected override Exception GetValidationException(Guid value, string name)
protected override Exception GetValidationException(int[] value, string name)
protected override Exception GetValidationException(Entity value, string name)
protected override Exception GetValidationException(Entity[] value, string name)...Or I'd find a way to make something like this work:
protected override Exception GetValidationException(TValue value, string name)Code Snippets
protected override Exception GetValidationException(int value, string name)
protected override Exception GetValidationException(Guid value, string name)
protected override Exception GetValidationException(int[] value, string name)
protected override Exception GetValidationException(Entity value, string name)
protected override Exception GetValidationException(Entity[] value, string name)protected override Exception GetValidationException<TValue>(TValue value, string name)Context
StackExchange Code Review Q#31354, answer score: 2
Revisions (0)
No revisions yet.