Let’s chew over an opportunity for improvement in the code hygiene

[bertllll]

Fantasies aside, this card being played would likely stir up a chaos in the codebase as long as the works on the big bodies of work like in #676 or maybe even #744 are continuing.

This is a spike card more than anything else. Discussion needed.

Introduction:

While exploring possibilities for addressing newly emerged facts around the PaymentAdjuster, as need for an improvement in the design has been obvious, I noticed something, irritated by signs that spoke for rather chaotic file placements within the project's directories, branched-out, having been being formed long time since the first days, and I had a similar feeling with some of the code structures themselves - raising questions about their right locations, because suggestions about some better ones seemed not be so hard to be made, the contrary, they can feel intuitive.

My long term perceptions became awaken this way. Not blaming anybody, because I know we usually didn't have enough resources for the coverage, I think we’ve always looked away from how good we could potentially do for the code hygiene if we were willing to gave it an extra effort. I like to think of setting some decent, not so bothering, rules that we'd accept to do our best trying to remember them while we are adding new files and even smaller units in and on. This of course sounds awfully abstract but there are some ideas here (not necessarily great ones) to bring it closer to one's mind.

Suggestions:

a)
Beginning with the more yummy bites, you could explore what name, more up-to-date, could be given to the sub_lib library in order to get rid of the obsolete reference to Substratum in it.

b)
Think about making obvious what kind of files should and should not go in the sub_lib library. If there used to be a clear reason that originated the creation of this library and there sure was some, can we review it from the today's perspective that we may hold differently. Let's make sure members of the team understand the purpose of this library if there is a definition that can be uttered and taught.

My take is that we've probably got used to refraining from letting this library grow bigger despite in the days of Substratum, I suppose, there was much less resistance to place new files in there.

c)
Attempt to define simple but systematic requirements for placing Actor messages into files, with focus on the relation to the actors they come to a contact with the most. (I’ve seen a lot of structs of this kind that might feel like suiting better in some alternative places.

One way of looking at it: They lie farther from the originator of the message. Instead I tend to like an idea of a rule, besides thin exceptions (e.g. NodeToUiMessage), that a message should be declared in the space of the actor that creates and dispatches it instead of the one that receives and handles it. I see it as logical and clear as it's more intuitive to imagine an onward trajectory of the message, that drives another advancement in the internal Node's processes, than thinking from the end back frontward. We would find many examples of messages whose placement could collide with this arrangement, naming some:

ReceivedPayments
SentPayables
ReportTransactionReceipts
RetrieveTransactions
QualifiedPayablesMessage
BlockchainAgentWithContextMessage
ReportRoutingServiceProvidedMessage
ReportExitServiceProvidedMessage
ReportServicesConsumedMessage
...

d)
I’ve been challenged many times to choose the right location for adding new components to be added to an actor. I got into there mostly because of the ambiguity that you can see between the mod file (usually serving as the main domain of it) and the files dwelling in the sub_lib, by tradition carrying the same name as the actor itself (the folder keeping it contained respectively).

I'm guessing there isn't really a known and stable definition of what kind of structures should go into one or the other and what misplacement is already considered obviously wrong. It is intriguing to think of rules helping with these considerations with which we would be less at such an easy risk of making mistakes from allowing inconsistency.

Moreover, what if we should not even stick with this simple layout having one file the main one and only one another for keeping two categorically different environments, the logic on one side and the rather function-uncoupled data structures separate from it.

(That idea might have a firm ground underneath, but with actors growing really big, even the auxiliary file unloading the main one, for all those miscellaneous things that necessarily involve in there and become more in their number, is going to turn out insufficient, yet slowly. That is it is again going to reach the issue with that file too large and hard to navigate oneself in). It keeps me believing this architecture is unbearable in a long term.

e)
There is inconsistency even in the way the actor's main-domain file got a name. It is usual to see that this file is the mod.rs file, intended by the Rust philosophy to keep the actor's module together, interconnected. Not sure, if such place is the right one, says me. Still it works quite fine and it can be learned and understood. On the other hand, there are places like in the BlockchainBridge where the main-domain file is not the mod.rs but a file named blockchain_bridge.rs and ranked under the same directory. (Let's remind that the others with the mod.rs locations: Accountant, Hopper, )

f)
Actors like Dispatcher don't have its own domicile, a folder. Looking into the other ones, it feels like a strange exception in the pattern. I believe there might even be multiple files cruising the dispatcher's sphere. It would make the codebase better systematized if simply each actor would be given its own folder in the highest hierarchy of the directory with the Node's source code.

g)
There is sometimes tension about a tight space in one or a few files that should contain together a functionality that is or is getting large enough to deserve its own folder. One example that works well are the scanners. Now the arrangement is based on basically just two files: mod.rs and scanner_utils.rs. However, that's quite disrespectful to how wholesome these three objects are and what components make them up. Each scanner would actually be better becoming if it had its own folder where we could have more space for maneuvering by more files established within each possibly hosting a bordered functionality as a part of the whole single scanner. Now it can hardly be so because the two mentioned files kind of dictates us that all extra code one wants to add to any of these scanners either has to go to the mod.rs, if it is rather a higher lever piece of code, or to be in scanner_utils.rs if it takes smaller and lower-level functions. This isn't much clear and systematic anymore while it certainly may have been so at a time.

[bertllll]

We discussed this with Dan's help and it seems like there might be a consensus in the team members that each would have a will to learn new principles of keeping the Actor messages in a folder to be designed for this purpose. We'd end up with files whose names point out a scope of functionality within Node that they take part in and belong to.

An illustration showing how we could lay those files into the possibly renamed sub_lib.

[masqrauder]

Other suggestions for shared_lib -> core_lib, common_lib
Structure suggestions to avoid repetition:

shared_lib/
    message/
        actor/
             payment/
             configuration/
             data
        ui/
            command/
    type/
        configuration/
            bootstrap/
            runtime/

I don't feel strongly about these suggestions but it does avoid the use of underscores which I think is a win!