I have a "shoppingcart" object and I want to add a "product" object to this shoppingcart, which will result in a "shoppingcartproduct" object.
Adding something to something else (in the non-mathematical sense) doesn't produce a new thing, it alters the existing thing (shopping cart) to now contain the thing (product) that it didn't contain before.
I suspect you may be thinking that a "shoppingcartproduct" should be created because it's the entry of the cross table between the shopping cart and product tables in the database. You are correct that (based on most webshop implementations) your database should be structured like that and thus create a new entry in the cross table when adding a product, but your business logic should be separate from your data persistence.
Your code is crossing a lot of boundaries that it shouldn't. You have your persistence (= data), business and endpoint logic all mashed into the same controller method, which is too much responsibility for one class (let alone method!).
This is making it hard to advocate for actual good practice given the lack of clean architecture. If I were handed your codebase, I would first dedicate my time towards properly separating the persistence, business and web logic into separate project.
But I'll let the rest of the answer focus on the question you've asked. Just keep in mind that when I present a solution, that it is not the final solution that cleans up your code, it's just an example of the specific issue your question focuses on.
If you have the time and effort, I strongly suggest doing an official (or at least lengthier) course on the basics of OOP. It will pay back dividends in the long run. This answer is only battlefield triage compared to what a course or tutorial can provide for you.
Don't get me wrong, I see the effort you're putting in and I applaud it. You're just struggling to get it right. Don't let my criticism get you down. You're on the path, just not at the finish yet. But in order to get things right, you must first understand the things you're doing wrong, hence my answer focusing on the mistakes you've made.
"Bundling"
As you can see I've now chosen the approach of bundling my methods by the name of the object that the method works with the most (usually the return object, but not always). This at first seemed logic, but I'm ending up taking a lot of time finding the methods when debugging.
There are a few approaches how to solve this:
- My current method: bundling by name of most used object.
- Bundling by name of the return object.
- Not bundling at all; make one method per (static)class and use a naming scheme for the class that represents the action it performs. Like
GetProductTask.Run
, AddProductToShoppingTask.Run
ect.
- ???
The fact that you refer to it as "bundling" suggests that you are conceptually still thinking of these as separate things, but you are now begrudgingly putting them together because of a good practice guideline you learned.
Doubly so because you are struggling to find the methods after you've bundled them, which suggests either that (a) your bundling is bad or (b) you're not comfortable with the clean structure that you've created. And if (b) is true, then I'm going to say that it's very likely that you didn't bundle things perfectly either.
Naming
Your question is on naming the things you've created, but the underlying issue is actually that what you're creating isn't quite right. So the main focus here shouldn't be on naming this thing, but rather on improving it.
Names should be descriptive, not prescriptive. By that I mean that it's more important for you to first conceptualize what the class' responsibility and interface is, before you decide what its name should be.
In order for your naming to be intuitive, you need to first have a good picture of the thing you're going to name, because otherwise the name will not reflect what it actually is.
Therefore, this answer mostly focuses on fixing the structuring of your classes, before delving into how to naming well-made classes.
Global scope
make one method per (static)class
I urge you to step away from static classes. What you've done so far (based on the capitalized names, since you didn't post any concrete classes) seems to be making all your logic static, which is not the way to go.
Because you've done this, it feels to me like you've originally written one big monolith method, and then started arbitrarily breaking it up and shoving it into arbitrarily decided static classes, just to conform to the letter of the clean coding guideline without really observing the spirit.
Static classes should be few and far between as far as your main codebase is concerned. C# is an object-oriented language, and you should leverage the benefits of OOP.
There are many things to improve from this example, but at the most rudimentary level, your code needs to instantiate its services, e.g.:
var productService = new ProductService();
var product = productService.GetById(productId);
Keep in mind that it's very uncommon for services to be instantiated on the line above them being used, but this is a massive topic in and of itself. This example is a barebones example meant to showcase how OOP is different from static methods.
Objects vs entities
- My current method: bundling by name of most used object.
- Bundling by name of the return object.
You keep referring to "objects", but you clearly seem to focus on your entities. It's very important that you understand the difference here, because it's (unsurprisingly) the core of object-oriented programming.
Entities are your data objects, i.e. what you store in the database. In your case: User
, ShoppingCart
, Product
, ShoppingCartProduct
. Objects are any instance of a class. Looking back at the previous code example, productService
is an object. Your entities are object (when instantiated), but so are many other things.
It's important to stop thinking of your logic as existing solely in service of your entity. Like you said yourself:
"separate your methods from models"
Part of that separation means that your logic needs to live its own life, and be defined by its own worth.
It's actually hard to pick are your specific rules 1 and 2, because they are technically correct in a lot of cases, but you're thinking of the wrong reasons. We don't call ProductService
that name because it's "the Product
logic bundle". We call it that because it is a service, and since your code will have many different services, we point out that this service is the service that focuses on product-related logic.
The difference here is that -Service
is the most important part of the name, and Product-
is just a subcategory to distinguish it from other services.
I advocated proper layer separation earlier. I want to bring that back up, to show you how the naming tends to work:
ProductController
: A controller handles the web requests, in this case product-related web requests.
ProductService
: A service acts as the main source of business logic, in this case product-related business logic.
ProductRepository
: A repository acts as the codewise representation of the (usually external to the runtime) data storage. In this case, it focuses on storing and retrieving products.
Notice how my description barely touched on the fact that a Product
is involved with these classes. The main focus of the description is what it does.
To use a real world example, your manager isn't called your manager because that's what superiors are called. He's called a manager because he manages his staff. Their job name is based on their job purpose, not what they manage.
Separated chunks of logic
- Not bundling at all; make one method per (static)class and use a naming scheme for the class that represents the action it performs. Like
GetProductTask.Run
, AddProductToShoppingTask.Run
ect.
While I strongly suggest you familiarize yourself with OOP before delving in to this, the approach you're hinting at here suggests that you'd be interested to learn about a combination of CQRS and the mediator pattern.
General advice
This answer cannot cover every question you have, as you have significant gaps in several key concepts of modern day OOP practices.
But, on the bright side, you do seem to be motivated and willing to learn, and that is more than half of the battle is already won.
Based on where you are now, this is what I suggest would be the most efficient course of action to bring you up to speed on modern day OOP clean coding practices:
- Take a refresher course/tutorial on OOP. Focus on understanding names and concepts, to avoid misunderstandings in the future bullet points.
- Read up on Clean Architecture. Learn how/why code needs to be separated into different layers.
- When you understand layer separation, delve into inversion of control, dependency injection, and dependency injection frameworks. In most enterprise-grade codebases, DI containers are the glue that holds the layers together (and they act not too dissimilar from what you expect your "factories" to behave like).
- You should now have a basic understanding of software architecture and how to separate things. From this point on, you just need to continuously look at improving your approaches whenever you see a problem (or when you have time/effort to spare). Focus on SOLID first, it covers many (but not all) of the common issues with OOP implementations.