Question on idiomatic imports / package cardinality

Technical Discussion
1

In a web API I maintain I have my queries split out in their own package. I like how clean it is to have all of my SQL in a single package. However, I run in to a situation semi-frequently where I want to unmarshal filters/values in a handler, then pass them directly to my data layer. Due to cyclical imports, I can’t do this because my controller imports queries, and queries can’t then import controller. So, what I end up doing is something like this:

// package controllers
type PossibleFilters struct {
	Filter1 []int    `json:"filter1"`
	Filter2 []int    `json:"filter2"`
	Filter3 []int    `json:"filter3"`
}
func SomeHandler(w http.ResponseWriter, r *http.Request) {
	body, _ := ioutil.ReadAll(r.Body)
	var filters = PossibleFilters{}
	// Obviously in real life, handle possible errors...
	_ := json.Unmarshal(body, &filters)
	list := queries.RunQuery(filters.Filter1, filters.Filter2, filters.Filter3)
	sendResponseToClient(list, w)
}

… and then in my queries package:

// package queries
func RunQuery(filter1 []int, filter 2 []int, filter3 []int) []ReturnItem {
	// run query based on filters
}

This gets verbose when I have a route/query with a lot of filters. It would be nice if I could do this:

// package queries
func RunQuery(filters PossibleFilters) []ReturnItem {
	// run query based on filters
}

… but as I mentioned this results in cyclical import. What is the idiomatic way of accomplishing this? My thoughts are:

  1. Put filter structs in own package (models?). I like the simplicity of declaring the struct right before the handler that uses it (these tend to not see a ton of re-use due to the nature of this specific API so having it in one place is handy). However, this would solve my problem.
  2. Maybe I already have too many packages and should just collapse to a single package? As I mentioned, I do like the idea of keeping my queries in their own package and I’ve been able to re-use them in other packages. But, I could potentially cut down on packages and solve the problem.

I’m leaning toward #1 but I was wondering what other people are doing and what has worked best for you. And most importantly if there’s another option I haven’t considered.

2

Did you consider moving PossibleFilters into the queries package?

I only can judge from the given code snippets that might give me a wrong impression, but it looks as if the filters are logically related to queries. If the controllers package uses the queries package anyway, it can easily do var filters = queries.PossibleFilters{} at no extra cost.

2 Likes
3

That does make sense but my only hesitation there was: it felt slightly wrong to embed json tags in a query package that should, in theory, not care anything about whether this came from json or somewhere else. Ideally, I would like my queries package to not know/care about how it’s being consumed but I’m probably over-thinking it. Since it is a type related to filters in a query, I think having queries.PossibleFilters makes the most sense and I can just deal with having json tags in my queries package.

4

I agree, the struct tags would not belong to the query package.

Would a “wrapper” func inside controllers solve the problem?

package controllers
...
type PossibleFilters ...
...
func RunQuery(filters PossibleFilters) []ReturnItem {
	return queries.RunQuery(filter1 []int, filter2 []int, filter3 []int)
}

This way, all controllers-related stuff remains in controllers while the query internals stay outside controllers. And queries would not depend on controllers anymore.

5

I like adding another layer of indirection, but that method still suffers from the problem wherein every time I add a new possible filter, I have to modify multiple functions and the number of params eventually becomes silly looking (like in some cases, for reporting for example, I can have a LOT of possible types of filters). Plus, trying to DRY here.

6

I obviously lack the knowledge about the internal requirements of your design, but I notice that the struct contains filters 1,2,3 that are all of the same type. Did you only simplify the code for this thread, or is this the “real” code?

I am asking because I am curious why this array of filters is packed into a struct, rather than into a slice or map. Can the JSON deliver the filters as an array?

I am thinking of this:

A filter type and a modified RunQuery() in queries:
where RunQuery is defined as:

// package queries

type filter []int

func RunQuery(filters []filter) []ReturnItem {
	// run query based on filters
}

And the controllers package could look like this:

func SomeHandler(w http.ResponseWriter, r *http.Request) {
	body, _ := ioutil.ReadAll(r.Body)
	filters := []queries.filter
	_ := json.Unmarshal(body, &filters)
	list := queries.RunQuery(filters)
	sendResponseToClient(list, w)
}

This seems pretty DRY to me. And the JSON can deliver as many filters as it wants to. And there are no cyclic dependencies.

7

Yeah - filters can be of different types and I was mainly copying/pasting in that example function. Again - I think for now it makes sense to just put the structs in the query package. They are related to running queries so it makes sense to have them there. Of course the only thing I don’t love is json struct tags in a query class, but it seems like the lesser of evils in terms of the solutions.

8

Yes, sometimes a small compromise is better than the alternatives. Giving up the convenience of struct tags (which are the reason for the initial problem about separation of concern, DRY, and circular imports) would mean you’d probably have to write a custom unmarshaller instead. Or go for some other solution that involves considerable code change. Too much effort for a small outcome if you ask me… And this solution is not carved in stone - you can still refactor later. (Yeah I know, one more post-it in the backlog column…)

9

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.