This is a sampling of code meant to demonstrate collections for attrio. #5

anithri · 2013-06-25T21:48:25Z

This request is meant for discussion, it is unready for merging.

I'm going to do this work, but I wanted to know what you guys thought of it. It could also be implemented as a plugin for Attrio.

I love the Attrio code so far, and I'm planning to use it in another project soon. However, I need to have similar functionality for collections of objects in addition to the attributes already provided. This outlines one way to accomplish this.

We start by modifying AttributesParser to include a #collection public method which allow you to define collections. Each collection has 3 groups of methods. a Reader which gives you full access to the collection. a AddElement, which adds a new element to the array, and a FindElement which will allow you to read a single element of the collection based on an index.

Collections may be 1 of 3 types. With no special options, Attrio::Collection will create an Array. With a :unique options key set to true and no :index key, Attrio::Collection will create a Set. With a :unique key set to true and an :index key, Attrio::Collection will create a hash, and will call the method defined by the :index key and use the result as the hash key. This would allow you to do something like...

class ClassSection
  include Attrio
  define_attributes do
    attr :name, String
    attr :teacher, Faculty
    collection :students, Student, unique: true, index: :student_id
    collection schedule, DateTime, unique: true
    collection reading_materials, String
  end
end

the students collection would be a hash in this case, using the return value from student.student_id, as the key.
The schedule collection would be a set, and the reading_materials collection would be an array.

FindElement can be used for any container, the default value defined for the collection will be returned if no element is found.

AddElement should be structured to accept an array of hashes that can be used to add a number of elements at the same time as well as adding only a single one at a time.

This code is not tested yet.

Readme changes are examples of using Collections.
attributes_parser had collection, create_collections, and
fetch_container_type methods added.
collection is a class parallel to Attribute for use with collections.

This code is not tested yet. Readme changes are examples of using Collections. attributes_parser had collection, create_collections, and fetch_container_type methods added. collection is a class parallel to Attribute for use with collections.

coveralls · 2013-06-25T21:49:57Z

Coverage decreased (-0%) when pulling 0779cf6 on anithri:collection_proposal into dda5c73 on jetrockets:master.

igor-alexandrov · 2013-06-26T13:57:25Z

Hello,
We reviewed a bit your code and these are our thoughts about it.

This is cool and should be added to Attrio
Implementation should be done as follows:

we need three different classes: Collection::Array, Collection::Set and Collection::Hash that all should be inherited from Array, Set and Hash instead of passing container to constructor of Collection.
these classes should be include module that will implement methods like reader_visilbilty and so on; this module should also be included into Attribute class.
3) index option should be renamed to key, because it reflects what it actually is; it is a key from Hash.

…ute. I used Attrio::AttributeBase as the module name for the shared methods, but I'm not happy with that name and when I come up with a better name, I'll rename it. I left the writer methods in Attrio::Attribute since only Attribute is going to use those methods. Factoring those into another module is something to keep in mind.

I used the DelgateClass as the basis for inheritence because it offers an easy way to wrap and extend classes while delgating all other calls to the original class. Specs are very simple, just testing creation and that they are acting a little like their delgate.

coveralls · 2013-06-28T22:25:17Z

Coverage remained the same when pulling 79dc5b6 on anithri:collection_proposal into dda5c73 on jetrockets:master.

…ble modules moved attr_reader into Readable module. removed InstanceMethods module from Readable decided not to define initialize in a module. This could be readdressed. If so, then the common initialize method needs to call a class specific method for additional initialization. moved Collection autoload block to the bottom of Attrio

coveralls · 2013-06-29T20:36:04Z

Coverage remained the same when pulling 0bbf141 on anithri:collection_proposal into dda5c73 on jetrockets:master.

coveralls · 2013-06-29T20:38:45Z

Coverage remained the same when pulling 167d2cd on anithri:collection_proposal into 717bb55 on jetrockets:master.

coveralls · 2013-06-29T20:41:14Z

Coverage remained the same when pulling 0b1fe1a on anithri:collection_proposal into 717bb55 on jetrockets:master.

fixed a bug in hash#initialize

coveralls · 2013-06-29T21:17:31Z

Coverage remained the same when pulling 90975f8 on anithri:collection_proposal into 717bb55 on jetrockets:master.

