Thoughts on this Real-Time Clock script?

Help and Feedback Code Review
1

So, i’ve made this real-time clock gui, that i released as a free model. It works fine, but i feel like the amount of or’s i used to shorten up the code makes it kind of hard to read. What do you guys think?

local runService = game:GetService("RunService")
local militaryTime = script.Parent.Parent["Military Time"]
local showSeconds = script.Parent.Parent["Show Seconds"]
local showShortDate = script.Parent.Parent["Show Shortened Date"]
local year = script.Parent.Year
local displayer = script.Parent.Number
local generatedTime

local months = {"January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"} --date["month"] only gives you a number so i used this to get the month's name

runService.Heartbeat:Connect(function()
	local date = os.date("*t", os.time())

	local hour = date["hour"] < 10 and "0"..date["hour"] or date["hour"]

	local mins = date["min"] < 10 and "0"..date["min"] or date["min"]

	local secs = date["sec"] < 10 and "0"..date["sec"] or date["sec"]

	local PM_AM = hour > 11 and " PM" or " AM"

	if militaryTime.Value then
		generatedTime = showSeconds.Value and hour..":"..mins..":"..secs or hour..":"..mins

	else --If it's in standard time
		hour = hour % 12 == 0 and 12 or hour % 12

		generatedTime = showSeconds.Value and hour..":"..mins..":"..secs..PM_AM or hour..":"..mins..PM_AM
	end

	local stringYear = tostring(date["year"]) --date["year"] returns a number so i gotta turn it into a string to be able to call :sub() on it
	year.Text = showShortDate.Value and months[date["month"]]:sub(1, 3).." "..date["day"]..", '"..stringYear:sub(#stringYear - 1, #stringYear) or months[date["month"]].." "..date["day"]..", "..date["year"]

	displayer.Text = generatedTime --Finally displaying the time
end)

Also, remember that the multiple values that the script checks are values that the player can change through gui buttons.

3 Likes
2 
print(os.date("%X"))

have you ever tried using this at all?

EDIT:
it shows military time
and I tested it so it looks like local time to me

if you want to edit the military time to standard time you can easily edit the string
you should try this

1 Like
3

Running on heartbeat for something that only needs to update every second?

1 Like
4

Nope. I didn’t really experiment with os.date that much.

EDIT: ily

5

Couldn’t that get a bit offsync?

6

No. You’re using os.time() which will return the correct time in seconds.

Change this to a while true do with a wait(1), nobody is going to care if 30 turns to 32 out of random.

Regardless, you should consider using os.date instead, helps from unnecessary work.

1 Like
7

I mean, i am using os.date. To get a dictionary.

8

So I changed the code, and I think it looks a lot better now.

local function Sub(st) --Yeah, I really don't know what to name this.
	--This function is used because %I will always return you 2 numbers.
	return st:sub(1, 1) == "0" and st:sub(2, #st) or st
end

local militaryTime = script.Parent.Parent["Military Time"]
local showSeconds = script.Parent.Parent["Show Seconds"]
local showShortDate = script.Parent.Parent["Show Shortened Date"]
local year = script.Parent.Year
local displayer = script.Parent.Number
local generatedTime

game:GetService("RunService").Heartbeat:Connect(function()
	local format = os.date

	if militaryTime.Value then
		generatedTime = showSeconds.Value and format("%H:%M:%S") or format("%H:%M")

	else --Standard time

		generatedTime = showSeconds.Value and Sub(format("%I:%M:%S %p")) or Sub(format("%I:%M %p"))
	end

	local day = Sub(format("%d")); year.Text = showShortDate.Value and format("%b "..day..", '%y") or format("%B "..day..", %Y")

	displayer.Text = generatedTime --Finally displaying the time
end)
9

You may use simple letters: a, b, c;

or these: _a, _b, _c

10

if you mean single letter variables, then that is bad practice

1 Like
11

I like to make my variable names clear so that I don’t have to keep going back and check what they are.