refactor uuid checks into a method #1001

Merged
merged 3 commits into from Dec 21, 2012

Projects

None yet

5 participants

@dogmatic69
Contributor

Make the code more dry with a method for checking if a field is uuid

@markstory markstory and 3 others commented on an outdated diff Dec 4, 2012
lib/Cake/Model/Model.php
@@ -1779,6 +1775,17 @@ public function save($data = null, $validate = true, $fieldList = array()) {
}
/**
+ * Check if the passed in field is a UUID field
+ *
+ * @param string $field the field to check
+ * @return array
+ */
+ public function isUUID($field) {
@markstory
markstory Dec 4, 2012 Member

Does this need to be public? I don't know if we really need to make Model's public API even larger.

@dereuromark
dereuromark Dec 4, 2012 Member

We got our own validation rule for uuids:

$pattern = '=[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}=is';
preg_match($pattern, $uuid, $matches);

Maybe we could add uuid() as validation rule instead? then there is no need to make this method here public and one could still validate uuids.

@dogmatic69
dogmatic69 Dec 4, 2012 Contributor

@markstory There are checks on related table fields here

@dereuromark This does not check data, it checks what the field is setup as.

@rchavik
rchavik Dec 4, 2012 Member

is it isUUID() or isUuid().

I noticed a few in RequestHandler that follows camelCased naming convention with the exception of isSSL().

@dogmatic69
dogmatic69 Dec 4, 2012 Contributor

I was not to sure about that either. The variables were $isUUID, there is also String::uuid().

Wikipedia has upper-case:

A universally unique identifier (UUID) is an identifier standard used in software construction

Seems like acronyms you spell out (U - U - I - D) are upper-case

@dogmatic69
Contributor

The failing test seems unrelated

PaginatorComponentTest::testPaginate
Failed asserting that two arrays are equal.

Seems the ordering is off in postgresql

@markstory
Member

I think this is reasonable as a protected method. Any objections?

@ADmad ADmad commented on an outdated diff Dec 8, 2012
lib/Cake/Model/Model.php
@@ -1772,6 +1768,17 @@ public function save($data = null, $validate = true, $fieldList = array()) {
}
/**
+ * Check if the passed in field is a UUID field
+ *
+ * @param string $field the field to check
+ * @return array
@ADmad
ADmad Dec 8, 2012 Member

Return type is boolean not array.
Also I personally would prefer the method name to be _isUUIDField() as it more clearly conveys that we are checking the field not the actual value.

@markstory markstory merged commit 69248df into cakephp:2.3 Dec 21, 2012
@dogmatic69 dogmatic69 deleted the dogmatic69:refactor-uuid-checks branch Dec 21, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment