search

nasa / CF

The Core Flight System (cFS) CFDP application.
Apache License 2.0
79 stars 45 forks source link

CF function CF_CFDP_IsSender(transaction_t *ti) is odd one out because it uses ti #30

Closed jphickey closed 1 year ago

jphickey commented 2 years ago

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1728] CF function CF_CFDP_IsSender(transaction_t *ti) is odd one out because it uses ti Originally submitted by: Gibson, Alan S. (GSFC-5870) on Tue Sep 14 10:58:59 2021

Original Description: Every other function in cfdp.c that uses a transaction_t* as an argument names it 't', but CF_CFDP_IsSender uses ti.

astrogeco commented 2 years ago

https://github.com/nasa/CF/blob/96cdf3071996eb98fd1920e7ea39d7af0a2f85c8/fsw/src/cf_cfdp.c#L142-L161

skliper commented 2 years ago

While I do appreciate not wasting characters... neither ti or t are all that descriptive/helpful in terms of variable names. Maybe transaction or txn or similar? I'd consider either of these options more "readable".

thnkslprpt commented 2 years ago

While I do appreciate not wasting characters... neither ti or t are all that descriptive/helpful in terms of variable names. Maybe transaction or txn or similar? I'd consider either of these options more "readable".

I would tend to agree with you Jake. However, there are several hundred uses of all 3 options (t, txn & transaction) in the code base. So for now, it might be best to just correct this particular instance. For CF_Transaction_t objects specifically, they practically all use ‘t’, so we can align these 2 aberrations to the majority, like Alan Gibson suggested. Perhaps future work could focus on improving the argument names in general, to make them more readable.