Calling functions inside other functions: bad?

I'm trying to make a good effort at teaching myself good coding practices because as my work gets more and more complex, my code gets messier and messier...so try to bear with my incessant questioning.

I recently came to a head on something I've been working on when I realized I had the exact same chunk of code in two different parts of a file I was working on. Being obviously ridiculous I tossed it all into a function and deleted the extraneous copy. Normally this wouldn't be a big deal, except that this particular function needs to be called for two different things: On a PLAYER_ENTERING_WORLD event (easy enough), and also as part of a slash command function.

For the sake of simplicity:

local function masterfunction(self)
    print("I'm doing some trivial crap.")
end
 
 
local function slashie(cmd, editbox)
    --SLASHIE
        if cmd == 'SLASHIE' then
            masterfunction()
        end
    end
 
 
local function eventdoer(self, event)
    if event == "PLAYER_ENTERING_WORLD" then
        masterfunction()
    end
end
 
 
SlashCmdList["ADDON"] = slashie;
SLASH_ADDON1, SLASH_ADDON2 = '/addon', '/add';
 
 
MF = CreateFrame("FRAME")
MF:RegisterEvent("PLAYER_ENTERING_WORLD")
MF:SetScript("OnEvent", eventdoer)

Good/bad/doing it wrong/won't work? Can/should I combine eventdoer and masterfunction so that one function can do all of it? I feel like I'm overlooking something so simple and blaringly obvious that I'm going to feel really stupid when it's pointed out.

[Xrystal]

As long as you validate any parameters needed to be used in the function that should be fine. You'll find you will use functions more and more as your addons grow and you find that you end up with code so similar that you just create one function and pass a parameter in to make the difference.

It might be just me but make sure that the functions that are used in other functions are above them in the file so that when it processes the functions further down it doesn't get confused.

[Torhal]

One of the main purposes of a function is re-use. In fact, if I find myself with a function that I'm using in only one place I replace the call to it with the direct code.

Also, here's a little snippet to get rid of all those "if event == "SOME_EVENT" then" checks:

MF:SetScript("OnEvent", function(self, event, ...)
				if self[event] then
					return self[event] (self, event, ...)
				end
			end)

function MF:PLAYER_ENTERING_WORLD()
        masterfunction()
end

Originally Posted by Xrystal It might be just me but make sure that the functions that are used in other functions are above them in the file so that when it processes the functions further down it doesn't get confused.

Yeah, you're right. That was just hastily copy/pasted and edited to remove all the non-pertinent stuff.

Originally Posted by Torhal Also, here's a little snippet to get rid of all those "if event == "SOME_EVENT" then" checks...

Yeah, those were just another carry-over from my copy/paste crapjob. Always interested in more ways to clean up shoddy code practices, though.

And I don't know why I'm so hung up over this. Even though I know it's not it just feels like a jump statement to me, and I've still got that nagging "GOTO/GOSUB is bad" voice in my head (even though this is nothing like that...well it's sort of like a GOSUB, but not really). Or maybe I'm just a neurotic mess. IDK.

Thanks for the help though, guys.