Could anyone help optimize/shorten/simplify these functions?

local inputPropertyReturnValue:{[string]:any} = {} 

local propertiesTable = {
	["cylinder"] = {"Color","Visible"},
	["displayPart"] = {},
	["joint"] = {"Position","Orientation","Visible"},
	["part"] = {"Position","Orientation","Size","Color","Material","Reflectance","Transparency"}
}
local function inputObjectReturnObjectTypeString(object):string
	if inputCylinderReturnArrayOfTwoJoints[object] ~= nil then return "cylinder"	
	elseif displayPartReturnsArrayOfTwoBlocks[object] ~= nil then return "displayPart"
	elseif inputJointReturnJoint[object] ~= nil then return "joint"	
	else return "part" end
end

local function hasProperty(object, propertyName):boolean

	local success, _ = pcall(function()
		object[propertyName] = object[propertyName] 
	end)

	return success

end

local function compare(new, old)

	local result = {}
	local equal = false

	if typeof(new) == "Vector3" then

		for _, xyz in {"X", "Y", "Z"} do

			if old[xyz] ~= "" and new[xyz] == old[xyz] then
				result[xyz] = new[xyz]
				equal = true
			else
				result[xyz] = ""
			end

		end

	else

		if old ~= "" and new == old then 
			result = new
			equal = true
		else	
			result = ""
		end

	end

	return result, equal

end
function module.UpdateUIAdding(lastSelectedObject) 

	for _, property in propertiesTable[inputObjectReturnObjectTypeString(lastSelectedObject)] do -- checks if 'lastSelectedObject' is a cylinder, displayPart, joint, or part, and then goes through the properties in the table for that object type
		
		if inputPropertyReturnValue[property] == "" then
			continue
		end
		
		local objectHasPropertyOrNil = nil

		if hasProperty(lastSelectedObject, property) == true then -- checks for the property or attribute of the last selected object
			objectHasPropertyOrNil = lastSelectedObject[property]
		elseif lastSelectedObject:GetAttribute(property) ~= nil then
			objectHasPropertyOrNil = lastSelectedObject:GetAttribute(property)
		end

		if objectHasPropertyOrNil ~= nil then -- if the property of attribute of the last selected object is found, then

			if inputPropertyReturnValue[property] == nil then -- if the table doesn't have the property (when the first/initial part is selected, such as from 0 to 1 number of parts selected, the table was blank before)

				if typeof(objectHasPropertyOrNil) == "Vector3" then 
					inputPropertyReturnValue[property] = { ["X"] = objectHasPropertyOrNil.X , ["Y"] = objectHasPropertyOrNil.Y , ["Z"] = objectHasPropertyOrNil.Z }
				else
					inputPropertyReturnValue[property] = objectHasPropertyOrNil
				end

			else -- if the table already has the property (when the number of parts selected (of the same type) is more than 1, the table already has the properties from the first part)
				
				inputPropertyReturnValue[property] = compare(objectHasPropertyOrNil, inputPropertyReturnValue[property])

			end

		end
		
	end

end

Thanks in advance!!!

I’d say just remove the needless spacing for every line, it is good to have an extra space for functions but just for every single line it’s too much. It does depend on what you find easier to read though

1 Like

In the local function compare(new, old), the variable equal is never used. I will not remove it however because I think I’ll need it in the future, maybe. If not, I know to remove it :D

1 Like

Are you trying to get all properties of instance?

I am not, only some of them, as shown by the propertiesTable
The keys of propertiesTable are special tags/attributes (but aren’t done though tags or attributes, rather I have an array of all my cylinders, joints, and everything else is just a part)

For each type, I only want specific properties :D

for inputObjectReturnObjectTypeString:

  • shorten varable names thats a headache to read.
  • consider restructuring object, to store its own object string. then you can eliminate this funciton as a whole

for hasProperty, you can condense this to simplify.

local function hasProperty(o,n)
     return table.pack(pcall(function() o[n]=o[n] end))[1] --return s instead of a tuple
end

compare:

  • Dont pass two types through the same variable name. Either pass a vector or nil, no need for any strings here. Same with the return. Implement like so
local function compareV3(v0,v1)
     local result = {}
     for _,v in {"X","Y","Z"} do
          result[v] = (v0[v]==v1[v] and v[0]) or nil
     end
     return result,v0==v1
end

Unsure if this still fits what you need? If its not enough, you can write a vector3 wrapper that defines iteration behavior, allowing you to iterate through the components of a vector3 in a for loop.

UpdateUIAdding:

  • Simplify variable names
    for example, naming a variable objectHasPropertyOrNil is by definition redundant. Everything is nil until its defined. You could get away with hasProperty here.

I didnt read that whole function because its a mess and the variable names are painfully long, but if you describe what youre trying to do I could provide some more guidance probably.

1 Like

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.