Is this a good way to do this script?

There are a few issues with your code; I will try as best I can to go through step by step as to what I did to fix it. Below is the working code.

game.Players.PlayerAdded:connect(function(p)
    p.CharacterAdded:connect(function(char)
        local humanoid = char:WaitForChild("Humanoid")
        humanoid.Walkspeed = 18
    end)
end)

1.) The listening event CharacterAdded only works on an active player, not on Workspace. If you wanted something to detect when objects enter workspace, ChildAdded is the way to go. To fix this, I simply added another anonymous function before CharacterAdded that detected players being added to game.Players.

2.) The third line of my code, after the functions were stated, defines the humanoid variable you wanted. While you did write out the userdata for the Humanoid, there were a few logic errors in doing this.

First, you didn't state the name of the variable. You simply wrote out userdata values, and this won't do you any good. Set it equal to a name like humanoid, and make it local to conserve script bandwidth.

Second, the userdata for this was incorrect. Player is not inside Workspace; it's a part of game. I fixed this by using char, since char is the variable I chose to represent the character being added; you can name this variable whatever you want.

Third, I noticed a syntax error, whenever you use :WaitForChild(), always use quotations for what is inside the parentheses. This method only accepts string values, so Humanoid would cause an error.

Fourth, you had conflicting conceptions. When you WaitForChild(), the program yields..Indefinitely.. until a descendant is found with a name of that string value. The if statement is useless when you do this because if that WaitForChild has unyielded, that means that you've found a Humanoid, so there's no guessing as to whether the Humanoid is there or not! I would also like to point out that if you did run that if statement, it would have had an error because of that userdata.

3.) This was a repeated error of misconstrued userdata, but I just wanted to point out that I used the humanoid variable I referenced on the line before to change the Walkspeed, instead of typing this line of userdata out again. It is generally a good rule in scripting to repeat userdata as little as possible.

I hope this helped, and if it did, make sure to upvote!

This is another way to make your script

function onPlayerEntered(newPlayer)
newPlayer.Character.Humanoid.WalkSpeed = 18 -- This changes the walkspeed
end
game.Players.ChildAdded:connect(onPlayerEntered)

function onPlayerRespawned(newPlayer) -- This is so if you die the walkspeed stays the same
h = newPlayer:findFirstChild("Humanoid")
if h ~= nil then
if game.Workspace:findFirstChild(h.Parent.Name) ~= nil then
h.WalkSpeed = 18 -- Just make sure this is the same number
end
end
end
game.Workspace.ChildAdded:connect(onPlayerRespawned)

The first end should not have a ) after it because it is closing the if statement.

CharacterAdded is also not an event of workspace. If you want to set everyone's walkspeed to 18 when they spawn, use the CharacterAdded event of Player:

game.Players.PlayerAdded:connect(function(player)
    player.CharacterAdded:connect(function(character)
        character:WaitForChild("Humanoid").WalkSpeed=18
    end)
end)

This is working code:

game.Players.PlayerAdded:connect(function(player)
    player.CharacterAdded:connect(function(character)
        character:WaitForChild("Humanoid").WalkSpeed=18
    end)
end)