Can I set a state within a function?

Question

I have this code:

procedure EstablishCommunication;
var
    State         : TStates;
    Attempts      : Byte;      
    procedure IncAttempts;
    begin
        Inc(Attempts);
    end;
begin
    State    := stReadDeviceID;
    Attempts := 0;

    while True do
    begin
        if Attempts >= MAX_ATTEMPTS then
        begin
            State := stError;
        end;
        case State of
            stReadDeviceID:
            begin
                // some code
                IncAttempts;
            end;
            stError:
            begin
                // Error code
            end;
        ...
        ...
        ...

I'd like to put the code that set state to stError within of the procedure IncAttempts, resulting:

procedure EstablishCommunication;
var
    State         : TStates;
    Attempts      : Byte;      
    procedure IncAttempts;
    begin
        Inc(Attempts);

        if Attempts >= MAX_ATTEMPTS then
        begin
            State := stError;
        end;
    end;
begin
    State    := stReadDeviceID;
    Attempts := 0;

    while True do
    begin            
        case State of
            stReadDeviceID:
            begin
                // some code
                IncAttempts;
            end;
            stError:
            begin
                // Error code
            end;
        ...
        ...
        ...

So, can I move the code to IncAttempts?

Is this a code smell?

If yes, Can you advice me a better way?

Answer 1

I would see this as perfect valid code. I ask myself the following questions when declaring a method inside another. Most of the time I don't do it, but sometimes it's results in better code.

• Will the internal function ever need to change as in a descendant class?
• Can I override External method without calling the internal method and be OK?
• Does the internal function have practical application outside of external method?
• Is the internal function complex enough that it should be unit tested outside the scope of there external method?

If any of the above apply don't use an Internal Method.

However if if you don't have any of the above, and it can remove repeated code and/or simplify the design then you can consider using a internal function.

Answer 2

No real problem with that, should work just fine. You are already modifying another local variable Attempts so there is no reason why modifying State should smell more. I do think you should be careful of using inline functions to much. The code often ends up hard to read/understand.