Am I avoiding the guard clauses technique?

I was wondering if this snippet is clean enough or do I need to revamp it for readablity?

function ReturnPaycheck(Player:Player?)
	if Player.MembershipType ~= Enum.MembershipType.Premium and not __MRKSERVICE:UserOwnsGamePassAsync(Player.UserId ,0) then -- not premium and does not own gamepass either
		return 2000
	end
	if Player.MembershipType ~= Enum.MembershipType.Premium and  __MRKSERVICE:UserOwnsGamePassAsync(Player.UserId ,0) then -- Not premium but owns gamepass
		return 4000
	end
	if Player.MembershipType == Enum.MembershipType.Premium then -- Player is premium but does not own gamepass
		return 3000
	end
	if Player.MembershipType == Enum.MembershipType.Premium and  __MRKSERVICE:UserOwnsGamePassAsync(Player.UserId ,0) then -- Premium and owns gamepass
		return 6000
	end
end

The snippet seems functional, but it could be improved for readability. Here are some suggestions:

  1. Use a variable to store the value of __MRKSERVICE:UserOwnsGamePassAsync(Player.UserId, 0) instead of calling the function twice in each conditional block.
  2. Use elseif instead of multiple if statements since they are mutually exclusive.
  3. Use descriptive variable names to make the code more readable.

Here’s an example of how the code could be refactored:

function ReturnPaycheck(player)
	local hasGamePass = __MRKSERVICE:UserOwnsGamePassAsync(player.UserId, 0)
	
	if player.MembershipType == Enum.MembershipType.Premium and hasGamePass then
		return 6000
	elseif player.MembershipType == Enum.MembershipType.Premium then
		return 3000
	elseif hasGamePass then
		return 4000
	else
		return 2000
	end
end

In this version, the code uses hasGamePass to store the result of __MRKSERVICE:UserOwnsGamePassAsync(Player.UserId, 0) so it’s not called multiple times. It also uses elseif to make the conditions mutually exclusive and uses descriptive variable names to make the code more readable.

2 Likes

This wont work, it will never check if the user owns the gamepass since if the player is premium & owns the gamepass, it will pass the “is premium” check.

I’d prolly write the script to:

function ReturnPaycheck(Player:Player?)
	local ownsGamepass = __MRKSERVICE:UserOwnsGamePassAsync(Player.UserId ,0)
	
	if Player.MembershipType ~= Enum.MembershipType.Premium then
		if ownsGamepass then
			return 4000
		else -- This could be end since above would return
			return 2000
		end
	end


	if ownsGamepass then
		return 6000
	else -- Could also be end
		return 3000
	end
end

Alternative:

function ReturnPaycheck(Player:Player?)
	local ownsGamepass = __MRKSERVICE:UserOwnsGamePassAsync(Player.UserId ,0)
	
	if Player.MembershipType ~= Enum.MembershipType.Premium then
		if ownsGamepass then return 4000 end
		return 2000
	end

	if ownsGamepass then return 6000 end
	return 3000
end

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