search

sebastienros / yessql

A .NET document database working on any RDBMS
MIT License
1.17k stars 195 forks source link

Improve readability of parameters in the recently changed SchemaBuilder interface #344

Open optiklab opened 3 years ago

optiklab commented 3 years ago

I'm learning Orchard CMS and using most recent versions of OrchardCore Preview. I tried to create and index migration logic with ISchemaBuilder.CreateMapIndexTable and ISchemaBuilder.AlterTable like it was done in the learning courses done by Lombiq. However, I noticed that list of parameters in ISchemaBuilder.CreateMapIndexTable was changed during refactoring #249.

It took a while for me to figure out how to correctly use new method together with AlterIndexTable. And most problematic thing was "collection" parameter that doesn't seem clear what is it for and how it works. I believe usual Orchard CMS developer would be happy to see some clear interface comments and parameter names.

Let's figure out the best way to improve that. Here are some ideas that I have:

  1. Collection parameter is actually just a part of a table and key name. So, let's rename "collection" to: "namePrefix", "typePrefix", "prefix" (not good as in case of foreign key it will be in the middle FK_collection_name).
  2. Collection parameter is actually optional and can be empty. But I pushed to send String.Empty anyway. So, let's define a default value for it (empty string or whatever).
  3. Let's add some comments clarifying behavior. Unfortunately, I can't suggest good comments as I'm not fully aware of the ideas behind.
sebastienros commented 3 years ago

Thanks for the feedback. This is a good start to get the thoughts going. If you want to provide some PRs for the missing documentations it doesn't need to be discussed.