Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

One Massive.cs file - for all supported db's. #164

Closed
jthope opened this Issue Dec 3, 2012 · 7 comments

Comments

Projects
None yet
3 participants
Contributor

jthope commented Dec 3, 2012

The differences between the massive files are starting to grow. Some of these changes are not related to TSQL generation, and it appears that they are not always merged to the other files. Is there any interest in going to a single file, that works for all supported databases (MSSQL, Oracle, Postgres, SqlLite)? If so, I would be interested in contributing to this effort as I work with MSSQL, Oracle, and Postgres on a regular basis.

Contributor

CreepyGnome commented Dec 19, 2012

First I think you really mean making one DynamicModel class to rule them all and not really a single file. As the ObjectExtensions class is identical except for one formatting a parameter name in Oracle. The DB class is identical on all and isn't even in the SQLite file.

So out of curiousness since you stated the differences are starting it seems to make sense to have them be separate files. At best maybe a base class to contain what is safe and centralizes duplicate code, and I think trying to make one "class" to handle all database implementations would introduce a lot of branching logic which would make the code too complex.

However even a base class creates a dependency that may not be wanted by all who use Massive.

Contributor

jthope commented Dec 19, 2012

I really mean one Massive.cs file. Depending on what provider is being used, will result in the specific SQL to generate. So maybe the Massive.cs class will have some helper classes for mssql, oracle, etc, that will just hold strings for their specific syntax. These classes will be used internally by massive, not the end user.

For example, this change 32b0dee, would no longer need to be merged into the other files. I also see more differences in the ObjectExtensions class than just a parameter name.

I don't see this adding much complexity, just an abstraction of SQL strings.

I just thought it would be easier to maintain new features being added by the community. And my use case may be a bit different than most (using oracle, mssql, and postgre daily). So doing something like this may not be worth the slight addition in complexity. But I still wanted to check with the contributors.

@jthope jthope closed this Dec 19, 2012

Contributor

CreepyGnome commented Dec 19, 2012

Hey you didn't have to close the issue, I am not the maintainer I am just a user with my own fork. Rob would have to make the call on the direction of his release.

If you want one Massive.cs file then like I said you can have that today. Just move all the name spaces into this one file. There is no "Massive" class the Massive.cs file is just a single file that has 3 classes in it (2 for SQLite). What you are asking really sounds like you want the DynamicModel class to be smarter and know how to talk to all the databases you listed. It can be done of course the question is should it be done. My opinion really is no, and that is my opinion. Rob may have his own, as other may as well.

You offered to contribute, and if you really want this feature fork Rob's Massive and go to town. You can submit a pull request back when you are done, or you can just keep your own fork for your own use. If others like it they will use your version.

Contributor

jthope commented Dec 19, 2012

Yes, I meant Massive.cs *file, not class. This can always be revisited if there becomes a need for it. Otherwise, it doesn't have to sit as an open issue.

Contributor

CreepyGnome commented Dec 19, 2012

Sorry if it feels like I am kicking a dead horse here, but how are you blocked form having a single Massive.cs FILE currently? Have you actually used Massive in a project or even in a test project?

Your insistence on FILE not CLASS is confusing to me as there is nothing stopping you from merging them in to a single file without any changes to the actual code. As all the other files have their own namespace and therefore would not conflict anything. You would have one file that does exactly what the separate files do.

Please help me understand what your blocking issues are for doing this yourself in less than a minute?

Your statement:

... it appears that they are not always merged to the other files."

Isn't fixed just because they are in a single file. They would still be 4 separate DynamicModel classes in it inside 4 separate namespaces. Someone would have to know all databases and merge the functionality into each one. Simple bug fixes are usually easy, however new functionality can be more difficult to enable in all versions of DynamicModel.

Also your statement:

Depending on what provider is being used, will result in the specific SQL to generate. So maybe the Massive.cs class will have some helper classes for mssql, oracle, etc, that will just hold strings for their specific syntax. These classes will be used internally by massive, not the end user.

You call out a Massive class which DOES NOT EXIST. Massive is the name of the project and the root name of the Namespaces and that is it. There is not a class called Massive. This is why I keep trying to understand why you are insistent on a single FILE as opposed to a single CLASS.

This statement also seems to imply you want the DynamicModel class be able to detect the data source type and then adjust to make sure it talks correctly to that data source. DynamicModel and ObjectExtensions are the two class where all the magic happens. The DB class isn't used by either of those and is only provided as a convenience class that uses the second connection string in your config file and provides a static way to get a DynamicModel class. This is odd that it uses the second connection string and doesn't really make since as DynamicModel as a static Open method that takes the name of a connection string to use.

Anyway... I apologize and I am trying to be helpful. Its just that on the surface it seems like you don't actually use Massive, however when you look at your fork you have made changes to the Oracle version which Rob has accepted your pull requests for, which I would assume means you are using the Oracle version of Massive.

I'm just trying to understand, as your statements about wanting "Massive" to be a little smarter and do a little more work for you seem reasonable and make sense until you say the Massive class and single Massive File and not the DynamicModel class.

Contributor

jthope commented Dec 20, 2012

Sorry for the mix-up on incorrectly labeling classes and files. Too much multi-tasking.

What I thought would be neat to do:

  • 1 file
  • 1 namespace
  • 1 static ObjectExtensions class
  • 1 static DB class
  • 1 DynamicModel class

That would work for all databases. Massive would then generate the appropriate SQL depending on the connection string and provider.

And yes, I do use massive.

Contributor

robconery commented Dec 20, 2012

I've thought of this, the problem is that the use of Massive between different platforms varies heavily. For instance there are completely different data types in Postgres (arrays, JSON, etc) then in SQL and I want people to be free to goof around.

So far it hasn't been an issue - the maintainers of the various files contribute as needed and I pull the changes in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment