0
\$\begingroup\$

I have the working code but it is pretty dirty. The goal is to input a number in an application. If it satisfies the min/max logic then jump to the authorization part. Afterwards we can confirm it or reenter it or cancel it. If fails then re-input it. So totally we can only input twice.

 public virtual CallerAuthorization GetCallerAuthorizationResult(decimal debitBalance)
    {
        bool success;
        try
        {
            _debitBalanceAmount = debitBalance;
            GetFundAmountToTransfer();
            success = BuildAmountMinMaxLogicLoop();
        }
        catch (HangupException)
        {
            throw new CallHangupException();
        }
        if (!success)
        {
            success = ReenterAmount();
        }
        if (!success)
            return null;
        AuthorizeTransfer();
        var result = GetProceedInstruction();
        if (result == "1")
            return _callerAuthorization;
        if (result == "2")
        {
            success = ReenterAmount();
            if (!success)
            {
                CancelAuthorization();
                return null;
            }
            AuthorizeTransferReenterAmount();
            return _callerAuthorization;
        }
        if (result == "3")
        {
            CancelAuthorization();
            return null;
        }
        return null;
    }

The other methods:

 public virtual bool BuildAmountMinMaxLogicLoop()
    {
        var isContainingSpecialChars = CheckWhetherInputContainingSpecialChars();
        var isNotAuthorizable = IsNotAuthorizable();
        if (isNotAuthorizable)
        {
            if (isContainingSpecialChars)
            {
                PlayTransferAmountLogicError();
                return false;
            }
            PlayTransferAmountLogicError();
            return false;
        }
        return true;
    } 

Also:

public virtual bool ReenterAmount()
    {
        PlayFundAmountToTransferPrompt();
        GetFundAmountToTransfer();
        var isNotAuthorizable = IsNotAuthorizable();
        if (isNotAuthorizable)
        {
            PlayGoodByeMessagePrompt();
            return false;
        }
        return true;
    }

    private void AuthorizeTransferReenterAmount()
    {
        GetAuthorizationResult();
        if (ResultInfo.Code == 0 && AuthorizationInfo.ResultInfo.Code == 0)
        {
            _callerAuthorization.AuthCode = AuthCode;
            _callerAuthorization.FundsTransferInfo = FundsTransferInfo;
        }
    }
\$\endgroup\$
  • 1
    \$\begingroup\$ Some of the methods are missing. It would help if you can post the missing code. \$\endgroup\$ – SiD Dec 8 '15 at 8:58
1
\$\begingroup\$
_debitBalanceAmount = debitBalance;

Assigning this to a member value doesn't seem right, and this is before any validation.

GetFundAmountToTransfer();

This function is implies it will return something, but you don't assign it anywhere. This is a code smell.

var result = GetProceedInstruction();
if (result == "1")
    return _callerAuthorization;

A meaningless string literal, not good. Make it an enum with a good name. Where did _callerAuthorization get assigned?

The control flow is not complicated and yet it's hard to read. Try breaking it up into smaller functions.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.