Iterator pattern approach

[GeKtvi]

SolidWorks API allows us to work with some collection, like features and annotations via the Iterator pattern. Currently we have an awkward approach with iterateing collection via callback. It has many restrictions and is not convenient.

Can we try other approaches?
Table of comparison approaches:

Approach	Lazy evaluation	User can dispose when want	User has way to automatically dispose	Understandable for beginners	Compatible with LINQ
Final collection	X	✓	X	✓	✓
Treating with callback	X*	X	✓	X	X
Enumerating	✓	✓	✓	X	✓

* - is possible to implement via return value or Args

I prefer to use Enumerating. Because implementation of Iterator pattern in .NET is IEnumerable. And most pure way is to wrap SW "GetNext" iterator methods to its .NET alternative - IEnumerable. But if we care about object disposing it seems not possible, but it's not.
I saw some similar approach in ReactiveUI. It has "DisposeWith" extensions method that allows to add new observable subscription to one composite disposable, it is awesome idea. I think we can use this approach when iterating through SW collection, and when user ends with it, he disposes input disposable (or use "using"). But it optional and we can don't do this (COM object dispose I mean).

Let me demonstrate example with model’s notes iteration.

Then let me explain.
In "firstNote" we terminate collection iteration when found note with matching Name. Advantage of lazy approach is if it first Note in model it will cost O(1) if last O(N) and if we average results we get ~ O(N/2). It’s just example doesn’t be strict.
"notEmptyNotes" and "extractedDigits" demonstrates how seamlessly transitions into LINQ.
When operating with enumerable is ended we can dispose "disposable" or use "using" that dispose in end of stack frame.

That's what it is, what do you think? Is it the best approach, or we have some better alternatives? Is this idea worth pursuing?

I can try to add PR for Enumerable for Note enumeration, and I think we can try something similar with SelectionManager.GetEnumerateSelected.

[brinkdinges]

I'm not a fan of the existing method of passing a function or action, so I'm open to improvements. I usually write a foreach to get all items in a list, then iterate over them. With solidworks, it's always tricky when modifying the objects in a list, I've had problems when using yield to enumerate items because the item no longer existed or worked. So I'm not too keen to implement that approach everywhere. I'll think about it.

The iterator approach does indeed work well with the GetNext methods. But it isn't trivial to use, with the composite disposable and reusing that object.

I'm not too focused on performance., You rarely have more than 100 items in a list and calling a rebuild is usually 100 times slower than anything we do.

[GeKtvi]

But it isn't trivial to use, with the composite disposable and reusing that object.
For beginners, perhaps - but in .NET, it’s common practice to either consume an IEnumerable immediately or materialize it (e.g., via ToList()/ToArray()) for deferred use.

I’ve tested this pattern in several add-ins, and it works reliably. In some cases, I’ve omitted explicit Dispose() calls, and the GC handles cleanup correctly. I also reviewed another SolidWorks API wrapper library and noticed it doesn’t use Marshal.Release - suggesting it might be unnecessary overhead.

I'm not too focused on performance.
I generally don’t optimize prematurely, but this is one case where it matters. I’ve already encountered performance bottlenecks when processing feature lists, which led me to propose this Issue.

While the iterator approach is more complex than a simple list, it offers significant flexibility and scalability - especially if you’re comfortable with LINQ patterns. It’s a powerful tool when used correctly.

[GeKtvi]

Public API: prefer IEnumerable<T> over List<T>

List is not a great choice when we’re trying to avoid breaking API changes.

You’re designing future problems here and breaking encapsulation with CustomFeatureNames.

Can we stop using List<T>? We can use IEnumerable<T> instead - not as “an enumerable” conceptually, but simply as the public signature/contract
When we use List<T>, we bind the API to a specific collection type, and that’s a really risky approach
In my experience maintaining internal company libraries, this is a good way to create long-term issues and make the library harder to extend later. List<T> is also not a great return type for an API: it’s not optimized for any specific case, and it’s not as lightweight as an array (it has more overhead).

