Suggestion to remove circular module dependency

[axiom-a]

There is a circular module dependency between traits.d and types.d that does not look like it needs to be there (the build system I use is very strict on this). The following patch removes it, but it may not be in line with the original intentions.

--git a/source/sqlbuilder/traits.d b/source/sqlbuilder/traits.d
index 47f07a6..2139c0f 100644
--- a/source/sqlbuilder/traits.d
+++ b/source/sqlbuilder/traits.d
@@ -336,16 +336,6 @@ template getAllowNullType(alias sym)
     alias getAllowNullType = result;
 }
 
-template getFetchType(T)
-{
-    static if(is(T == AllowNullType!Args, Args...))
-        alias getFetchType = T.type;
-    else static if(is(T == RowObj!U, U))
-        alias getFetchType = U;
-    else
-        alias getFetchType = T;
-}
-
 
 unittest
 {
diff --git a/source/sqlbuilder/types.d b/source/sqlbuilder/types.d
index 6ced72a..baf8021 100644
--- a/source/sqlbuilder/types.d
+++ b/source/sqlbuilder/types.d
@@ -1,8 +1,17 @@
 module sqlbuilder.types;
-import sqlbuilder.traits;
 
 @safe:
 
+template getFetchType(T)
+{
+    static if(is(T == AllowNullType!Args, Args...))
+        alias getFetchType = T.type;
+    else static if(is(T == RowObj!U, U))
+        alias getFetchType = U;
+    else
+        alias getFetchType = T;
+}
+
 enum Spec : char
 {
     none = '\0',

[schveiguy]

Hm... the point of traits is to ask questions about the types of things. The point of types is to define the types.

Now, it doesn't need to be really split this way, but I rather like the idea of the semantic separation. Of course, this means AllowNullType needs to be accessible to the trait. I dislike moving the trait to the types module, just to satisfy a build system.

Can you disclose what the build system this is, and why it is important? I am not 100% married to this split of the module semantics, and am certainly willing to either combine or compromise here. There was no "deep deliberation" in how to split up the symbols between modules. But D usually does pretty well at circular dependencies....

[axiom-a]

Yes - I felt that the patch might not align with your intentions. The build system is a custom one that enforces a strict dependency relationship (no circular dependencies), which eliminates a range of build issues. My codebase is a mix of D and c++, hence the build system approach. (I do not like dub!)
This patch was just a quick'n'dirty so that I could build your library and evaluate it - feel free to ignore it!

[schveiguy]

I'm thinking AllowNullType can actually be moved instead. I believe it's only used in std.traits to deliver a type that hints to the serializer that the database type could be null, but if it is, it should be set to a default value instead. So in essence, it's a type, but not exactly a type anyone would use other than part of the trait.

I will think about this some more, and see if I can eliminate the circular dependency.

[schveiguy]

To be honest, this part of the code was kind of hacky. What I was trying to do is reproduce some serialization mechanisms I had in a previous serializer project that did this kind of thing. I'm also open to suggestions on how this might be done better.