search

Zn4rK / php-withings

PSR-FIG Compatible Withings Body metrics Services API written in PHP.
17 stars 14 forks source link

Collection isn't a collection #11

Closed mattstauffer closed 8 years ago

mattstauffer commented 9 years ago

The thing named Collection is actually more like an entity, or a value object. Collections--especially in the Laravel sense--are like a grouping of many similar items. So the SubscriptionList profiles parameter would be a collection; it's many profiles. Or, the array of groups that's attached to a measure could be a collection instead of an array. But most of the things called "Collection" here are really a single class of objects with a particular set of properties, etc. which would mean they're not really a collection.

I'd be happy to write a PR, but I want to be sure that you're interested in it first; I think they should be converted to be entities (naming difference) with their properties set explicitly rather than dynamically; e.g.

<?php
class Measure extends Entity
{
    /** @var Carbon **/
    public $updatedtime;

    /** @var MeasureGroupCollection */
    public $groups;

    // etcetera...
}
Zn4rK commented 9 years ago

You are totally right. It dawned on me a few months after I actually used laravel in a project that I had it wrong.

I would love to see a PR with a fix!

mattstauffer commented 9 years ago

Great; I'll PR it after you have time to merge in the others. No rush, just having fun cleaning this all up. :)

On Sun, Nov 16, 2014 at 2:29 PM, Alexander Liljengård < notifications@github.com> wrote:

You are totally right. It dawned on me a few months after I actually used laravel in a project that I had it wrong.

I would love to see a PR with a fix!

— Reply to this email directly or view it on GitHub https://github.com/Zn4rK/php-withings/issues/11#issuecomment-63234373.

Zn4rK commented 9 years ago

I'm glad you find it fun! It's been a while since I touched this project, but I still feel like I can learn more so I'm just happy that you found this project and want to contribute.

It kinda feels like we also need to update the readme, it feels outdated. English is not my native language, so feel free to modify it if you have any suggestions :). I think we can remove all my 'thoughts' from it and just do a description, todo, and contributors link or something.

If you don't have time for it, I'll probably fix it in the comming week or so.

Zn4rK commented 8 years ago

Closing this one.