darwinia-network / legacy-apps

Legacy apps of Darwinia Network
https://legacy-apps.vercel.app
Apache License 2.0
7 stars 3 forks source link

Discussion: paramters passthrough #233

Open sxlwar opened 3 years ago

sxlwar commented 3 years ago

In this repo, I found a lot of function parameter pass-through. e.g.

function a(p1) {
    b(p1) // p1 is just for b function
    ...
} 
function b(p1) {
    c(p1) // p1 is just for c function
    ...
}

function c(p1) { 
    ...   // the truth is that the p1 parameter is needed here
}

a(param)

Once we need to trace the source of the parameter, we need to check the caller function. We need to determine whether the parameters have changed in the caller function and so on.

On the opposite, if we want to check what happened as the function called, we need to always be concerned about where is the parameter used.

The most obvious is the translation function - 't', it is passed into other functions quite frequently just to translate some tip message. Even worse, if the pass-through argument is not as easy to understand as the translation function, this is extremely unfriendly to the maintenance and understanding of the code.

Codes like hints, message, alerts should belong to view layer, it should not be coupled with the logic or configuration codes. So I think we should avoid parameters pass-through deeply, for the code above, if we refactor it to these below, maybe it more clear

function a() {
    ...
} 
function b() {
    ...
}

function c(p1) { 
    ...   // the truth is that the p1 parameter is needed here
}

a();
b();
c(param);

// or we can also aggregate all the steps into a single function
/*
   function fn(p1) {
       a();
       b();
       c(p1);
   } 

   fn(param);
*/

any idea?

WoeOm commented 3 years ago

Are there some actual code examples.

sxlwar commented 3 years ago

https://github.com/darwinia-network/apps/blob/e546e37be71c3db58e878a1355b2c2d7a1a4550d/packages/page-accounts/src/Accounts/modals/MultisigApprove.tsx#L84.

trace 'threshold' value, it comes from the Account component, actually comes from the meta field which comes from its parent component. In the MultisigApprove component, it is just passed to the TxButton component without any operation, and in TxButton pass-through continually. In the end, when the 'threshold' param is consumed, maybe we have forgotten it comes from where.

For the translate function example, you can view its route configuration. In my thought, these are configuration code, why not use a JSON, just because we need to translate?