Is this using too many nested if statements?

Help and Feedback Code Review
, , , ,
#1

Please ignore the other bits of code, only focus on the if and for statement within the function.

The while loop is a MUST have. I will ignore any comments on it as it is fine where it is.
Please read the full script snippet and give me your thoughts on anything I could do better

-- ai sits idle like in NFS 2015 and waits for a new target to appear if theyre above a certain speed
	function new.idle()

		local originPosition = new.seat.CFrame.Position
		local radius = 30 -- STUDS DISTANCE

		local findNearbyInstances = function(position, radius)
			local parts = workspace:GetChildren()
			local nearbyInstances = {}

			if #parts > 0 then

				for _, part in ipairs(parts) do

					if part:IsA("Model") and #part:GetChildren() > 0 and part:IsDescendantOf(script.Parent) == false then
						
						-- i need to rewrite this part. it is old and outdated for what i am doing.
						if part:FindFirstChildOfClass("VehicleSeat") then

							local distance = (part:FindFirstChildOfClass("VehicleSeat").CFrame.Position - position).magnitude

							if distance <= radius then
								table.insert(nearbyInstances, part)
							end

						end								

					end
				end

			end

			return nearbyInstances
		end

		local nearbyInstances = findNearbyInstances(originPosition, radius)

		local TargetModel = new.ChaseTarget or nil
		local TController = nil
		
		-- here we resolve the controller for the target
		local resolveTController = function(TModel: Model)
			
			return TModel:WaitForChild("Scripts"):WaitForChild("Controller")
			
		end
		
		if TargetModel ~= nil then
			TController = require(resolveTController(TargetModel))
			
		end

		if new.isIdle == true and new.inChase == false and new.ChaseTarget == nil then

			for _, instance: Model in ipairs(nearbyInstances) do

				if instance ~= nil then

					if instance:WaitForChild("Inputs") then
						
						local Tinputs = instance:WaitForChild("Inputs")
						
						local Throttle = Tinputs:GetAttribute("throttleInput")

						if Throttle ~= 0 then
							
							if TController ~= nil then
								
								local speed = TController:getChassisForwardSpeed()
								
								if speed == nil then
									continue
								end
								
								if speed > 0 and speed > 20 or speed < 0 and speed < -20 then
									
									new.isIdle = false
									new.inChase = true
									new.ChaseTarget = instance -- sets the target model

									task.spawn(coroutine.wrap(function()
										
										local seat = instance:WaitForChild("DriverSeat")
										
										while task.wait(1/15) do
											new.advancedPath(seat)
										end

									end))
									
								end
								
							else
								continue
							end
							
						else
							continue
						end

					else -- if its not a car then skip
						continue
					end

				else -- if it is nil then skip
					continue
				end
				
			end -- end of for

			-- gets ran once when the ai is started
		elseif new.isIdle == true or new.inChase == false or seat == nil or new.ChaseTarget == nil then

			new.inChase = false
			new.ChaseTarget = nil
			new.isIdle = true
			new.pathfindingCompleted:Fire(true)
		end

	end
1 Like
#2

Yeah, readability wise, this is terrible, way too much nested if statements, try to separate your code into smaller chunks, or make some of them 1 line, for example:

if distance <= radius then
	table.insert(nearbyInstances, part)
end

replace it with something like this

if distance <= radius then table.insert(nearbyInstances, part) end

another example:

if instance ~= nil then

    if instance:WaitForChild("Inputs") then

can be:

if instance ~= nil and instance:WaitForChild("Inputs") then

It condenses your code, making it easier to read, a thought I always keep in mind, is

  • If I came back to this project a month from now, will I be able to understand the code?

  • If I was working in a team, would the developers understand my code easily?

I always keep these thoughts in mind when coding, so I can keep my project more organised.

3 Likes
#3

I can personally always read my code. I don’t exactly expect others to read these let alone have it touch anyone else eyes apart for mine but yeah you are right I could merge some of them and one line however for me it is easier to read ones that are formatted than ones that are one line.

The way I read is top down instead of left to right (its just easier on the eyes imo) so I probably won’t one line them if not needed however I will keep this in mind in the future if I want to or think it will be applicable.

Thanks again.

2 Likes
#4

I would suggest looking at the early exit pattern.

2 Likes
#5


(use guard clauses)

5 Likes
#6

I’m not new I know what I am doing just I feel I am using way too many if statements nested within each other.

#7

I do here and there. I usually nest a couple if’s here and there as I need to type check etc.

#8

The answer is yes you are. This article suggests ways of improving your code - sorry if the advice you asked for hurt your ego!

#9

Not hurt ego here? It doesn’t hurt me as I wouldn’t ask otherwise, odd you’d assume.

#10

dude he’s 17 leave him alone he’ll get out of that phase soon

#11

Besides having way too many if-statements, using :GetChildren() is extremely slow to run every time, you should keep track of your objects in a table and manually add/remove them based on events instead.

Second, to improve the readability of your code, considering using early-return.

Example Early-return

Flawed / Hard-to-read

if condition1 then

  if condition2 then

    if condition3 then

      DoFunction()

    end

  end

end

Correct / Better readability

if not condition1 then return end

if not condition2 then return end

if not condition3 then return end

DoFunction()

Try doing that as much as possible (and where it makes sense of course), it’ll drastically increase the readability of your code.

5 Likes
#12

I suggest doing what @C_Corpze did here. The correct if statements are called gaurd clauses. They make your code look more readable, look simpler and organized.

2 Likes