Design Pattern Feedback

Recently I created a GitHub project that shows an implementation of a design pattern that I implement frequently. I've never before created a project of such type, and thus would like some constructive feedback.

Instead of creating a constant list of subclasses for a certain type, this project allows me to subclass a single class that automatically handles storing an object of that type. There are more features, of course, such as searching by classmate or a property, a complete separation of searchable types of the 'BaseType' property, and caching of 'Find' and 'FindAll' functions.

Thank you and I look forward to hearing some improvements.
https://github.com/jpgordon00/UniqueTypeRegistry/blob/main/README.md

 

I don't see any practical application of this, and the declaration of type registry is way too verbose.

On other hand, there isn't a whole lot of code in the sample class so this is something that people would be able to write from scratch, if, for a reason I can't fathom they ever needed this.

Additionally, formatting/indents are busted in your code.

Spoiler

^^^ Look at this.

Your editor is mixing tabs and spaces for indents, which is not a good idea, and your editor is also assuming that tab size is 4 spaces. Which is also not a good idea.

 

Unity already has a FindComponent system?

And it's very easy to add a static list of components to a class or add a manager container class for looking after all objects of a class.

If your problem requires more complex data processing of classes then maybe a more data driven approach would be better than a complex and potentially inflexible sub classing OOP hierarchy.

 

Sorry but the repo doesnt seem useful in the slightest, the indendation is all over the place and the actual concept seems obsolete at best? I cant imagine why someone would use this over just writing it themselves (with proper formatting etc), seeing as its basically a very simple concept that you have written in a very syntactically verbose way (way too much to do way too little).

Theres also smarter, faster and less verbose ways of doing what you are trying to do that already exist.

I think you are trying to solve a problem for developers that doesnt exist If you have genuinely had to do this in each project as you said, then I suggest you try and learn some real programming patterns, which may help alieviate the need to use strange methods like this to do simple things

Basically, keep it simple

 

What are the use cases for this? Or, in other words, what kind of problem may I run into as a programmer that this is a solution to?

I'm not suggesting that there aren't any, I'm just having trouble getting from the description of what it does to understanding why I would want to use it.

Is this meant to be so that I can find objects of a certain type without needing to use reflection?

 

Before addressing the use cases for such design pattern, I am open to and would love to hear suggestions to my existing code. What particularly isn't verbose about my technique (specifically the code in the static constructor)? What improvements could be made to make this code better?

Using reflection could replace my technique in the repository, but again, I'm not sure about benefits or drawbacks. I am aware that having to subclass UniqueTypeRegistry.cs is a significant limitation the code. Another limitation I would like to solve would be the requirement for parameter-less object construction.

And again, I'm not sure if my caching technique actually solved anything. To analyze the caching, the function would normally run at O(n) time where n is number of types plus a 'ContainsKey' cache invokation. By caching the function it would run at O(k) time where k is the number of items in the cache. In my opinion, I don't think caching made anything faster.

 

I'm addressing the use of unit tests and the incomplete read me slowly, while I update the repository over time. I apologize for posting an incomplete repository, though I am greatly appreciating all of the feedback.

You are correct, it would never work with classes that have to inherit others and with classes that require parameters.

Is it even worth caching the types? It has been difficult for me to judge if caching would provide any speed benefits over a list lookup that I've been preforming.

 

First you would need to show good examples explaining valid use case, and why your approach is better tha any other.

 

Finally, I'll address the use case for my repository - I'm open to the idea of there being a better solution. To everyone who gave me feedback: thank you, and I welcome more!

I have often found my self writing polymorphic code where I add different subclasses over time. For example, when writing a Mirror server implementation, I wrote classes that subclassed an interface-like-class. These subclasses were brought into a static list and then the current mode was selected by a string from this list.

I used the same design pattern to implement a base class for weapons in a Unity project, this time having the base class subclass UniqueTypeRegistry.cs. I no longer have to store a static (should even exist pre-compilation in headers) list of classes and have to update it for each weapon I add; I now just add new classes and they can be matched by a string.

 

Have you tried using component based approach instead?

A polymorphic behavior can be achieved without actual polymorphism.

 

I believe what I'm trying to achieve is a component based approach, where you can lookup components by string or classname. Could you provide a general programming (not Unity related) example of using a component based system instead of what I described above (or in the examples of my repository)?

 

ECS

 

I don't understand why this is a thing you have to do in the first place. What is that static list used for? Edit: My mistake, you did explain that. Selecting between classes which handle modes identified by strings in a networking library.

Unless I'm misunderstanding something, what you describe is a hierarchical approach as opposed to a component oriented approach. Specifically, you say that you are "add[ing] different subclasses" to support new behaviour. That's not component oriented. If I wanted to make a car and a bus and a motorcycle with this approach I would define "Vehicle" and then have subclasses "Car" and "Bus" and "Motorcycle".

A component oriented approach can define new things without adding subclasses, but instead by attaching components to instances of existing classes. With this approach I would still keep the "Vehicle" class, but I would not make subclasses "Car" and "Bus" and "Motorcycle". Instead, Vehicle would contain a collection of Components. I would implement things such as Chassis and Wheel and Motor as subclasses of Component. I can then make by car by creating a Vehicle instance with a Chassis configured to be mid-sized, a Motor and 4 Wheels. My bus is almost the same but has a much bigger Chassis. The motorcycle has a smaller Chassis and only 2 Wheels.

