Code review needed on my round module

Help and Feedback Code Review
#1 
function roundModule.round(value, requiredPlayers)
	
	round.Value = true
	
	local playersInRound = {}
	
	for _, player in ipairs(Players:GetPlayers()) do
		if player and not table.find(playersInRound, player) then
			table.insert(playersInRound, player)
			ServerStorage[player.Name].InRound.Value = true
		end
	end

	  for i = value, 0, -1 do
		
		gameStatus.Value = toSM(i)
		
		for i, player in ipairs(playersInRound) do
			
			if player then
				
				local character = player.Character 
				
				if character then
					
				else
					table.remove(playersInRound, i)
				end
				
				if ServerStorage[player.Name].InRound.Value == false then
				    print(string.format("Player was removed from round table: %s", player.Name))
					table.remove(playersInRound, i)
				end
				
			else
				table.remove(playersInRound, i)
			end
		end
		
		if #playersInRound < requiredPlayers then
			print("BREAKING_MODULE")
			break
		end
		
		if i == 0 then
			gameStatus.Value = "Round ended"
			wait(2)
			break
		end
		
        wait(1)
    end
end	

return roundModule

Iā€™m all ears.

1 Like
#2

If there is only one function in this table then return that function instead of a table

2 Likes
#3

There are more functions. Do you think that the code is flexible and efficient?

1 Like
#4

Your variable naming should be improved to make the code more self-documenting, e.g. ny renaming the value param to roundLength, and round.Value to round.InProgress.

Some of your solutions are a bit weird. When putting players in the playersInRound table, you dont need to check if theyre already in the table because you just inited the variable to a new empty table and :GetPlayers() never returns duplicate players.

One part of your code is incorrect. Inside the countdown loop, if a player leaves, the for ipairs loop would skip a player because you modified the table while iterating over it. You can fix this by iterating over it bacwards instead. I thinkā€¦

You also incorrectly remove an element at index i at two separate locations in the code, both of which can be reached in the same iteration.

1 Like