HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Improve performance by excluding multiple conditions and List

Submitted by: @import:stackexchange-codereview··
0
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 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.