How to Distinguish Redundant Rooms

A couple of interesting Revit API issues were resolved during my recent absence.

Let's start with this question raised by Miroslav Schonauer and resolved by Diane Christoforo:

Question: Using the terminology as shown in Schedules, I need to report all 'Not Placed', 'Redundant' and 'Not Enclosed' rooms.

I cannot rely on any particular schedule being present in the model.

So far, my research shows that:

The only remaining issue is how to distinguish between 'Redundant' and 'Not Enclosed' rooms.

Do you have any ideas how can I do this comprehensively and deterministically from the room properties and params?

BTW, I'm aware that an alternative approach would be via the Failure API. I can see, e.g., the enumeration value BuiltInFailures.RoomFailures.RoomNotEnclosed.

After just reading about it and looking into some samples, I worry about this approach and think that it is possible to 'override' the failures, which may prevent me from determining whether the rooms really are Not Enclosed or Redundant.

Here is sample code illustrating the approach I have been able to develop so far:

  /// <summary>
  /// Distinguish 'Not Placed', 'Redundant' and 'Not Enclosed' rooms.
  /// </summary>
  void DistinguishRooms(
    Document doc,
    ref StringBuilder sb,
    ref int numErr,
    ref int numWarn )
    sb = new StringBuilder();
    FilteredElementCollector rooms
      = new FilteredElementCollector( doc );
    rooms.WherePasses( new RoomFilter() );
    foreach( Room r in rooms )
      sb.AppendFormat( "\r\n  Room {0}:'{1}': ",
        r.Id.ToString(), r.Name );
      if( r.Area > 0 ) // OK if having Area
        sb.AppendFormat( "OK (A={0}[ft3])", r.Area );
      else if( null == r.Location ) // Unplaced if no Location
        sb.AppendFormat( "UnPlaced (Location is null)" );
        sb.AppendFormat( "NotEnclosed or Redundant "
          + "- how to distinguish?" );

Any ideas if this is possible?

Answer: Rooms that are redundant still have normal boundaries.

So you can call Room.GetBoundarySegments to test.

The list will be empty for a non-enclosed or unplaced room.

Response: Thank you, Diane!

Here is the final solution implementing your suggestion:

  public enum RoomState
  /// <summary>
  /// Distinguish 'Not Placed',  'Redundant' 
  /// and 'Not Enclosed' rooms.
  /// </summary>
  RoomState DistinguishRoom( Room room )
    RoomState res = RoomState.Unknown;
    if( room.Area > 0 )
      // Placed if having Area
      res = RoomState.Placed;
    else if( null == room.Location )
      // No Area and No Location => Unplaced
      res = RoomState.NotPlaced;
      // must be Redundant or NotEnclosed
      SpatialElementBoundaryOptions opt
        = new SpatialElementBoundaryOptions();
      IList<IList<BoundarySegment>> segs
        = room.GetBoundarySegments( opt );
      res = ( null == segs || segs.Count == 0 )
        ? RoomState.NotEnclosed
        : RoomState.Redundant;
    return res;

Jeremy adds: I added Miro's test methods as DistinguishRoomsDraft and DistinguishRoom to The Building Coder samples release 2016.0.126.8 in the module CmdListAllRooms.cs.

Many thanks to Miro and Diane for the interesting question, research and answer.

I actually cleaned up both the draft and final methods before publishing them, with the two following strong recommendations on performance and exception handling:


Looking at your code, I notice an important inefficiency:

  foreach( Element elRoom in fecRooms.ToElements() )
    Room r = elRoom as Room;

You can save yourself the duplication of the entire list of rooms by eliminating the call to ToElements, and the cast is not needed either:

  foreach( Room r in fecRooms )

I hope this helps... systematically!

I hope you can eliminate a thousand calls to ToElements all over all your projects.

I discussed this and other similar issues many times here in the past, e.g. for FindElement and filtered element collector optimisation and ToElementIds performance.

Exception Handling

I eliminated a catch-all exception handler from Miro's solution.

Are you aware of the strong recommendation against never, ever, catching all exceptions?

Using a catch with no specific exceptions listed is very strongly discouraged, cf. e.g. these articles on C# catching all exceptions and why catch(Exception) and empty catch is bad.

Here is another more extensive discussion on exception handling in C# with the "Do Not Catch Exceptions That You Cannot Handle" rule in mind.