Jump to content

Code Readability and Cleanness


Crossy101

Recommended Posts

  
function vehicle(thePlayer, command, id) 
    id = tonumber(id) 
    if id then 
        if (id >= 401) and (id <= 611) then 
                local posX, posY, posZ = getElementPosition(thePlayer) 
                local createdVehicle = createVehicle(id, posX + 5, posY, posZ, 0, 0, 0, "Crossy") 
                local vehicleName = getVehicleName(createdVehicle) 
                warpPedIntoVehicle(thePlayer, createdVehicle) 
                outputChatBox("Vehicle Created : "..vehicleName, thePlayer) 
            else if (id <= 400) or (id >= 612) then 
                outputChatBox("Incorrect ID : "..id, thePlayer) 
            end 
        end 
    end 
end 
addCommandHandler("v", vehicle) 
  

I was just wondering if that's a good way for code to look like are other coders able to read that with no problems as I want my code as Cleanness and Readable as possible.

Link to comment

Just some random thoughts:

1) You could avoid nesting some if's by returning negative cases early

2) don't need or should use parenthesis around simple statements

  
if not id then 
  outputChatBox("ID was not specified: ", thePlayer) 
  return 
end 
-- calling tonumber after not id check, because if id was nil I don't really know what tonumber does, and checking "if not some_number" is ambiguous imo 
id = tonumber(id) 
if id <= 400 or id >= 612 then 
  outputChatBox("Incorrect ID : "..id, thePlayer) 
  return 
end 
  
local posX, posY, posZ = getElementPosition(thePlayer) 
local createdVehicle = createVehicle(id, posX + 5, posY, posZ, 0, 0, 0, "Crossy") 
local vehicleName = getVehicleName(createdVehicle) 
warpPedIntoVehicle(thePlayer, createdVehicle) 
outputChatBox("Vehicle Created : "..vehicleName, thePlayer) 
  

Link to comment

I actually find the OP's code to be more clean and easier to understand. :lol: Parenthesis's are not needed, no, but they do make it easier to read.

I made a tutorial on naming conventions a while ago. You seem to follow it for the most part, what I would suggest would be better function names. Also the check for whether id is not nil or false is kind of redundant since you check if it's an integer later on. (And you don't have an else statement to do something if it indeed is nil or false)

Edit:

I would've written it something like this;

function CreateVehicleFromID(thePlayer, theCMD, theID) 
    if(type(tonumber(theID)) == "number") then 
        if(theID >= 401 and theID <= 611) then 
            local pX, pY, pZ = getElementPosition(thePlayer) 
            local theVehicle = createVehicle(theID, pX+5, pY, pZ, 0, 0, 0, "Crossy") 
            local vehicleName = getVehicleName(theVehicle) 
            warpPedIntoVehicle(thePlayer, theVehicle) 
            outputChatBox("You've created a "..tostring(vehicleName), thePlayer, 0, 187, 0) 
        elseif(theID <= 400 or theID >= 612) then 
            outputChatBox("Error! Entered ID is invalid.", thePlayer, 187, 0, 0) 
        end 
    else 
        outputChatBox("Error! ID needs to be a number!", thePlayer, 187, 0, 0) 
    end 
end 
addCommandHandler("v", CreateVehicleFromID) 

Also the first example on getElementMatrix lets you make sure the car is always spawned at a fixed location relative to you.

Link to comment

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...