silverstripe/sapphire — comment on commit 8fa266462f2ef3aac5065045649f75f776498728
File: forms/CheckboxSetField.php line 190
By sminnee:

This looks dodgy. What if the data just happens to meet the `ArrayLib::is_associative` criteria but isn't supposed to be used that way? Perhaps you should do check the field name instead - if it ends with `[]` then you know that the values are supposed to be in the

Of course, this need to deal with two completely different data formats is a result of the fact that you want to butcher `CheckboxSetField` to turn it into a `MultiItemSelectField`. Perhaps it would be better to separate these two field types explicitly? Yes, the data is similar but there's a lot about the two fields that are different.

The dev cycle appears to have been "oh, hey, it's just a template update! oh, wait, there's also a data format change." I'm not sure what the value in keeping these two fields within a single class is, but I'd suggest you reconsider.