nozavroni / csvelte

🕺🏻 CSV and Tabular Data library for PHP
http://phpcsv.com/
Other
6 stars 0 forks source link

Refactor Collection class #141

Closed nozavroni closed 7 years ago

nozavroni commented 7 years ago

The CSVelte\Collection class needs a refactor. It's trying to do way too much and it's becoming unwieldy. I have pretty extensive notes about this in the green binder, but to summarize:

The Collection class needs to be broken up into a hierarchy, like so:

CSVelte\Collection
    \AbstractCollection - base collection class containing methods common to all 
        \MixedCollection - collection of anything: arrays, numbers, strings, etc. 
            \ArrayCollection - collection of arrays; underlying data is at least a 2D array
                \TabularCollection - tabular collection contains an array of arrays with identical keys
        \CharCollection - collection of chars; basically a string; has methods like join(), ord(), etc. 
        \NumericCollection - collection of numeric values; has methods like avg(), sum(), mode(), etc. 

Another aspect of a collection I hadn't yet considered was its orderability. That is, whether or not the order of its items matters. I think this is an important distinction. And, at least for this library, an ordered collection is going to be of much greater use than one whose order isn't of any consequence. But I think perhaps there is a way (probably a whole bunch) to allow orderability without having to resort to the nightmare that is OrderedCollection, UnorderedCollection, OrderedTabularCollection, UnorderedTabularCollection, etc. The first thing that comes to mind is the decorator pattern. You would do something like so:

$table = new TabularCollection;
$orderedTable = new OrderedCollection($table);

But then I feel that in this library it makes more sense to default to orderability and to opt-out of this type of collection rather than opt-in. So if you wanted a collection that does NOT care about the order of items, you would do something like:

$numbers = new NumericCollection();
$unordered = new UnorderedCollection($numbers);

But then again, it also occurs to me that orderability may in fact be something that makes more sense to specify at the class level rather than the object. So it may make sense to implement this functionality as a trait. Collections that need to care about order would use a "IsOrdered" trait.

<?php
namespace CSVelte\Collection;

use CSVelte\Contract\IsOrdered;

class TabularCollection extends MultiCollection 
{
    use IsOrdered;
}

Where the IsOrdered trait would look something like this...

trait IsOrdered 
{
    public function getKeyAtPosition($offset) 
    {
        // ...
    }
    public function getValueAtPosition($offset) 
    {
        // ...
    }
    public function setKeyAtPosition($offset, $key) 
    {
        // ...
    }
    public function setValueAtPosition($offset, $value) 
    {
        // ...
    }
    public function __offsetGet($offset) 
    {
        // ...
    }
    public function __offsetSet($offset, $value) 
    {
        // ...
    }

    // there would also be a bunch of methods that would like, overwrite standard unordered methods here...
}

For full description of this refactor, see the notes in the green CSVelte binder.

Also, check out this list of functions. There's a lot of stuff you can pull from this:

https://github.com/nikic/iter/blob/master/src/iter.php

For instance, look how they use yield in their map function (and other places). You could definitely benefit from something like that.

nozavroni commented 7 years ago

See duplicate issue #148 for more on this...

nozavroni commented 7 years ago

I'm going to close this because I feel like I accomplished everything I set out to do with the refactor, but the Collection classes are by no means finished. There is a ton of work yet to do on them. I 'll just create new tickets for that stuff