Is there a way to reduce this abysmally large if chain?

Help and Feedback Code Review
#1

Sup, I just wonder if there is a way I can reduce this abysmally large if chain:

--value: Is a variable containing the value.
--key: Is the key for a set of folders I am making. Basically the name of the value
--v: The BaseValue that will be created

			local v;
			if(type(value) == "number" and math.floor(value) == value) then v = Instance.new("IntValue");
			elseif(type(value) == "number") then v = Instance.new("NumberValue");
			elseif(type(value) == "string") then v = Instance.new("StringValue");
			elseif(type(value) == "boolean") then v = Instance.new("BoolValue");
			elseif(typeof(value) == "Instance") then v = Instance.new("ObjectValue");
			elseif(typeof(value) == "CFrame") then v = Instance.new("CFrameValue");
			elseif(typeof(value) == "Vector3") then v = Instance.new("Vector3Value");
			elseif(typeof(value) == "BrickColor") then v = Instance.new("BrickColorValue");
			elseif(typeof(value) == "Color3") then v = Instance.new("Color3Value");
			elseif(typeof(value) == "Ray") then
				v = Instance.new("RayValue");
			end
			
			if(not v) then error("Error at Configuration[Set]\nCould not set value '" .. tostring(value) .. "' with type '" .. typeof(value) .. "'.");
			else
				v.Value = value;
				v.Name = key;
				v.Parent = self:WaitForChild("Keys");
			end;

Thanks!

#2

You can create a dictionary of what you make:

local valueTypes = {
    ["number"] = "IntValue",
    ["string"] = "StringValue",
    --> etc.
}

Index the table with the datatype:

local v = Instance.new(valueTypes[typeof(value)])

Because you want a number and int value, just make it check for the number value and have the rest of the data types be instantiated with the above method.

1 Like
#3

Edit: For anyone wondering what code I did at the end, here is the result:

		local types = {
			string = "StringValue",
			boolean = "BoolValue",
			Instance = "ObjectValue",
			CFrame = "CFrameValue",
			Vector3 = "Vector3Value",
			BrickColor = "BrickColorValue",
			Color3 = "Color3Value",
			Ray = "RayValue"
		};
...
			local t = typeof(value); local v;
			if(types[t]) then v = Instance.new(types[t]) end;
			
			if(not v) then error("Error at Configuration[Set]\nCould not set value '" .. tostring(value) .. "' with type '" .. typeof(value) .. "'.");
			else
				v.Value = value;
				v.Name = key;
				v.Parent = self:WaitForChild("Keys");
			end;
#4

I’m not sure i’d describe it as an unnecessary variable. His code calls typeof only once, and indexes it from the table instantaneously, whereas yours calls typeof every single time it is checking for a condition. In my book it’s quite necessary

1 Like
#5

It would also error (Doing it with just the typeof) since there is no name with any of the typeof values. It is always RayValue and not just Ray. Another thing is that I can’t add “Value” to every one since some types don’t have that name. numberValue or InstanceValue. But now that you mention it, it would also be a good idea to not call it every time. Thanks

local t = typeof(v)

And doing it with the table is even more work since you need to make one for every other one.
number > NumberValue
Instance > ObjectValue

#6

You’re still left with the ugly if-chain, as well as performance loss

But this is a lot less work than writing several if-chains? Programming in general is a lot of work, as well. I suggest at least trying @VegetationBush’s solution as it is the most optimized.

1 Like
#7

I thought about it and you are right. I accept my stupidity as I came to the realization that I was completely wrong. Thanks for making my code better, you guys really helped improve my script!

1 Like