Is it a good practice to create a new class every time a method is invoked? I would guess, we should declare classes outside, construct objects in methods and return the instances. What is the reason behind taking your approach?

In D3248#87324, @karol-bisztyga wrote:

I'm not sure about this. I looked around a bit and have not found any information about this. Basically, local classes can be used and I've not seen any big issues with it.

Even when this is a valid language construction, we need to check if it should be used in this context. And currently, it looks like there are no benefits of using this approach compared to an alternative approach.
Currently we have four template methods, with two type parameters and one argument. In each of these methods, we create a local class that uses template type parameters when determining its parent class. None of the classes use function parameter context. So at this point, each of inner classes could be moved outside the function and all the code would compile and work the same.

So at the moment, defining the classes locally has only one advantage: we don't allow anyone to instantiate them outside the function - if that was the intention, then we can consider keeping this approach.

Local classes can be extremely useful when their definition depends on function argument. Currently it's not the case, but maybe this is going to change. What is the plan for these? (even if the code depends on the argument, it could as well be passed to the constructor, though)

To summarize, could you explain why local classes were used in this case? There exist some valid reasons, but at this point this code could be simplified to use normal classes, so I'm wondering what is the plan here.

I prefer this as is. I assume you prefer the opposite.

It's not that I have a strong preference for one solution or another. But I do have preference to use the default approach unless there are good reasons to use something different.

the fact that these classes cannot be used outside of these functions is one thing

Agree, this is a valid point

Another is that we have the implementations in one place

Also a good point. But if we created normal classes in this file, it would be also in one place.

For me, it's easier to read

This surprised me. For me it's a lot worse to read. We have a couple of levels of additional indentation - and our experience from CommCoreModule suggests that this is a real issue.
If a local class is short, then the readability would be ok, but here it looks like the opposite will be true.

Please note that these classes contain just one method with the logic, maybe some of them will have some simple state.

That makes sense. We can't use static members inside them but simple state would be ok.

I find them better where they are, it indicates explicitly that they're supposed to be used only in this one specific place, plus we have a code with the logic directly inside of these functions.

Why should they be used only here? Is there something which makes them unusable in other places?

I guess the fastest option to move forward here is to get an opinion of someone else and proceed with the outcome.

I'm not going to block you on this as it looks like you think your approach is better here. I only wanted to hear why is that.

Requesting changes to clarify core dumps issue