Avoid return anti-patern

[orik007]

Hey

One of the reason i decide to rewrite steambot was of return antipatern there. The worst scenario is something like

bool CreateTradeOffer(params notImportant, out tradeOfferId)
{
    //only example
    if (params is wrong)
    {
        return false;
    }
    try
    {
        //call steam api       
        //tradeOfferId = callApi(params);    
    }
    catch
    {
        return false;
    }
}

This solutions has many disadvanates 1) You have to always check true/false and somehow handle it, keep watching return it to higher level, etc, it is called pyramide of doom in programming 2) You never know, why it is not working 3) Consuming exceptions is rly bad idea :-) 4) Exceptions should bubble on the top, where rollbacks transactions and should be handled by exception handler, etc

I undersntand is in SteamBot, because it is make mostly for newbs, or people which just need their account accepts treadeoffer autmaticaly during hole day. SteamBot can not be used in any serios project, it is excelent knowledge source. And newbs programmers wont handle exceptions, so they call if(call) then, its more or less understable.

But SteamAuth is library. its not for newbs its for programmers. It should look like

long CreateTradeOffer(params notImportant)
{
    //only example
    if (params is wrong)
    {
        throw new OurValidationException(what is wrong);
    }
    try
    {
        //call steam api       
        //tradeOfferId = callApi(params);    
        return tradeOfferId;
    }
    catch(Exception ex)
    {
        _log.Error(params + ex.ToString())
        throw;
    }
}

1) So u can always rely on result, its always success or exception 2) Exception will stop hole your process 3) You can have one handling place 4) Difference between types of exceptions, you can make loop in x level higher to repeat 5) More readable and clean code 6) Everything is logged

I know now it is too late for change, and this library was made in rush to cover the 2fa mobile needs.

Now i dont know if i have rewrite it too or not :-))). Perhaps not, cos im rly lazy and set mobile auth is only one step. I will watch that confirmations.

If you read till here, you re rly persistent :-) I just need to split it out, thx for watching.

[JustArchi]

Throwing exceptions is not always proper - https://github.com/JustArchi/ArchiSteamFarm/blob/master/ArchiSteamFarm/WebBrowser.cs#L80.

There are many cases where executed functions are actually allowed to fail, for many reasons, and that is totally acceptable, as you should always check if answer makes sense. When I'm e.g. executing steam API request, what matters for me is if request suceeded or not (true/false or answer/null), and calling try catch on every function call is pointless.

Throwing exception is costly, catching it as well. Using exceptions everywhere just to differ one fail from another is pointless, unless you have a valid reason to follow that approach. In most cases you don't have such reason, because what matters for you is if function succeeded or not - the actual reason for failure is not important.

Do not overcomplicate things that are not supposed to be so. It's easier to execute function and check if it returned non-null rather than trying and catching every single function call. It's not only performance that is considered, but code readability as well.

If I followed your suggestion in my project, I could as well wrap whole Main() in try catch.

[shravan2x]

I agree with @JustArchi on this. Additionally, all important functions of this library (UserLogin.DoLogin,AuthenticatorLinker.AddAuthenticator,etc) return enum values which describe exactly what went wrong. I think that should suffice. Maybe this will be changed in the future, but I doubt it.