Is this a good practice / worth avoiding?

Help and Feedback Code Review
1

Basically I’m writing a framework for my game intended to be used by other developers I’m unlikely to meet / able to walk through the steps of each aspect. I’ve heard somewhere that having so many unnecessary if else statements is quite bad and should be avoided if possible. Keep in mind this script is being ran on the client.

function Module:addDebounce(debounceName : string, duration : number, toggleVal : boolean)
	if debounceName then
		if typeof(debounceName) == "string" then
			if duration and not toggleVal then
				if typeof(duration) == "number" then
					if not DebounceTable[debounceName] then
						DebounceTable[debounceName] = {['duration'] = duration, ['lastTick'] = tick()}
					else
						print('Debounce already exists! consider changing / removing! type /duration - number/')
					end
				else
					print('Incorrect Arg type, /number/ expected, got '..typeof(duration))
				end
			else
				if toggleVal and not duration then
					if typeof(toggleVal) == "boolean" then
						if not DebounceTable[debounceName] then
							DebounceTable[debounceName] = {['toggleVal'] = toggleVal}
						else
							print('Debounce already exists! consider changing / removing! type /boolean/')
						end
					else
						print('Incorrect Arg type, /boolean/ expected, got '..typeof(toggleVal))
					end
				else
					print('Conflicting Args/toggleVal - duration/ please do not use both! - Check both Args are not /nil/!')
				end
			end
		else
			print('Incorrect Arg type, /number/ expected, got '..typeof(debounceName))
		end
	else
		print('Missing Arg/debounceName/ please provide a string!')
	end
end

I’m thinking since right now I’ll be the only person using it I might just remove the print messages after testing. Thanks!

( Also I’d appreciate any advice on how to actually improve this code! )

2

You should consider using “Return values” and “or” statements, and set them close to eachother. This way, it’s more organized and easier to read.
Example:

module.addDebouce = function(name,duration,number,toggleVal)
	if not name or not duration or not number or not toggleVal then
		warn("Missing val: "name,duration,number,toggleVal) --I use warn because it just looks better than print, but they basically do the same thing
		return -- you seem pretty good at scripting but if you don't know what this does, it returns a value and ends the script here, not allowing the below code to run, if return is ran of course.
	end
	--Continue on with your script
end

I gotta go somewhere else soon so if you have other questions hopefully someone else will answer them, if not, well I’ll be back by in a few hours : p

3

Just noticed I have my syntax wrong using : instead of . but I am curious, is there a reason to usue Module.AddDeboucne = function over my method? Would you suggest I write this type of thing using OOP instead of just using the module to store the function? I’m trying to simply create extra functions I can call at any time to get a result / value.

4

There’s no difference between function Module.addDebouce() and
module.addDebouce = function() it’s just preference on how you want to do it.

And should you use OOP to write this function? Well, from the looks of what you’re trying to do, it seems like it would be better to just use OOP, as the script seems to be constantly checking if something exist, and if it doesn’t, it just sets it. Which OOP would fix

Also I forgot to answer your topic, yes it is bad practice to do a bunch of if else statements, even if it’s on the client. Cause instead of the Server being overloaded, now it’s the Client, and if the Client is getting overloaded, this can cause lag or more battery power, and if you’re a mobile device, the last thing you want to play is something that drains your battery. Not only that, as I said before, it’s hard to read what it does, if you take like a week break from coding, you’re probably gonna forget what you’re script does, and now you’re gonna have to relearn what you were just making. AND ALSO, debugging would be annoying, just look at the amount of print statements you have :d

5

tbh if it works it works

☆*: .｡. o(≧▽≦)o .｡.:*☆

2 Likes
6

Is this an improvement? How would you improve this further?

function Module.addDebounce(debounceName : string, duration : number, toggleVal : boolean)
	if typeof(debounceName) == 'string' and DebounceTable[debounceName] == nil then
		if typeof(duration) == 'number' and toggleVal == nil then
			DebounceTable[debounceName] = {['duration'] = duration, ['lastTick'] = tick()}
		else
			if typeof(toggleVal) == 'boolean' and duration == nil then
				DebounceTable[debounceName] = {['toggleVal'] = toggleVal}
			end
		end
	end
end
7

it is indeed bad, you can simply avoid that by using table and few if statements instead of many elseifs, create function for specific input and then fire it when if detects some scenario

8

like iATaCk_Noobsi mentioned, you should use if statements like this:

function Module.addDebounce(debounceName : string, duration : number, toggleVal : boolean)
	if typeof(debounceName) ~= 'string' or DebounceTable[debounceName] ~= nil then
       return --less unnecessary indentation
    end
	if typeof(duration) == 'number' and toggleVal == nil then
		DebounceTable[debounceName] = {['duration'] = duration, ['lastTick'] = tick()}
	elseif typeof(toggleVal) == 'boolean' and duration == nil then-- elseif instead of else if, less indentation
		DebounceTable[debounceName] = {['toggleVal'] = toggleVal}
    end
end

you can also make tables for checking the typeof() of duration and toggleVal, but I would only do that if you had many more cases to check for them

9

I see so avoid indentation where possible for neater code?

10

generally, yes.
but it’s not completely about indentation, because indentation also improves readability:

function idk(num)
   if num<5 then return end
   --more code
end

vs

function idk(num)
   if num<5 then
       return
   end
   --more code
end

mainly you just want to avoid large stacks of if statements & make your code’s logic easy to understand

11

Avoid nesting your code:

--Bad
if condition then callback() else error end
--Good
if not condition then error return end
callback()

If you follow the good approach, you can put all of your code will be much more readable

12

That makes sense. I appreciate the feedback from everyone here and will begin implementing it shortly.