API Make DataList and ArrayList immutable #1020

Merged
merged 3 commits into from Dec 14, 2012

Projects

None yet

2 participants

@hafriedlander
SilverStripe Ltd. member

In 3.0 there was some confusion about whether DataLists and ArrayLists
were mutable or not. If DataLists were immutable, they'd return the result, and your code
would look like

$list = $list->filter(....);

If DataLists were mutable, they'd operate on themselves, returning nothing, and your code
would look like

$list->filter(....);

This makes all DataLists and ArrayList immutable for all searching operations.
Operations on DataList that modify the underlying SQL data store remain mutating.

  • These functions no longer mutate the existing object, and if you do not capture the value
    returned by them will have no effect:

    ArrayList#reverse
    ArrayList#sort
    ArrayList#filter
    ArrayList#exclude

    DataList#dataQuery (use DataList#alterDataQuery to modify dataQuery in a safe manner)
    DataList#where
    DataList#limit
    DataList#sort
    DataList#addFilter
    DataList#applyFilterContext
    DataList#innerJoin
    DataList#leftJoin
    DataList#find
    DataList#byIDs
    DataList#reverse

  • DataList#setDataQueryParam has been added as syntactic sugar around the most common
    cause of accessing the dataQuery directly - setting query parameters

  • RelationList#setForeignID has been removed. Always use RelationList#forForeignID
    when querying, and overload RelationList#foreignIDList when subclassing.

  • Relatedly,the protected variable RelationList->foreignID has been removed, as the ID is
    now stored on a query parameter. Use RelationList#getForeignID to read it.

@hafriedlander
SilverStripe Ltd. member

Related CMS patch at silverstripe/silverstripe-cms#256

@sminnee
SilverStripe Ltd. member

So, which methods are mutable?

@sminnee
SilverStripe Ltd. member

This all looks good, and is a much needed cleanup despite being an API change, but I think that one thing we should do is ensure that every method that is mutable has that fact mentioned in the docblocks. "Operations on DataList that modify the underlying SQL data store remain mutating" is a good general principle but I think telling users the specific methods that this refers to would also help.

Hamish Fried... added some commits Dec 12, 2012
Hamish Friedlander API Make DataList and ArrayList immutable
In 3.0 there was some confusion about whether DataLists and ArrayLists
were mutable or not. If DataLists were immutable, they'd return the result, and your code
would look like

  $list = $list->filter(....);

If DataLists were mutable, they'd operate on themselves, returning nothing, and your code
would look like

 $list->filter(....);

This makes all DataLists and ArrayList immutable for all _searching_ operations.
Operations on DataList that modify the underlying SQL data store remain mutating.

- These functions no longer mutate the existing object, and if you do not capture the value
returned by them will have no effect:

  ArrayList#reverse
  ArrayList#sort
  ArrayList#filter
  ArrayList#exclude

  DataList#dataQuery (use DataList#alterDataQuery to modify dataQuery in a safe manner)
  DataList#where
  DataList#limit
  DataList#sort
  DataList#addFilter
  DataList#applyFilterContext
  DataList#innerJoin
  DataList#leftJoin
  DataList#find
  DataList#byIDs
  DataList#reverse

- DataList#setDataQueryParam has been added as syntactic sugar around the most common
cause of accessing the dataQuery directly - setting query parameters

- RelationList#setForeignID has been removed. Always use RelationList#forForeignID
when querying, and overload RelationList#foreignIDList when subclassing.

- Relatedly,the protected variable RelationList->foreignID has been removed, as the ID is
now stored on a query parameter. Use RelationList#getForeignID to read it.
27113f8
Hamish Friedlander FIX Make sure ArrayList#limit uses clone so for subclasses it returns…
… instances of same subclass
9979b11
Hamish Friedlander FIX Make sure you can only remove items from a DataList that are actu…
…ally in it
bd59f84
@sminnee
SilverStripe Ltd. member

Cool, merging!

@sminnee sminnee merged commit 1303d82 into silverstripe:3.1 Dec 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment