> I've just taken a quick peek, and have a few comments that you can keep > in mind during refactoring.
Thanks for taking this time. > * We don't use "_" prefixes anywhere else in the code, so please refrain > from that. Sure. > * I see you make one connection and share it. That is not how > connections are supposed to be used. It would be much better if you > inject a DataSource (implemented by a connection pool) into the > EntityStore, and then retrieve/return connections on each JDBC call. > Otherwise concurrency and error handling becomes a nightmare. I was planning doing exactly this. We will do it before moving the code out of sandbox. > * Consider not doing table management in the EntityStore. I'm using > LiquiBase in Streamflow for all table management, on an application > level, and it simplifies evolution a lot (which you can't handle in the > current setup). Let the EntityStore focus on one thing only. I'm not sure it's valuable here as the ES is really an infrastructure thing and the current implementation goal is to provide a _non-cutomizable_ db schema. Maybe we could provide external SQL code per db vendor to create the schema. WDYT ? > * Does the entity table include a version column? In order to do proper > concurrency checking you pretty much have to, and then use it on UPDATE > calls. Much like the OptimisticLock VERSION column used by Hibernate. Good idea. Will do. > * Don't replicate Activatable by doing startDatabase/stopDatabase. Just > let the service implement Activatable. Sure. > * What is the "PK" column for? Isn't it enough with the identity one? This idea is to have a PrimaryKey column generated by a sequence in the underlying database to prevent concurency issues and to keep the db index small and fast. Curently the PK is generated in java code until we sort out entitystore/indexing syncronism. I'll let Stanislav answer more precisely on this one if needed. > * Please use SLF4J for logging instead of System.out. Part of the code is already using slf4j, we will make sure it is everywhere. Paul _______________________________________________ qi4j-dev mailing list [email protected] http://lists.ops4j.org/mailman/listinfo/qi4j-dev
