JustArchiNET / ArchiSteamFarm

C# application with primary purpose of farming Steam cards from multiple accounts simultaneously.
Apache License 2.0
11.09k stars 1.04k forks source link

refactor: remove duplicate code block #3059

Closed nolddor closed 10 months ago

nolddor commented 10 months ago

Checklist

Changes

New functionality

N/A

Changed functionality

N/A

Removed functionality

N/A

Additional info

Since MobileAuthenticatorPlugin#OnBotCommand() calls Commands#OnBotCommand() under the hood. There is no need to check for function arguments here as Commands#OnBotCommand() is performing the exact checks already.

nolddor commented 10 months ago

I was just checking source code to see if I manage to learn anything from this awesome project. My knowledge about C# is almost inexistent so kindly reject this PR if makes no sense to you or my assumptions are wrong.

EDIT: Looks like qodana action failed as PR orginated from forked repos cannot access target secrets on workflows triggered by 'pull_request' events. You can conditionally disable this action by checking source and target repo names within the workflow file this way: if: ${{ github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name }} OR if: ${{ github.event.pull_request.head.repo.full_name == github.repository }}

JustArchi commented 10 months ago

It doesn't matter that function we redirect to has safeguards, every function is responsible for validating arguments its receiving at the first occurance, regardless of what it does furtther. If any function doesn't, then we should correct it.

nolddor commented 10 months ago

Gotcha!

For me makes sense that functions should be responsible for validating arguments in use within their own logic. But, in this concrete example, function only forward parameters to next function, and the function is capable to do so even if all arguments are null so there is no point to validate anything here.

Just personal tastes I guess, thanks Archi!