ArcanePlugins / Treasury

🏦 A powerful multi-platform library for next-level plugin integrations.
https://hangar.papermc.io/ArcanePlugins/Treasury
Other
56 stars 13 forks source link

`EconomyTransaction`: Rename some variables, getters and setters #254

Closed lokka30 closed 1 year ago

lokka30 commented 1 year ago

Some variables (along with their respective getters and setters) should be renamed:

MrIvanPlays commented 1 year ago

Why?

Jikoo commented 1 year ago

Not opposed to these, but is it a large enough concern to bother? I do prefer Google's style of lowercasing subsequent letters in abbreviations. Is that specifically an inconsistency across the project?

lokka30 commented 1 year ago

@MrIvanPlays and @Jikoo:

transactionAmount -> amount

Prefixing amount with transaction whilst it is already clearly inside a transaction namespace (the EconomyTransaction class) is verbose and pointless.

economyTransactionType -> type

Identical reasoning to the above, though this is even more verbose than the transaction amount name.

currencyID -> currencyId

As Jikoo mentioned - inconsistency. Lowercase the 'd'.

MrIvanPlays commented 1 year ago

What you're doing here is creating an elephant from a fly.

lokka30 commented 1 year ago

What you're doing here is creating an elephant from a fly.

yup - I'm big on quality control. 😁

Regardless, would you be fine with the proposed changes?

MrIvanPlays commented 1 year ago

fixed in https://github.com/ArcanePlugins/Treasury/commit/ed5847d6be5a629ddf8d3ec1afb73cef4996fec7 and https://github.com/ArcanePlugins/Treasury/commit/57e8a3eddffe95b4ac245459d8f1c6f74a991142