Refactor helper methods: utils class vs command classes

[appieschot]

Discussion started here: #2918 (comment)

We currently have some base methods that provide helper functionality in that base class. We also use the getAppObjectId in the m365 aad app commands. We could pick to have a an abstracted AppCommand class so we can have 1 implementation instead of the multitude we have now (used in app-get, app-role-add, app-role-delete, app-role-list and app-set commands (as well as this one but this might be a bit of an odd duck as this in the future should inherit from whatever we decide in #2961.

Yet discussing that @waldekmastykarz pointed out it makes more sense to revisit and refactor or extend our Utils and provide (grouped) helper methods there. It feels like we should design both how our Utils should work and to research what we could move there for a V1.

[waldekmastykarz]

There was also another thread related to the getAllItems method that we have in some base classes and which is responsible of retrieving all data from paged results. There are some limitations in the current implementation and the suggestion was to change its implementation so that it can be used as a utility rather than something tied to a base class.

The original motivation behind implementing some methods in the command base class was to expose to contributors only what's relevant. That way for example, you're not bothered with things like request digest when building commands based on Microsoft Graph. That said, using a base class is not the only way to implement it and alternatively, we could also consider implement it all such functions in a separate module, like Utils today. To avoid Utils become a catch-all place for adding code and simplify discoverability, we should consider breaking it up in multiple, logically related modules, like validation, spo, etc. If we moved all utils to this one place, it would make it easier to document and reuse them.

As for more specialized methods like getAppObjectId, there is a trade-off between reusability and specialization. If we keep this command in the specific class, we can optimize it for use in just this one command. If we have it in one place, we need to ensure that it will stay generic and work with all commands. On the other hand, if we have it it one place, we could theoretically cut some code and reuse what we already have saving development and maintenance time.

I think breaking up utils and moving functions from base classes to utils is an easy choice. Moving specialized commands like getAppObjectId is something that might require additional investigation/thinking.

[appieschot]

All valid points; for me having a Utils that groups a bunch of helper methods would e a great first step. Like you mention we would tackle a getAllItems with that. And I am pretty sure if we would spent some time figuring out what other things we can move there are other functions that are currently duplicates and thus can be grouped. My 2ct is that is worth investigating a bit what we could move to improve the maintainability.