Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

New Data package #1712

Merged
merged 2 commits into from Nov 28, 2012

Conversation

Projects
None yet
6 participants
Contributor

eddieajau commented Nov 23, 2012

Overview

This pull request adds a Data package with two new classes: JData and JDataSet.

While similar to the familiar JObject class, the get and set methods are removed in favour of using direct property accessors ($object->key) that are controlled by magic methods. The properties are held in a private array within the class, completely separate from the real class properties. The setProperties method is replaced with a bind method which is used elsewhere in the API (cf. JTable->bind). A 'dump' allows you to obtain a stdClass representation of the data object. This is also used to make the class implement the JsonSerializable interface.

A JData can also act as an array iterator allowing you to use it in a foreach construct.

The JDataSet class provides a convenient way to work with lists of JData objects. Aside from the regular iterator functionality, the magic get method acts like a "get column" operation on the entire list, and the magic set method acts like a "set column" operation on the entire list. The magic call method allows you to call a method (if available) on each of the objects in the list. See the documentation for more information.

Additional notes and comment can be found in the documentation.

This PR supersedes #1673 (and #1703) which looked at making improvements to the existing JObject class. After review, it was found reasonable to incorporate the proposed new features and architecture in a new package thereby allowing JObject, an artefact of bridging work between PHP 4 and PHP 5, to be deprecated at some point in the future (such work does not form part of this pull request and will be left for others to pursue).

Documentation

Documentation is provided with the pull request.

https://github.com/eBaySF/joomla-platform/blob/data-package/docs/manual/en-US/chapters/packages/data.md

Some historic information on JObject is also provided:

https://github.com/eBaySF/joomla-platform/blob/data-package/docs/manual/en-US/chapters/packages/object.md

Business Case

The JObject class has severe limitations when trying to merge domain logic with a value or data object patterns. This can be seen in classes like JTable and particularly JUser where you need to perform some coding gymnastics to avoid column mismatches when saving to the database. To this end, we needed a better way [for eBay applications] to interact with the value object itself, and also so it could interact with a Data Mapper pattern (a long term alternative/replacement for the JTable-way of doing things) so that the value object could remain independent of the data source (unlike JTable which is a value object that is tightly coupled to a specific datasource).

JData and JDataSet provide a convenient way for working with structured, semi-structured and unstructured data.

Contributor

eddieajau commented Nov 23, 2012

Fixed since #1703:

  • PHP strict problem in JDataSet
  • Recursive dumping on JData and JDataSet objects changed to JDumpable
  • Improved documentation verbiage

@LouisLandry LouisLandry commented on an outdated diff Nov 23, 2012

libraries/joomla/data/data.php
+ }
+
+ return $this;
+ }
+
+ /**
+ * Set a property value if not alreay set.
+ *
+ * @param string $property The name of the property to define.
+ * @param mixed $value The value to give the property.
+ *
+ * @return mixed If the property is defined, the current value, otherwise, the new value is returned.
+ *
+ * @since 11.1
+ */
+ public function def($property, $value = null)
@LouisLandry

LouisLandry Nov 23, 2012

Contributor

I really don't think there is a need to have def() for this object.

Contributor

eddieajau commented Nov 24, 2012

def removed.

Contributor

florianv commented Nov 24, 2012

I think it could be useful to implement Countable in JData which count only one level of properties (not recursively)

Contributor

eddieajau commented Nov 25, 2012

@florianv that would be fine. Either make a pull request against my branch or you could make a pull request after this is merged.

@LouisLandry LouisLandry added a commit that referenced this pull request Nov 28, 2012

@LouisLandry LouisLandry Merge pull request #1712 from eBaySF/data-package
New Data package
44abef1

@LouisLandry LouisLandry merged commit 44abef1 into joomla:staging Nov 28, 2012

Just noticed this, but you've capitalized the o in instanceof. It doesn't make a difference in the parsing, but my IDE didn't highlight it properly. (Which is why I noticed)

Contributor

eddieajau replied Nov 28, 2012

What a dumb mistake :) I'll make a not to fix it. Might add that countable interface while I'm at it.

Contributor

florianv commented Nov 28, 2012

@eddieajau I made a pull request for Countable on your repo some time ago

Contributor

eddieajau commented Nov 28, 2012

Ah ok, sorry, I missed the email for that somehow. Since it's merged, would you mind making it against the this platform, and also, would you mind fixing up that "instanceOf" in JDataSet that Don found?

Thanks in advance.

Contributor

florianv commented Nov 28, 2012

Done in #1723

Contributor

eddieajau commented Nov 28, 2012

Thanks. I'll just wait for the pull tester to roll over.

Contributor

Yehonal commented on 8047f39 Nov 29, 2012

instead to create more methods to get/set class variables , we could extend JData with JObject instead...isn't right?

What do you mean by "extend JData with JObject"?

N.B.: the original PR was a re-animated JObject, but after discussion it was changed to JData.
See https://groups.google.com/forum/#!topic/joomla-dev-platform/fPY9oFcmXnA

Contributor

eddieajau replied Nov 29, 2012

The magic get an set replace the concrete get and set found in JObject. No, we won't be extending JData from JObject. JData is a replacement for JObject.

Contributor

Yehonal replied Nov 29, 2012

ok so we can use $test->__set("var1", $var); instead extend JObject to do the same thing, right?

Contributor

eddieajau replied Nov 29, 2012

No. You do $test->var1 = $var. See:

https://github.com/joomla/joomla-platform/blob/8047f39634dd148826a42b5c87fceb5786a95a90/docs/manual/en-US/chapters/packages/data.md

The difference between JData and JObject is that in JData, it will add the property to an internal array. In JObject, it will add the property to the actual class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment