How do I optimize this elseif disaster?

Help and Feedback Code Review
#1

Hello, I have a big issue here. I don’t know how to optimize this code. I’ve put a lot of elseifs on it and now I feel it will make things worse.

What this does is both put a hologram of a piece in one of three places which changes if it’s looking the other way around
And while it does that, it accomodates the camera by putting in 4 different directions, one of them being interchanged with the key s. What it’s doing here is use the keys Q W E A S D for this function, but I find it disgusting to have many elseifs on it.

local function botones(key)
	if uno == true then
		if key == "q" and puedebocha1 then
			numpos = 1
			choclon.Position = banana.Position + Vector3.new(0, 11.789, -41.102)
		elseif key == "w" and puedebocha2 then
			numpos = 2
			choclon.Position = banana.Position + Vector3.new(0, 11.086, 4.292)
		elseif key == "e" and puedebocha3 then
			numpos = 3
			choclon.Position = banana.Position + Vector3.new(0, 15.22, 42.588)
		end
	elseif uno == false then
		if key == "q" and puedebocha1 then
			numpos = 3
			choclon.Position = banana.Position + Vector3.new(0, 15.22, 42.588)
		elseif key == "w" and puedebocha2 then
			numpos = 2
			choclon.Position = banana.Position + Vector3.new(0, 11.086, 4.292)
		elseif key == "e" and puedebocha3 then
			numpos = 1
			choclon.Position = banana.Position + Vector3.new(0, 11.789, -41.102)
		end
	end
	if key == "a" then --camara
		camera.CFrame = banana.CFrame *			 CFrame.fromEulerAnglesXYZ(0,math.rad(180),0) + Vector3.new(0, 0, -150)  --a
	elseif key == "s" then
		if uno == true then
			camera.CFrame = banana.CFrame * 	 CFrame.fromEulerAnglesXYZ(0,math.rad(90),0) + Vector3.new(100, 0, 0) --s1
			uno = false
		elseif uno == false then
			camera.CFrame = banana.CFrame *		 CFrame.fromEulerAnglesXYZ(0,math.rad(-90),0) + Vector3.new(-100, 0, 0) --s2
			uno = true
		end
	elseif key == "d" then
		camera.CFrame = banana.CFrame *			 CFrame.fromEulerAnglesXYZ(0,math.rad(0),0) + Vector3.new(0, 0, 150) --d
	end
end

I did a post on how to optimize this but I still don’t have it clear.

#2 
local function _(_)numpos=uno and(_=="q"and 1 or _=="w"and 2 or _=="e"and 3)or(puedebocha1 and _=="q"and 3 or puedebocha2 and _=="w"and 2 or puedebocha3 and _=="e"and 1) choclon.Position=uno and(puedebocha1 and _=="q"and banana.Position+Vector3.new(0,11.789,-41.102)or puedebocha2 and _=="w"and banana.Position+Vector3.new(0,11.086,4.292)or puedebocha3 and _=="e"and banana.Position+Vector3.new(0,15.22,42.588)or choclon.Position)or(puedebocha1 and _=="q"and banana.Position+Vector3.new(0,15.22,42.588)or puedebocha2 and _=="w"and banana.Position+Vector3.new(0,11.086,4.292)or puedebocha3 and _=="e"and banana.Position+Vector3.new(0,11.789,-41.102)or choclon.Position) camera.CFrame=_=="a"and banana.CFrame*CFrame.fromEulerAnglesXYZ(0,math.rad(180),0)+Vector3.new(0,0,-150)or _=="s"and(uno and banana.CFrame*CFrame.fromEulerAnglesXYZ(0,math.rad(90),0)+Vector3.new(100,0,0)or banana.CFrame*CFrame.fromEulerAnglesXYZ(0,math.rad(-90),0)+Vector3.new(-100,0,0))or _=="d"and banana.CFrame*CFrame.fromEulerAnglesXYZ(0,math.rad(0),0)+Vector3.new(0,0,150)or camera.CFrame uno=_=="s"and(not uno)or uno end

tell me if it doesn’t work i will try to fix it

#3

Can you put separations between the parts of code? then notify me later, I can’t tell what the code is doing.

#4

local function botones(key)

numpos = uno and (key == “q” and 1 or key == “w” and 2 or key == “e” and 3) or (puedebocha1 and key == “q” and 3 or puedebocha2 and key == “w” and 2 or puedebocha3 and key == “e” and 1)

choclon.Position = uno and (puedebocha1 and key == “q” and banana.Position + Vector3.new(0, 11.789, -41.102) or puedebocha2 and key == “w” and banana.Position + Vector3.new(0, 11.086, 4.292) or puedebocha3 and key == “e” and banana.Position + Vector3.new(0, 15.22, 42.588) or choclon.Position) or (puedebocha1 and key == “q” and banana.Position + Vector3.new(0, 15.22, 42.588) or puedebocha2 and key == “w” and banana.Position + Vector3.new(0, 11.086, 4.292) or puedebocha3 and key == “e” and banana.Position + Vector3.new(0, 11.789, -41.102) or choclon.Position)

camera.CFrame = key == “a” and banana.CFrame * CFrame.fromEulerAnglesXYZ(0,math.rad(180),0) + Vector3.new(0, 0, -150) or key == “s” and (uno and banana.CFrame * CFrame.fromEulerAnglesXYZ(0,math.rad(90),0) + Vector3.new(100, 0, 0) or banana.CFrame * CFrame.fromEulerAnglesXYZ(0,math.rad(-90),0) + Vector3.new(-100, 0, 0)) or key == “d” and banana.CFrame * CFrame.fromEulerAnglesXYZ(0,math.rad(0),0) + Vector3.new(0, 0, 150) or camera.CFrame

uno = key == “s” and (not uno) or uno

end

#5

Put enter between the parts so we can know what it’s supposed to be like without having to scroll through the line

#6

If you tried it, please tell me if it works. I did it in a hurry.

#7

It sends a lot of code errors first hand, and considering I don’t know what each part is doing, it basically doesn’t work.

#8

Hello, errors in running or pasting

Edit:
“_” is not recommended i did it just to minify the code

#9

Hey, listen. It’s okay if you don’t know how to do this. I’ve been coding since, around 2019, and have taken long breaks in the middle (school). I, an amateur at coding, am struggling to make some parts of code, you will struggle even more. Don’t worry, I’ll figure things out.

#10 
local function botones(key)
numpos = 
	uno 
	
	and 
	(
		(key == "q" and 1) or 
		(key == "w" and 2) or 
		(key == "e" and 3)
	) 
	
	or 
	(
		(puedebocha1 and key == "q" and 3) or 
		(puedebocha2 and key == "w" and 2) or 
		(puedebocha3 and key == "e" and 1)
	) 

choclon.Position = 
	uno 
	
	and 
	(
		(puedebocha1 and key == "q" and banana.Position + Vector3.new(0, 11.789, -41.102)) or 
		(puedebocha2 and key == "w" and banana.Position + Vector3.new(0, 11.086, 4.292)) or 
		(puedebocha3 and key == "e" and banana.Position + Vector3.new(0, 15.22, 42.588))
	) 
	
	or 
	(
		(puedebocha1 and key == "q" and banana.Position + Vector3.new(0, 15.22, 42.588)) or 
		(puedebocha2 and key == "w" and banana.Position + Vector3.new(0, 11.086, 4.292)) or 
		(puedebocha3 and key == "e" and banana.Position + Vector3.new(0, 11.789, -41.102))
	) 
		
	or 
		(choclon.Position)

camera.CFrame = 
	(
		key == "a" 
		
		and 
		banana.CFrame * CFrame.fromEulerAnglesXYZ(0,math.rad(180),0) + Vector3.new(0, 0, -150)
	) 
	
	or 
	(
		key == "s" 
		
		and 
	    (
		    uno 
			
		    and 				
		    (banana.CFrame * CFrame.fromEulerAnglesXYZ(0,math.rad(90),0) + Vector3.new(100, 0, 0)) 
			
		    or 
		    (banana.CFrame * CFrame.fromEulerAnglesXYZ(0,math.rad(-90),0) + Vector3.new(-100, 0, 0)) 
	    )	
	) 
	
	or
	(
		key == "d" 

		and 
		(banana.CFrame * CFrame.fromEulerAnglesXYZ(0,math.rad(0),0) + Vector3.new(0, 0, 150))
	)
	
	or 
	    (camera.CFrame)

uno = (key == "s" and (not uno)) or uno end

Here is a even more optimized version, bugs could be found easier for you :confused:

1 Like
#11

The most basic shape of it works, but I think it still needs optimizations. I will do it, thanks for your help…

#12

You can either use dictionaries or a switch statement (basically the same but more fancy.)

Dictionaries:

local keys = {
    ["Q"] = function()
        -- stuff here.
    end
}

local function botones(key)
    keys["Q"]() -- will run the function Q.
end

Switch:

local Switch = function(Value, Cases)
    assert(type(Cases) == "table", "Switch statement second argument must be a table with the possible cases!");
    
    local Function = (Cases[Value] or Cases["default"]);
    if (Function) then Function() end;

    return (Cases[Value] and true or false);
end;

local function botones(key)
   Switch(key, {
      ["q"] = function()
         -- stuff here
      end
   })
end

(https://github.com/Fenix7667/Lua-Switch-Statement)

3 Likes
#13

Separate it into tables and use the selected table data.

local puedebochas = {1 = true; 2 = true; 3 = true;};
local UnoValues = {};
UnoValues[true] = {};
UnoValues[false] = {};

UnoValues[true]["q"] = {NumberPosition = 1; VectorOffset = Vector3.new(0, 11.789, -41.102)};
UnoValues[true]["w"] = {NumberPosition = 2; VectorOffset = Vector3.new(0, 11.086, 4.292)};
UnoValues[true]["e"] = {NumberPosition = 3; VectorOffset = Vector3.new(0, 15.22, 42.588)};

UnoValues[false]["q"] = {NumberPosition = 3; VectorOffset = Vector3.new(0, 15.22, 42.588)};
UnoValues[false]["w"] = {NumberPosition = 2; VectorOffset = Vector3.new(0, 11.086, 4.292)};
UnoValues[false]["e"] = {NumberPosition = 1; VectorOffset = Vector3.new(0, 11.789, -41.102)};

local function botones(Key)
 if not puedebochas[UnoValues[uno][Key].NumberPosition] then return end

 numpos = UnoValues[false][Key].NumberPosition
 choclon.Position = banana.Position + UnoValues[false][Key].VectorOffset
end

This is essentially just a really big dictionary; Read above!

3 Likes
#14

That did seem to work, and it does seem to optimize the code. Thank you…

thanks to you, @AsynchronousMatrix for your help, too…

1 Like
#15

thats not optimized at all… just have a table lol

local keyDictionary = { -- maybe store this data in a module
   q = {
        checker = puedebocha1,
        integer = 1,
        offset = Vector3.new(0, 11.789, -41.102),
   },
}
local function botones(key)
    if uno == true then
        local indexedKey = keyDictionary[key]
        if indexedKey and indexedKey.checker then
            numpos = indexedKey.integer
            choclon.Position = banana.Position + indexedKey.offset
        end
    else
        -- too lazy to continue but u get the point :/
    end
end

oh oofers someone already gave a solution :pensive:

1 Like