WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#9313 closed enhancement (wontfix)

Store the date format string "Y-m-d H:i:s" in a constant

Reported by: simonwheatley Owned by:
Milestone: Priority: low
Severity: trivial Version: 2.7
Component: Date/Time Keywords: needs-patch
Focuses: Cc:

Description

Currently the format string "Y-m-d H:i:s" is scattered through the WP codebase. It would seem more sensible and dryer (?) to have the format string in one place, perhaps a constant, and then refer to that constant wherever it is needed.

The attached patch does just that.

Attachments (1)

MYSQL_DATETIME format string constant.diff (15.8 KB) - added by simonwheatley 5 years ago.
A patch to change all dateformat strings like "Y-m-d H:i:s" to refer to a constant instead

Download all attachments as: .zip

Change History (11)

comment:1 simonwheatley5 years ago

One thing about this proposal I am unsure about, and which I would welcome a second opinion on, is where to set this constant. I did consider wp-db.php, but settled on wp-settings.php as the constant isn't purely used in a DB context.

comment:2 jacobsantos5 years ago

I know it is more or less micro-optimization verses readability, but seriously? So if a fix for one of those is to change it to 'Y-m-d' because it changed to DATE instead, would the person decide to update the constant or just replace the constant with another string? What about having a constant for Date, that is used outside the context of MySQL as well?

simonwheatley5 years ago

A patch to change all dateformat strings like "Y-m-d H:i:s" to refer to a constant instead

comment:3 simonwheatley5 years ago

  • Priority changed from normal to low

I accept this is a relatively minor issue, and that the world will keep turning either way. :) The pattern of using constants to specify date formats is, IMHO, a useful one and as I'm sure you're aware is even used in PHP 5 (http://uk2.php.net/manual/en/class.datetime.php#datetime.constants.types).

Re the misguided developer changing this constant and creating problems in places where they might not be looking... surely we expect out developers to be careful? If not, then surely this is what tests are for?

(Updated patch to fix a stupid bug it introduced.)

comment:4 Denis-de-Bernardy5 years ago

Funny... to me, Y-m-d H:i:s reads as MYSQL_DATETIME, much like /(?:\b[.+]{1,2})*?\d+?\b/ reads as plain english. :D

Seriously, though, while we're on the topic of using an app-wide constant rather than constant string that gets dumped in the functions, I'd be curious to know the memory and performance footprint of both options.

comment:5 Denis-de-Bernardy5 years ago

  • Milestone changed from Unassigned to 2.8
  • Version set to 2.7

comment:6 ryan5 years ago

  • Component changed from General to Date/Time
  • Owner anonymous deleted

comment:7 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; has-patch 2nd-opinion removed
  • Milestone changed from 2.8 to Future Release
  • Severity changed from normal to trivial

patch is no longer functional. suggesting wontfix, personally.

comment:8 Denis-de-Bernardy5 years ago

  • Resolution set to wontfix
  • Status changed from new to closed

no apparent interest

comment:9 Denis-de-Bernardy5 years ago

  • Milestone Future Release deleted

comment:10 simonwheatley5 years ago

@Denis-de-Bernardy - I'd be happy for this ticket to be deleted. (Is that even possible?) I think I can chalk this one down to a burst of over-enthusiastic DRYness.

Note: See TracTickets for help on using tickets.