Earlier I was working thinking that Array.Hash.Set were being used as a parallel to Attribute, when actually those should be used as the instance values of a collection, and that we needed Collection as a class to be parallel to Attribute. With that new understanding, I changed Collectable back to Collection, and have been building that class up to do it's job. Followed the lead of Attribute and so have no specs yet, though there isn't anything very complicated in that class. Hash is the first of the collections that I've started to flesh out. the type_cast method shoul be factored out into a Collections::Base class which we can also use to set the interface with the always popular NotImplementedError. The key_method works well for providing a unique value to use for the key of the hash. There seems to be a use case for some functionality around attempting to add_elements. Both in raising errors when a key is used again (redefining a value) and when a value can't be coerced into the correct type. If i wanted to add the ability to ignore values that can't be cast, I'd put in a ignore_type_error option check and add in a catch clause to catch TypeErrors then continue instead of reraiseing. If I wanted to add the ability to raise errors when reusing a key, I'd add a raise_on_duplicate option, and put in a check on the key value before the new value is assigned. There are a few gotcha's with the way DelegateClass works, and so I have kind_of? overridden, but I'm not sure that's needed. I'm sure when I implement Array and Set it'll become clear what needs to be done. While all specs are passing, this commit is not complete by itself.

coveralls · 2013-06-30T00:55:55Z

Coverage remained the same when pulling 96b1751 on anithri:collection_proposal into 717bb55 on jetrockets:master.

coveralls · 2013-06-30T01:21:44Z

Coverage remained the same when pulling 0429aa4 on anithri:collection_proposal into 717bb55 on jetrockets:master.

There is the potential for adding more advanced find methods. Don't think they are needed, but they are there. for a collection the default value is going to be the return value of find_element when no element was actually found. Users can use this to provide useful defaults such as an appropriate Null class. Should we provide an :initial_values options to populate the initial collection? Next up is to finish the CollectionBuilder and Collection and then to test integration

coveralls · 2013-06-30T02:52:09Z

Coverage remained the same when pulling 9662ef6 on anithri:collection_proposal into 717bb55 on jetrockets:master.

Changed Collections to use SimpleDelegator instead of ClassDelegator updated specs to use .create_collection

coveralls · 2013-07-01T20:41:04Z

Coverage remained the same when pulling cab66db on anithri:collection_proposal into 717bb55 on jetrockets:master.

anithri · 2013-07-01T22:59:28Z

Alright, things are getting close now. Have a few questions though.

should both attributes and collections go into the :as/attributes hash? If not we need a different option to rename collections to.
DefaultValue has 2 different meanings for collections: 1 is to use as a return value when no element is found using #find_element (Call it :not_found ?) and the other is to initialize the collection with (call it :initial_values ?)
Should we put in any functionality for the following?

#add_element can take a single value or an array of values. Currently if 1 element of the array is the wrong Type, a TypeError gets raised and and #add_element is a partial success, all elements before the wrong type are added, none of the ones after are. we could pretty easily do all the Type checks before any got added, we could also add an option to :skip_type_errors to just skip the incorrect values. either way this adds some complexity.
when #add_element is used in a Hash, new elements added that use an existing index will succeed, overwriting the existing value. Should we add a check to raise an error when :raise_on_redefine?
in your builders, you use: a unless klass.method_defined?(options[:method_name]) block, I prefer to use a guard clause like: return if klass.method_defined?(options[:method_name]). But I don't want to change your style without asking.
#find_element finds the first element in an Array (and the only element in a Set). Expanding #find_element offers the chance for multiple matches and thus returning an Array of results instead of a single one. Should there be a sear
#find_element is pretty bare bones. I like it for Hashes because it already accounts for the proper index function. it could be easily extended to also take a hash, where the keys of the hash are methods to call for each element and the value is the value(s) you want the results of those methods to match. This makes #find_element much more useful for Sets and Hashes. my_obj.find_users(last_name: ['Jones','Smith','Doe'], first_name: "John") Maybe use a search_elements for this instead and have it always return an array.

I still need to add integration tests to test the whole thing top to bottom.

anithri · 2013-07-07T21:58:01Z

Ok after some more code reading and thought, this is the next set of stuff that I'm going to implement.

:collections_as options

:collections_as alias(:c_as) will be the option name to set the name of the collections hash, defaults to "collections"

obj.reset_collections(collections_to_reset) to empty all collections in the collections_to_reset array

obj.reset_collections_initial_values(collections_to_reset) to set collections in the collections_to_reset array to empty, and then filled with initial values

In addition, I'm going to differ from the attributes and make the collections hash a first class object, move most of the functionality for reset and initialize into the object and use dispatch to connect the instances. If it works out, I'd say that the attributes hash could be similarly refactored.

This separation offers some nice opportunities. They could be used as the basis of a plugin system which allows the possibility to add depth in the way classes are created, marshalled, stored, initiliaze...

:unfound_value option

