Is this strict luau code ok?

Help and Feedback Code Review
#1

This is my second I think shot at trying to code with only strictly typed code (preparing for lower level languages) and since the types in luau are so broad, I would be more than happy if you know more about types than me and could point out mistakes or bad practices here in my code. Thank you :slight_smile:

--!strict

-- types

type Table = {[any]: any}

type AllTranslationsType = {translations: Table}

type AllTranslationBooksType = {books: Table}

type HttpReturnType = boolean | Table | nil?

-- services

local HttpService: HttpService = game:GetService("HttpService");

-- constants

local MAX_HTTP_RETRY_AMOUNT: number = 5;

-- variables

local HttpRetryCount: number = 1;


--// START UP CHECK //--


if (not HttpService.HttpEnabled) then
	error("Experience doesn't have HTTPService enabled!");
end


--\\\\\\\\\\\\\\\\\\\\\--
--// LOCAL FUNCTIONS //--
--\\\\\\\\\\\\\\\\\\\\\--


local function RunHTTP(URL: string): Table | nil
	
	local Success: boolean, DecodedJSON: Table = pcall(function()
		return HttpService:JSONDecode(HttpService:GetAsync(URL));
	end)

	if (Success and type(DecodedJSON) == "table") then
		return DecodedJSON;
	end

	return nil;
	
end


local function TryHTTP(URL: string): HttpReturnType
	
	local Result: HttpReturnType = RunHTTP(URL);
	
	if (Result) then
		
		return Result;
		
	elseif (HttpRetryCount <= MAX_HTTP_RETRY_AMOUNT) then
		
		warn(`TryHTTP failed with URL = {URL} {HttpRetryCount} times! Retrying`);

		HttpRetryCount+=1;
		
		return TryHTTP(URL);
		
	end
	
	HttpRetryCount = 1;
	
	return false;
	
end


--\\\\\\\\\\\\\\\\\\\\\--
--// MAIN FUNCTIONS //--
--\\\\\\\\\\\\\\\\\\\\\--


function AllTranslations(): AllTranslationsType | boolean
	
	local DecodedTranslations: AllTranslationsType | HttpReturnType = TryHTTP(`https://SomeUrl.com/alltranslations`);
	
	if (type(DecodedTranslations) == "table" and DecodedTranslations.translations) then
		return DecodedTranslations.translations::AllTranslationsType;
	end
	
	return false;
	
end


function AllTranslationBooks(PassedTranslationName: string | {string | number | {[number]: string}}): AllTranslationBooksType | boolean

	local DecodedBooks: AllTranslationBooksType | HttpReturnType = TryHTTP(`https://SomeUrl.com/{PassedTranslationName}`);

	if (type(DecodedBooks) == "table" and DecodedBooks.books) then
		return DecodedBooks.books::AllTranslationBooksType;
	end
	
	return false;
	
end

please note that the URL’s are just some random placeholders just for show, they arent real sites I think. Thanks again for reviewing! :smiley:

#2

i went through your code for maybe 5-10 minutes and it looks good in my opinion

the only things i would change is the amount of comments, semi-colons, and whitespace you use (i would use less; this is a personal preference)

i like your use of EXPO_CASE (SCREAMING_SNAKE_CASE) for constants and PascalCase for everything else; i would use PascalCase in dictionaries too

type AllTranslationsType = {["Translations"]: Table}

type AllTranslationBooksType = {["Books"]: Table}

I never organize my variables with comments (Unless there’s over 30, excluding types)

--!strict

-- types
-- I would make a Types module (child) if there are a lot of types being used
type Table = {[any]: any}

type AllTranslationsType = {["translations"]: Table}

type AllTranslationBooksType = {["books"]: Table}

-- I put parentheses around union types
type TranslationName = (string | {string | number | {[number]: string}})

type HttpReturnType = (boolean | Table | nil)?

-- services
local HttpService: HttpService = game:GetService("HttpService");

-- constants
local MAX_HTTP_RETRY_AMOUNT: number = 5;

-- variables
local HttpRetryCount: number = 1;

--// START UP CHECK //--

-- Use assert instead
assert(HttpService.HttpEnabled, "Experience doesn't have HTTPService enabled!")

--\\\\\\\\\\\\\\\\\\\\\--
--// LOCAL FUNCTIONS //--
--\\\\\\\\\\\\\\\\\\\\\--

-- You do not need whitespace between every line of code
-- Seperate related lines using whitespace

local function RunHTTP(URL: string): (Table | nil)
  local Success: boolean, DecodedJSON: Table = pcall(function()
    return HttpService:JSONDecode(HttpService:GetAsync(URL))
  end)
  
  -- Handle the 'bad returns' first 
  if (not Success or type(DecodedJSON) ~= "table") then return nil end
  return DecodedJSON 
  -- The semi-colon on a return is optional in most languages.
  -- I believe it's considered best practice to include the last semi-colon (I don't)
end


local function TryHTTP(URL: string): HttpReturnType
  local Result: HttpReturnType = RunHTTP(URL);
  if (Result) then return Result end
  
  -- I avoid else/elseif
  if (HttpRetryCount <= MAX_HTTP_RETRY_AMOUNT) then
    warn(`TryHTTP failed with URL = {URL} {HttpRetryCount} times! Retrying`);
    
    -- Put spaces between operations/comparisons/assertions
    HttpRetryCount += 1;
    return TryHTTP(URL)
  end

  HttpRetryCount = 1;
  return false
end

--\\\\\\\\\\\\\\\\\\\\\--
--// MAIN FUNCTIONS //--
--\\\\\\\\\\\\\\\\\\\\\--

function AllTranslations(): (AllTranslationsType | boolean)
  local DecodedTranslations: (AllTranslationsType | HttpReturnType) = TryHTTP(`https://SomeUrl.com/alltranslations`);

  if (type(DecodedTranslations) ~= "table" or not DecodedTranslations.translations) then return false end
  return DecodedTranslations.translations :: AllTranslationsType
end

-- Assuming PasseTranslationName would be type 'TranslationName'
function AllTranslationBooks(PassedTranslationName: TranslationName): (AllTranslationBooksType | boolean)
  local DecodedBooks: (AllTranslationBooksType | HttpReturnType) = TryHTTP(`https://SomeUrl.com/{PassedTranslationName}`);
  
  -- Since the DecodedBooks line is long, I would put this guard statement on one line
  if (type(DecodedBooks) ~= "table" or not DecodedBooks.books) then return false end
  return DecodedBooks.books :: AllTranslationBooksType
end

i don’t see any strict issues, and your types look good. (i rewrote some of the script on the forum)
my message really turned into a “how i would format things” example, but i do want to see how others would write it out

1 Like