Defining and Better words

Help and Feedback Code Review
#1 
local HttpService = game:GetService("HttpService")

local Module = {}

function Module:Define(String)
	local UniformResourceLocator = "https://www.dictionary.com/browse/" .. String

	local GetAsync = HttpService:GetAsync(UniformResourceLocator)

	local String2 = string.match(GetAsync, "content=%b\"\"")
	String2 = string.gsub(String2, "content=", "")
	String2 = string.gsub(String2, " See more.", "")
	return String2
end

function Module:Synonym(String)
	local UniformResourceLocator = "https://www.thesaurus.com/browse/" .. String
	local GetAsync = HttpService:GetAsync(UniformResourceLocator)

	local Table = string.split(GetAsync, "><a font-weight=\"inherit\" href=\"/browse/")
	local Table2 = {}
	for Number, String in pairs(Table) do
		local String2 = string.match(String, "%w+")
		if String2 ~= "DOCTYPE" then
			table.insert(Table2, String2)
		end
	end
	local MathRandom = math.random(1, #Table2)
	return Table2[MathRandom]
end

return Module

What do you think of this its my first time every using HttpService:GetAsync()

I think I did pretty good :slight_smile:

Not sure its Efficient

2 Likes
#2

Looks good. There’s some small efficiency improvements where you could use captures instead of having to gsub things out of the matched string in your Define function, but that’s really not a big issue and given the delay of loading the website itself, the microseconds saved here would be inconsequential.

The biggest issue is correctly handling HTTP errors and failures if the site is down or you reach the HttpService limits. It’s always best practice to pcall your results and handle errors gracefully rather than letting them affect the rest of the stack.

#3

How do I catch Http Errors etc. Never used pcall() exept to not let things error while doing game:GetChildren()

#4

There’s an example in point 3 of this point:

The exact behaviour you choose to do in the situation is dependent on what you’re using this module for. If this is critical you may display a message to the user to let them know the call failed, by returning false or perhaps the error message itself.

You might put something more descriptive in the output using warn or print to help you debug later. If the lack of response doesn’t affect gameplay then you might choose to do this and silently fail without saying anything to the user.

You might, in the case of the synonym, to prevent it breaking the game just return the same word that was originally given. Not ideal, but better than nothing at all, and it can be accompanied with a user friendly message.

The possibilities are endless and it’s your decision in terms of what this module is actually for in its wider context and how it fits into your game or system. Currently only you have the knowledge of the full context to be able to make that decision.

#5

Updated: Fixed Returning Unrelated words.

local HttpService = game:GetService("HttpService")

local Module = {}

function Module:Define(String)
	local UniformResourceLocator = "https://www.dictionary.com/browse/" .. String

	local GetAsync = HttpService:GetAsync(UniformResourceLocator)

	local String2 = string.match(GetAsync, "content=%b\"\"")
	String2 = string.gsub(String2, "content=", "")
	String2 = string.gsub(String2, " See more.", "")
	return String2
end

function Module:Synonym(String)
	local UniformResourceLocator = "https://www.thesaurus.com/browse/" .. String
	local GetAsync = HttpService:GetAsync(UniformResourceLocator)

	local Table = string.split(GetAsync, "><a font-weight=\"inherit\" href=\"/browse/")
	local Table2 = {}
	for Number, String in pairs(Table) do
		local String2 = string.match(String, "%w+")
		if String2 ~= "DOCTYPE" then
			if string.find(String, "IN A SENTENCE BELOW") then
				local String2 = string.match(String, "%w+")
				table.insert(Table2, String2)
				break
			end
			table.insert(Table2, String2)
		end
	end
	return Table2
end

return Module

Example:

local Module = require(game:GetService("ReplicatedStorage").Module)

for Number, String in pairs(Module:Synonym("Part")) do
	print(Number, String)
end

No problem.

(Also anyway to make it more efficient would be helpful).

#6

Looks nice, but please don’t use colons in your code when you aren’t using OOP. A common response to this, “It doesn’t affect anything performance wise.”. Well ofc not its just bad practice.

Module:Define(String) = Module.Define(self, String)

Basically you are including an unnecessary self argument that you won’t even use. Efficiency wise, your code looks fine to me.

#7

Is there any reason in particular on why you decided to give your Url variable the name of “UniformResourceLocator” instead of something shorter? Just curious as to why you would do that other than preference.

#8

You also using Module. in OOP as well?

#9

Yeah, whenever I use OOP I typically require them and create objects from my client or server scripts.