:unfound_value alias(:unfound) will be the option that sets the return value of the find_ method when no matching element is found. This could be used to return a null object or a default value.

:initial_value option

:initial_value alias(:initial_values,:initial) will be the option that takes the values used to initialize the collection. This can be a single value or an array of elements to add.

This code is not tested yet. Readme changes are examples of using Collections. attributes_parser had collection, create_collections, and fetch_container_type methods added. collection is a class parallel to Attribute for use with collections.

…ute. I used Attrio::AttributeBase as the module name for the shared methods, but I'm not happy with that name and when I come up with a better name, I'll rename it. I left the writer methods in Attrio::Attribute since only Attribute is going to use those methods. Factoring those into another module is something to keep in mind.

I used the DelgateClass as the basis for inheritence because it offers an easy way to wrap and extend classes while delgating all other calls to the original class. Specs are very simple, just testing creation and that they are acting a little like their delgate.

…ble modules moved attr_reader into Readable module. removed InstanceMethods module from Readable decided not to define initialize in a module. This could be readdressed. If so, then the common initialize method needs to call a class specific method for additional initialization. moved Collection autoload block to the bottom of Attrio

fixed a bug in hash#initialize

Earlier I was working thinking that Array.Hash.Set were being used as a parallel to Attribute, when actually those should be used as the instance values of a collection, and that we needed Collection as a class to be parallel to Attribute. With that new understanding, I changed Collectable back to Collection, and have been building that class up to do it's job. Followed the lead of Attribute and so have no specs yet, though there isn't anything very complicated in that class. Hash is the first of the collections that I've started to flesh out. the type_cast method shoul be factored out into a Collections::Base class which we can also use to set the interface with the always popular NotImplementedError. The key_method works well for providing a unique value to use for the key of the hash. There seems to be a use case for some functionality around attempting to add_elements. Both in raising errors when a key is used again (redefining a value) and when a value can't be coerced into the correct type. If i wanted to add the ability to ignore values that can't be cast, I'd put in a ignore_type_error option check and add in a catch clause to catch TypeErrors then continue instead of reraiseing. If I wanted to add the ability to raise errors when reusing a key, I'd add a raise_on_duplicate option, and put in a check on the key value before the new value is assigned. There are a few gotcha's with the way DelegateClass works, and so I have kind_of? overridden, but I'm not sure that's needed. I'm sure when I implement Array and Set it'll become clear what needs to be done. While all specs are passing, this commit is not complete by itself.

There is the potential for adding more advanced find methods. Don't think they are needed, but they are there. for a collection the default value is going to be the return value of find_element when no element was actually found. Users can use this to provide useful defaults such as an appropriate Null class. Should we provide an :initial_values options to populate the initial collection? Next up is to finish the CollectionBuilder and Collection and then to test integration

Changed Collections to use SimpleDelegator instead of ClassDelegator updated specs to use .create_collection

…collection_proposal

coveralls · 2013-07-08T20:19:52Z

Coverage decreased (-0%) when pulling 04b14f5 on anithri:collection_proposal into 717bb55 on jetrockets:master.

We prefer :c_as, but will use :collections_as if present attrio.rb changed to include collection hash and methods added AttributesParser#add_collection refactored collection to use #add_collection Extended Reset to have a #define_attrio_collection_reset method that defines :reset_collections, :initialize_collections, :empty_collections all of which call a method on each collection Extended CollectionBuilder to delegate to reset_collection, empty_collection, and initialize_collection on the object. Refactored calls into a set_delegate and define_delegate methods Extended Collection to have methods for _name and _visibility methods for reset, empty and initialize collections. This is ripe for a refactor, maybe setting up method_missing and crunching the bits into a Constant hash using the keys as the base and a string format as the value Refactored Array, Set, and Hash to use __mk_empty_collection__ in initialize and empty_collection methods Refactored the type check into #__normalize_value__ method that handles calls for default values and type_cast everything else. Extended Inspect to include collections replaced all mocks with doubles replaced all stubs with doubles cleaned up a few TODOs

fixed type in set_spec altered common to return self from #reset_collection, empty_collection, and #initialize_collection

coveralls · 2013-07-11T19:00:03Z

Coverage remained the same when pulling a26ba2e on anithri:collection_proposal into 717bb55 on jetrockets:master.

added values.flatten! to all #add_element(values)

coveralls · 2013-07-11T19:23:05Z

Coverage remained the same when pulling ebfa661 on anithri:collection_proposal into 717bb55 on jetrockets:master.

anithri · 2013-07-11T21:14:29Z

