Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply Frans feedback on the AnimatorPlayer.cs #62

Open
5 tasks
originalnicodr opened this issue Feb 26, 2024 · 0 comments
Open
5 tasks

Apply Frans feedback on the AnimatorPlayer.cs #62

originalnicodr opened this issue Feb 26, 2024 · 0 comments
Labels
animator Anything related to the Animator panel feature refactor

Comments

@originalnicodr
Copy link
Owner

Describe the issue

Frans was kind enough and did some code review on the AnimatorPlayer class. This is what he had to say:

  • It's best to store member fields in one spot and properties in another. I usually define properties at the bottom and member fields at the top of a class. Public fields are frowned upon, hide these with a property, you can simply transform a public field into a property with a hidden compiler generated field using public SomeType FieldName {get;set;}
  • Try to avoid Count() on arrays or collections. Use Length on arrays and Count on collections. Count() will enumerate the entire object if you're not careful. (later .net versions have happy paths for this, but don't count on it in older mono cruft)
  • AddRange() calls often deal with IEnumerable, so you don't need to append .ToList() to a linq query when you're passing the result to an AddRange() method. Adding ToList() will enumerate the entire set, add it to a collection object, then pass that object and it's then enumerated again.. 🙂
  • Types prefixed with I are interfaces. they don't have a concrete implementation. You can use them to implement multiple type inheritance on a single inheritance hierarchy as C# doesn't have multiple implementation inheritance. So try to avoid naming classes with a I prefix 🙂
  • PropertyDescriptors are static objects. This means that you can obtain them once for a type, store them in a dictionary with the name as key and avoid reflecting types again the second time. Reading the value of a property P on an object O is then as simple as pull the property descriptor from the dictionary and call descriptor.GetValue(O). So what I'd do is reflect at startup over types you have to use reflection on, cache these results in dictionaries and after that use these descriptors. Reflecting descriptors is really slow so you want to avoid doing that a lot. If setting the property is slow too (setting a property/getting a property value through descriptors isn't the fastest), you can build an expression tree and compile that lambda at runtime and use that to get/set the values. but that's advanced stuff I'd avoid for now :doggo:
@originalnicodr originalnicodr added refactor animator Anything related to the Animator panel feature labels Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
animator Anything related to the Animator panel feature refactor
Projects
None yet
Development

No branches or pull requests

1 participant