Another day, another code review. Today I came across a function that generates non-meaningful XML. This is something that at first glance feels like it has a nice structure to it, but there’s a better way. First, let take a look at the XML (I’ve changed the XML and the code to make it anonymous, but preserved the idea; the original had several distinct param elements):
<search-criteria id="1"> <param key="keywords">food</param> <param key="project-name">Important Work</param> <param key="date">2011-12-31</param> </search-criteria>
As you can see, the function is creating generic element names (“param”) with an attribute designed to give it meaning. And here’s the function:
declare function local:create-search-criteria($params as map:map) as element(search-criteria)
{
let $form-elements := (
"project-name",
"date"
)
let $keywords as xs:string := (map:get($params, "keyword"), "")[1]
let $grant-id as xs:unsignedLong := local:get-next-id()
return
<search-criteria id="{ $grant-id }">
<param key="keywords">{ $keywords }</param>
{
for $key in $form-elements
let $value := map:get($params, $key)
return
<param key="{ $key }">{ $value }</param>
}
</search-criteria>
};
Why Not?
It’s certainly fair to ask, “Why not do it this way?” After all, we can still use XPath to select a specific part of the XML. For instance:
local:create-search-criteria($params)/param[@key = "date"]
That’s good enough to get the date parameter element.
While that’s true, I can think of two arguments against doing it this way, one specific to MarkLogic, one that is perhaps a purist’s argument.
Meaningful Structure
The first reason I would avoid this structure is simply that there is a way to express it that is more concise without being unclear. As with writing, if you can say the same thing more briefly without losing clarity, then it is probably a good idea. In this situation, the param element name does not contribute to our understanding of the content.
Indexes
In MarkLogic, there is no way to set up a range element index that is dependent on the value of an attribute. For instance, we could not set up a range element index on param that only had the values where the key parameter is “date”. A range element attribute index doesn’t solve the problem, because that indexes the values in the attribute itself.
Revised
Let’s start with how we might like the XML to look:
<search-criteria id="1"> <keyword>food</keyword> <project-name>Important Work</project-name> <date>2011-12-31</date> </search-criteria>
Now our structure is simple and concise and it will be easy to set up some useful indexes.
For the function, we can let the $param map govern the output structure.
declare function local:create-search-criteria($params as map:map) as element(search-criteria)
{
<search-criteria id="{ local:get-next-id() }">
{
for $key in map:keys($params)
return element { $key } { map:get($params, $key) }
}
</search-criteria>
};
Or if we prefer to control which fields get pulled out of the map, that’s a simple matter of setting up a sequence, similar to the original.
declare function local:create-search-criteria($params as map:map) as element(search-criteria)
{
<search-criteria id="{ local:get-next-id() }">
{
for $key in ("keyword", "project", "date")
return element { $key } { map:get($params, $key) }
}
</search-criteria>
};
Tags: anti-pattern, marklogic, xquery