What is the best way to write functions?

When I'm creating a listener function (one that listens for an event to fire) and it's not going to have that much depth to it, I'd use the anonymous function (second code). When it's not a listener function (one that isn't connected to an event), I'd usually create named functions. HOWEVER, the best way to do this is to go by your instincts. Nobody will really care if you use either one anyway.

As a bit of an aside:

anonymous functions are useful because they require fewer lines of you to write. Two fewer, specifically. When you 'stack' them:

game.Players.PlayerAdded:connect(function(player)
    player.CharacterAdded:connect(function(character)

    end)
end)

You write code with a typographical layout that exactly matches the logical layout. Basically, it makes it easier to follow along with what's actually being done in the code.

The cost of this is speed: an anonymous function has to be re-declared in memory when it's called. This is because the entire function goes out of scope once it finishes running. For most cases, this isn't an important distinction, but large functions can become laggy for no reason other than this.

On the topic of actually using them, it's whether or not you need to use it repeatedly. Inline code such as this is by nature one-off. You can call it in a loop, or inside another event-connected function, of course, but it still technically only works once.

If, for instance, you had a PlayerAdded event that connects after one or more Players have already joined the game, using a named function will be easier:

function PlayerAdded(player)
    player.CharacterAdded:connect(function(character)
        --This doesn't stop use from using stacked ones, though!
    end)
end

for _, v in ipairs(game.Players:GetPlayers()) do
    PlayerAdded(v)
end

game.Players.PlayerAdded:connect(PlayerAdded)

I usually prefer giving the function a name. That lets me invoke it myself, rather than just allowing the event to call it.

This actually tends to be very useful. I can re-use a kill function:

function Kill(part)
    if part.Parent and part.Parent:FindFirstChild("Humanoid") then
        part.Parent:BreakJoints()
    end
end

I can now use this with a .Touched event, or I can use it with anything else that needs to kill things.

A particular place where this is necessary is with PlayerAdded. When testing, scripts won't start quickly enough to catch when it fires usually. Thus I usually have this code:

function newPlayer(player)
end

game.Players.PlayerAdded:connect(newPlayer)
for _, player in pairs(game.Players:GetPlayers()) do
    newPlayer(player)
end

It also helps you avoid deeply-nested function definitions in events, which becomes difficult to read. It's easier to read the name of the function and understand what's happening in just the name.

All of this in favor of giving them names, anonymous functions are still a really nice feature. If you really don't need something in more than one place, e.g., second parameters to table.sort, then keeping it a bit briefer and just using the anonymous function is a good idea.

I personally like the 2nd one since it is shorter, but sometimes you need to use the first one if you want to use it multiple times. Use the 2nd one though, the second one is called anonymous functions. A downside to these functions are that you can only use them in the expression they're formed in. Most of the high reputation users use these anonymous functions.

Hope it helps!

I personally believe that this is based on preference, but is a good idea to learn both. If you have both in mind when creating a script, then you can shorten your script with the second one. As for me, I personally use the first function style you listed. It just comes easier to me and I use it most often. I would use the second one if you have several functions in the script though, to make it shorter.