BMeshUnity

BMeshUnity copied to clipboard

Summary

Change the return type of methods that return List<> to IEnumerable<>.

Motivation

Manipulating the lists that returns from the methods have no effect on the actual BMesh, as such there is really no reason you ever should or would need to add/remove items to the List<>s. It also does not clearly communicate the purpose and use of the returned collection when it is a List<>. Where as IEnumerable<> more clearly communicates the intent behind the method. Returning a List<> does not follow the C# design guidelines. An added benefit of returning IEnumerable<> is that the methods can make uses of yield return for potentially slightly better performance.

Drawbacks

This is a breaking changing if people use any of the List<> methods such as Add() Remove() or Contains(), or for people that declare the variable type when assigning the returned value List<Edge> edges = vertex.NeighborEdges().

Alternatives

One option is to return IReadOnlyList<> instead so people still have access to the indexer and Count property. However this makes it less generic and can no longer use yield return.

I would be happy to make a PR for this change if desired after any further discussion.

Agreed as well!

---

Since we are on the way to breaking changes, it is also time to think about performances a bit. I really wanted to foster ease of use first, but it would be worth thinking about for instance limiting new memory allocation. For instance:

v1.attributes["uv"] = new FloatAttributeValue(0.0f, 0.0f);

is less efficient than

var uv = v1.attributes["uv"].asFloat();
uv.data[0] = 0.0f;
uv.data[1] = 0.0f;

but the latter is a bit annoying to write.

(edit: see #11)