Ok, This now contains a working implementation of collections with working and passing specs. There are some small changes to be made, mostly to match coding styles (I tend to use the {test: "Me"} way of doing hashes while you use {:test => "Me"}). Also it's possible you have more apt names for some of these classes or methods.

I invite all comments, suggestions, changes.... Let me know what you think.

thanks!

coveralls · 2013-07-11T21:36:56Z

Coverage increased (+0%) when pulling 8aaf8b3 on anithri:collection_proposal into 717bb55 on jetrockets:master.

igor-alexandrov · 2013-08-26T21:05:43Z

So...
I see a lot of work done here. Have you seen our light implementation of collections in Attrio? I am thinking about some combination of your approach and our.

anithri added 2 commits June 28, 2013 16:15

anithri added 2 commits June 29, 2013 13:02

Renamed AttributeBase to Readable

17046aa

Removing a mistyped line

0b1fe1a

Added a spec for Readable, small bug fix and formatting changes

90975f8

fixed a bug in hash#initialize

anithri added 3 commits June 29, 2013 19:09

renamed collection directory to collections to match module name

9c7ba93

Refactor added Attrio::Collections::Common changed collections to use it

fcd8006

Forgot to add specs to last changeset

0429aa4

Refactored Collections to use .create_collection factory method

cab66db

Changed Collections to use SimpleDelegator instead of ClassDelegator updated specs to use .create_collection

anithri added 4 commits July 8, 2013 14:13

Renamed AttributeBase to Readable

795a1f0

anithri added 10 commits July 8, 2013 14:13

Removing a mistyped line

a4048df

Added a spec for Readable, small bug fix and formatting changes

5e09954

fixed a bug in hash#initialize

renamed collection directory to collections to match module name

1ea1b7e

Refactor added Attrio::Collections::Common changed collections to use it

310a6f4

Forgot to add specs to last changeset

1b65b85

Refactored Collections to use .create_collection factory method

0a0c2d6

Changed Collections to use SimpleDelegator instead of ClassDelegator updated specs to use .create_collection

Merge branch 'collection_proposal' of github.com:anithri/attrio into …

04b14f5

…collection_proposal

anithri added 2 commits July 9, 2013 21:00

Added spec for Attrio::Collections::Common

a26ba2e

fixed type in set_spec altered common to return self from #reset_collection, empty_collection, and #initialize_collection

anithri added 2 commits July 11, 2013 13:01

Added collections into embedded_value_spec

6a2095f

added values.flatten! to all #add_element(values)

Added collections into inheritance_spec

ebfa661

Updated README.md to reflect usage of collections

8aaf8b3

anithri closed this by deleting the head repository Jul 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

This is a sampling of code meant to demonstrate collections for attrio. #5

This is a sampling of code meant to demonstrate collections for attrio. #5

anithri commented Jun 25, 2013

coveralls commented Jun 25, 2013

igor-alexandrov commented Jun 26, 2013

coveralls commented Jun 28, 2013

coveralls commented Jun 29, 2013

coveralls commented Jun 29, 2013

coveralls commented Jun 29, 2013

coveralls commented Jun 29, 2013

coveralls commented Jun 30, 2013

coveralls commented Jun 30, 2013

coveralls commented Jun 30, 2013

coveralls commented Jul 1, 2013

anithri commented Jul 1, 2013

anithri commented Jul 7, 2013

coveralls commented Jul 8, 2013

coveralls commented Jul 11, 2013

coveralls commented Jul 11, 2013

anithri commented Jul 11, 2013

coveralls commented Jul 11, 2013

igor-alexandrov commented Aug 26, 2013

This is a sampling of code meant to demonstrate collections for attrio. #5

This is a sampling of code meant to demonstrate collections for attrio. #5

Conversation

anithri commented Jun 25, 2013

coveralls commented Jun 25, 2013

igor-alexandrov commented Jun 26, 2013

coveralls commented Jun 28, 2013

coveralls commented Jun 29, 2013

coveralls commented Jun 29, 2013

coveralls commented Jun 29, 2013

coveralls commented Jun 29, 2013

coveralls commented Jun 30, 2013

coveralls commented Jun 30, 2013

coveralls commented Jun 30, 2013

coveralls commented Jul 1, 2013

anithri commented Jul 1, 2013

anithri commented Jul 7, 2013

:collections_as options

:unfound_value option

:initial_value option

coveralls commented Jul 8, 2013

coveralls commented Jul 11, 2013

coveralls commented Jul 11, 2013

anithri commented Jul 11, 2013

coveralls commented Jul 11, 2013

igor-alexandrov commented Aug 26, 2013