IEnumerable<T> is the most abstract interface for “collection-like” data (its contract is quite simple and every collection implements it), and it should be preferred over List<T> in API signatures, because IEnumerable<T> can be backed by any .NET collection. This gives more tolerance for internal SolidDNA changes. It’s also more mockable since it requires only enumeration, which can help in some test scenarios

• IEnumerable<out T> uses the out keyword, which allows covariance and gives more flexibility to change internal implementation later
• Returned IEnumerable<T> can’t be changed by the user without an explicit cast, unlike List<T> which is directly mutable and can lead to unexpected problems

Let see at his lines:

public List<string> CustomFeatureNames { get; }

public static SelectionType CreateCustomFeatureType(SelectionType baseType, List<string> featureNames) 
    => new SelectionType(baseType, featureNames);

We accept one List instance and store it - that’s very risky. It’s the same instance passed by the user, so they can mutate it, and we don’t expect that. Then we expose it back to the same user and effectively say “mutate it as you wish”, which breaks encapsulation. SelectionType shouldn’t be designed to mutate, it feels like an outdated approach. And what happens if this becomes a bottleneck later - do we add CustomFeatureNames2 as a HashSet and support two properties from now on? That’s not a good direction.

With SolidDNA we return wrappers around SolidWorks COM objects (and those COM objects are themselves wrappers around SolidWorks internal implementation). In that kind of API, returning a mutable collection is usually a bad idea because we’re typically returning a “snapshot” of data. We should use mutable collections only when the API also wraps the underlying SolidWorks collection operations (add, remove, etc.).

What is the alternative? Just use an interface. I’m not a big fan of “read-only” interfaces, so I’d prefer IEnumerable<T>, but other options are fine too - the important part is that it should be effectively immutable from the public API point of view. In the current example:

public IEnumerable<string> CustomFeatureNames { get; } // Expose `IEnumerable<T>` publicly, but store a concrete collection privately (e.g. array/list); simplified here for example

public static SelectionType CreateCustomFeatureType(SelectionType baseType, IEnumerable<string> featureNames) 
    => new SelectionType(baseType, featureNames.ToList().AsReadOnly());

It has benefits: it’s extendable, we can change the internal implementation when we want, and we can use any collection behind the signature. It also lets callers pass arrays/sets/LINQ results without forcing them to allocate a List<T> just to match the signature. It makes it clearer this is read-only “snapshot” data (not something the user should mutate). It also lets us copy/normalize internally (e.g., trim, Distinct, sort) so behavior stays predictable, while still keeping the public contract minimal. If it was something like IEnumerable<out ISomeType>, it adds even more possibilities to change internals without breaking changes.

So if you’re focused on fewer breaking changes, you should use more abstract input/return types. Less public contract is better for extensibility without breaking the API. Using List<T> while aiming for “no breaking changes” is a conflicting approach.

I think the simplest rule is: use List<T> inside the library, but return only IEnumerable<T> to the user. If someone inspects the implementation and casts IEnumerable<T> to List<T>, and we later change the implementation, that’s their problem (but if you prefer, we can also return immutable collections internally to avoid even that).

This isn’t just my opinion there’s a StackOverflow question that addresses this too.

I hope this doesn’t come across as nitpicking - I’m just trying to raise the API-stability concern early.

[brinkdinges]

I agree and I'd like to use IEnumerable for future APIs. Mostly because we can use yield and a user can stop iterating once they find the item they're looking for. That's not always faster because most solidworks APIs return the full array, but it can be useful when traversing features or custom properties. Your point about mutating a list is good as well.

CustomFeatureNames is a messy thing from the past that I don't fully understand yet.

[GeKtvi]

That's not always faster because most solidworks APIs return the full array

Not entirely accurate, COM casting also has a performance cost, though I suspect it's relatively minor.

CustomFeatureNames is a messy thing from the past that I don't fully understand yet.

It's used for selecting specific features like Extrude or Edge-Flange. The existing enum doesn't cover these values. I can explain it in PR.

Regarding CompositeDisposable:
It was suggested as a way to ensure proper disposal of all objects returned by the library. However, if proper disposal isn't needed, it's better to avoid using it - it adds unnecessary overhead.