Last week, I presented a drastic change
to RevitLookup contributed by
Andy @awmcc90 McCloskey, RevDev Studios,
using Reflection
to provide more complete coverage of all the Revit database element methods and properties.
Now Victor Chekalin, aka Виктор Чекалин, took a critical look at this new version and cleaned it up significantly to address a number of raw edges in his pull request #25 – old bug fixes and improvements of the new approach .
At first sight, Victor was not at all impressed. Happily, on second thoughts, all is well and order has been restored again:
At first sight, Victor was not pleased at all with the radical changes.
I think this is important to share our discussion on the first impression, since his sentiments are probably shared by other RevitLookup aficionados as well:
I wanted to make some changes in RevitLookup (actually fix the bug) and noticed the bad thing; after Andrew's commit with serious changes, RevitLookup looks ugly. He completely changed the algorithm of data retrieving, using
Reflection
, but lost some functionality. In the latest version, for example, I cannot get symbol geometry. It's disabled. So, the idea is good, bad the implementation needs to be improved :-) And I'll get send you pull request soon with bug fixed.
On second thoughts, luckily, things cleared up a bit:
Probably yesterday I was a bit emotional. Because my first opinion was – what the hell? Everything is absolutely different, not as usual, difficult to find a property, and I could not get the desired property
SymbolGeometry
. I found that the new version is very raw. Looking at it in more detail, I like the general idea to use reflection. Indeed, it allows to get more information and not worry about the new methods/properties in future versions.My biggest concern is with the methods. We are getting the methods using reflection. Andrew gets all the methods without parameters and return type is not void. But we cannot ensure the method is just a 'get' method. A method might modify something and return a value. For example,
bool Remove() { // remove something return true; }
Some issues: The properties/methods are not sorted. In the previous version, they were not sorted either. But as all the properties were added manually, the order looked more intelligent, like
Id
andName
at the top. With reflection, the properties are sorted in the custom order and difficult to find a particular property. I think would be better to sort them. I'll do that, no prob.
SymbolGeometry
is not populated because this is a bug. I've found where exactly in the code –GeometryInstance
is cast toElement
, but is not in fact derived from it. Will try to fix it myself or submit an issue on GitHub.
Here is Victor's subsequent pull request commit summary, followed by a detailed list of the enhancements made to bring the new version of RevitLookup using Reflection
up to par with what we had before:
Public
methodsSymbolGeometry
property for GeometryInstance
Fine
detail levelDetailed pull request description with illustrations:
I changed it to get only public methods, explicitly declared only, and sorted by name.
Before:
After:
Changed behaviour to show enum values.
Before:
After:
Fixed the issue related with GeometryInstance.SymbolGeometry. We could not drill down this property.
Before:
After:
get_
Property Getter MethodRemoved property getter from the methods extraction. Otherwise, for each property, we see the property itself as well as a method like get_SomeProperty
:
Before:
After:
Fixed the very old bug, related with geometry extraction. Whatever DetailLevel
you select, it always showed the geometry for Fine
.
For active view, Medium
DetailLevel
:
Before:
After:
Changed visual style of the separator. Changed colour and shifted the title a bit.
Before:
After:
The most up-to-date version is always provided in the master branch of the RevitLookup GitHub repository.
Victor's bug fixes and enhancements are provided in RevitLookup release 2017.0.0.16 and later versions.
If you would like to access any part of the functionality that was removed when switching to the Reflection
based approach, please grab it
from release 2017.0.0.13 or earlier.
I am also perfectly happy to restore code that was removed and that you would like preserved. Simply create a pull request for that, explain your need and motivation, and I will gladly merge it back again.
Andy responds to the update and answers the question on the methods that might modify something:
The changes look great, and yes, this version is absolutely more raw, but, when all is said and done, I think it will be a lot better.
As far as your concern for modifying the model by executing methods that would modify the model data: this cannot happen, given that we are outside of a transaction. Method such as
Rotate
, etc., will returnfalse
when they cannot execute, which is what you are seeing.This is one thing that I recognised early on as a potential issue but is not a problem – unless there is something I'm completely missing here. Let me know if you find anything to the contrary; otherwise, I still believe this version is safe.
Ever so many thanks again to Andy for his brave initiative and for Victor for his critical and constructive clean-up!