On Dec 28, 1:27 am, Alexey <[email protected]> wrote: > I am fixing bugs in fixtures.rb, and would like to propose some API > and internal code changes. > > 1. Make TestFixtures::fixture_table_names hold a hash? > > I would like to make the (undocumented?) class attribute > `TestFixtures::fixture_table_names` to hold a Hash of the form > > ``` > { "fixture_name" => :table_name, ... } > ``` > > similarly to `TestFixtures::fixture_class_names` holding a Hash of the > form > > ``` > { "fixture_name" => FixtureModelClass, ... } > ``` > > Currently `TestFixtures::fixture_table_names` holds an Array, which > does not seem very useful, seems inconsistent with > `fixture_class_names`, and is not apparently used anywhere except for > tests. > > 2. Models vs classes in variable and method names? > > I would also propose to rename `fixture_class_names` to > `fixture_model_names` and `set_fixture_classes` to > `set_fixture_models`: this seems more flexible and descriptive to me. > > 3. Make set_fixture_classes only accept classes? > > Currently both are acceptable: > > ``` > set_fixture_classes "admin/users" => Administrator, > :customers => "Client" > ``` > > Is there a situation where one cannot pass the class and can only pass > its name? > If `set_fixture_classes` would only accept classes, the code would be > somewhat simpler, i guess. > > 4. Standardize the fixture name in the form "admin/users" (rather > than :admin_users) and use it as fixtures's identifier, instead of > table name? > > It seems that originally the idea was to use the table name as the > unique identifier of the collections of fixtures intended to fill that > table, but now this seems all messed up, variable names like > `fixture_name`, `table_name`, `fixture_path` are used with different > contents and sometimes interchangeably. > Since the method `fixtures` accept their names in the form `"admin/ > users"`, i propose to use this short path to the fixture file as the > name and identifier of the collection of fixtures, and use > `fixture_name` only in this sense. > The table name should be defined in the associated model, or by a new > method `set_fixture_tables` analogous to `set_fixture_classes`, and > only when there is no associated model, or the associated model does > not respond to `table_name` method and the table name was not set with > `set_fixture_tables`, should the table name be inferred from fixture > name by substitution `tr('/','_')`. > > 5. Rename Fixture to FixtureInstance and Fixtures to > FixtureCollection? > > It took me some time to understand that `ActiveRecord::Fixtures` is > used for the contents of fixture files, and `ActiveRecord::Fixture` is > for a single record. > How about renaming `Fixture` to `FixtureInstance`? > I do not think this class is even documented, so shouldn't be a > problem. > Also, maybe 'FixtureCollection' would be better than `Fixtures`? > > It is also not clear to me why `ActiveRecord::Fixture` is needed: > isn't it enough that each entry of `ActiveRecord::Fixtures` can be > converted to a hash or to an instance of the associated model? > > What do you think? > > Alexey.
I forgot one more: 6. Rename ActiveRecord::TestFixtures.fixture_path to ActiveRecord::TestFixtures.fixtures_directory? `ActiveRecord::TestFixtures.fixture_path` holds the path to the fixture files directory, and is sometimes stored in `fixtures_directory` variable elsewhere (in `ActiveRecord::Fixtures.create_fixtures`), and `fixture_path` is often used for the path to a single fixture file. There seem also to be confusion with the use `fixture_path` and `fixtures_directory` local variables. A. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