This pushes much of the heavy lifting out of your object hierarchy and into your data. And then if you want a list of different vehicle types you can just look at that data.

 

I do not understand what you're trying to achieve in the first place and why you use polymorphism. And no, that wouldn't be component based.

For example, in case of user weapons, with component based approach all weapons would use the same class. They would differ by parameters.

For examplee, both pistol and machinegun would likely have the same "Gun" component, but they would have a different ammunition type, different rate of fire, different clip size, different mesh, and different bullet prefab and bullet sound.

Likewise, in a game with cars a "Taxi" and "Heavy Truck" would be a same "Car" component. The object would have different configuration, but it would be the same class.

So you shouldn't ever need to have a list of derived classes in your game. You could need a list of prefabs to spawn. Or a list of scriptable objects that store parameters for something. But there would be no inheritance/polymorphism involved.

 

I think for me the main concern is along with what @angrypenguin and @neginfinity say, you are essentially fighting against or circumventing the engine instead of working with it. There is already stuff in place to achieve what you want, but in a more sensible and logical fashion, and that is through the use of components as has been said.

Where I disagree slightly with what has been said in here, is that "there wouldnt be any inheritance involved", you can still have components that have a derived class if it adds additional functionality to said component that would not make sense to have in the base component (usually this can be configured solely by parameters in the base though without a need for derived as has been said). But again due to the way you can handle things with stuff like generics etc, there shouldnt be a need for a list of derived classes even in that instance where you are having some derived components.

I have never had the issue of saying "I cant get at" or "I cant see what" a derived class is or "I cant access the derived class" and then needing to make a master list, and if that is a problem you are having or the solution that is required; then its time to go back to the drawing board

 

The guy that makes dwarf fortress says his biggest regret he has was using polymorphic classes for items.

It can become a nightmare when you want to mix behaviors and have another subclass that acts kinda like subclass A and subclass B, you end up subclassA, subclassB, subclassC, subclassAB, subclassBC, subclassAC, subclassABC, etc. and probably duplicating code in multiple places becoming a maintenance nightmare.

It's great for things that substitute for each other and override base behavior, like a network transport class that might use sockets on windows or websockets on webgl, or an input class that might be for a mouse, or joystick, or touchscreen, where they all have an x,y axis but go about getting it specific ways.

 

Point of order!

I didn't say that there can't be inheritance used in a component oriented design. I just said it can work without it. It makes a lot of sense to have subclasses of component types in many cases.

Yeah, it's pretty rare that I need to interrogate from one piece of code to see what subclass an object is, and in those cases I always consider how I can avoid needing to do that. Regardless of whether you're using composition or inheritance, if you need to do this a lot then it makes me wonder if there's a broader design issue going on. There may not be, because rare != never, and different domains have different common use cases.

That said, in my experience, if you're following "SOLID" or a similar set of guidelines then objects of subclasses are generally interchangable with one another as far as any client object is concerned. If that's not the case then I usually feel like I've mucked up my design at some earlier point.

For instance, if I have a list of Vehicles in a class then that class shouldn't care that some are cars and others are helicopters and that I might add bikes later. The Vehicle class should provide useful abstractions for whatever I need to do with arbitrary vehicles, or I shouldn't be using a list of Vehicles.

 

With the main points being:

• Defining a set of classes as part of a global list.
• Each defined class in the list is a singleton instance.
• Other classes use the global list to get whatever class instances they need to function.

I think what you were trying to create here is a sort of pseudo dependency-injection system.

If we take one of the examples from the README...

Code (CSharp):

1. //Define a set of global object instances...
2. class Mode : UniqueTypeRegistry {
3.   ...
4. }
5. class PreMode : Mode {
6.   ...
7. }
8. class PostMode : Mode {
9.   ...
10. }
11.  
12. //Get the references to the global object instances...
13. class Server : {
14.   static Server() {
15.     List<Mode> Modes = UniqueTypeRegistry.FindAll<Mode>(); // list of all subclasses of mode
16.    
17.     // find mode by classname
18.     // note you don't need to cast polymorphic return types
19.     PreMode mode = UniqueTypeRegistry.Find<Mode>("PreMode");
20.     PostMode mode = UniqueTypeRegistry.Find<Mode>("PostMode");
21.   }
22. }

...This looks pretty similar to something like a .NET MVC/API application:

Code (CSharp):

1. public interface IPreMode { }
2. public interface IPostMode { }
3.  
4. public class PreMode : IPreMode { }
5. public class PostMode : IPostMode { }
6.  
7. public class Startup
8. {
9.   public void ConfigureServices(IServiceCollection services)
10.   {
11.     //Define a set of global object instances...
12.     services.AddSingleton<IPreMode, PreMode>();
13.     services.AddSingleton<IPostMode, PostMode>();
14.   }
15. }
16.  
17. public class Server
18. {
19.   //Get the references to the global object instances...
20.   public Server(IPreMode preMode, IPostMode postMode)
21.   {
22.  
23.   }
24. }
25.  

 

Hello All,

I have taken feedback from this post and updated my repository. The new repository is here.

I have also created another post similar to this one, located here, where I would like feedback on this repository.

Thank you!