Rate my FizzBuzz code

Help and Feedback Code Review
#1

Watched a video about FizzBuzz and decided to try coding it! This is my outcome. Any ways of improving it?

local words = {3, "Fizz", 5, "Buzz"}

for i = 1, 100 do
	
	local output = ""
	
	
	for j = 1, #words - 1, 2 do
		if i % words[j] == 0 then
			output = output .. words[j+1]
		end
	end
	
	if output == "" then
		output = i
	end
	
	
	print(output)
	
end
2 Likes
#2

I like how you used an array to select fizz or buzz. It makes it much easier to extend the code for different uses. However, it’s a bit finicky as tables can be used as a hashmap. Why not use that to your advantage. Make a hashmap in which the keys are integers and the values are strings. Something like this:

local words = { [3] = "Fizz", [5] = "Buzz" } -- using [n] to set key.

--##Then in the loop...##--
for i, selectedString in next, words do
    -- So on...

Here are some challenges for you!
(or else it won’t be a true interview hehe)

  1. After 3 numbers are found that are divisible by 5, the output should only be “Surprise!”
  2. If a number is divisible by 3, the output should be “Baz”. Everything other conditions remain.
  3. If a number is divisible by 30, then the output should be “Foo” only. It shouldn’t take priority over other conditions.

Rating? 8.5/10 not bad kid.

2 Likes
#3

EDIT: Whoops, I misread your code as if you were building the entire output of the program into a single string and printing that at the end. The way you’re doing it, you still only end up with Fizz, Buzz and FizzBuzz, so no problem at all. Still some decent information below tho :sweat_smile:

This is actually kind of bad, performance wise. You shouldn’t do that in code where performance is any kind of concern. Instead, let output be a list and insert each word into that list. When you want to print it, use table.concat to turn all of the little strings into one big string.

AFAIK, Lua internally “saves” each string you create. The first time you create the “Fizz” string there’s some overhead, but the next time it just works with the same string instead of having 100s of copies of the same string. However when you concatenate “Fizz” to the existing output, that’s an entirely new string and Lua has to have it as a separate thing. That means you end up with more than 100 different strings in memory potentially at the same time, or even more if you decide you want to Fizz all the way to 1000000.

Using the way I suggested, you only end up with 100s of references to the same string, which is a lot better. You end up with only 3 strings in memory, “Fizz”, “Buzz” and the final string that gets printed.

In short, avoid iteratively building large strings from smaller strings with the .. operator / string concatenation, and instead use the built-in table.concat to concatenate all the smaller parts all at once at the end.

Souce: https://www.lua.org/gems/sample.pdf

Although AFAIK Roblox has their own internal version of Lua that might have different optimizations, so take it with a grain of salt and ideally do your own testing.

1 Like