On Fri, Jun 15, 2012 at 12:44 PM, Henrik Ingo <henrik.i...@avoinelama.fi> wrote:
> I didn't look at HTTPHandler yet. Will do that separately. Looks
> really good now!
http_handler.cc
handleRequest():
I don't know if this is official or just me, but for local variables,
don't use the leading underscore:
const char* _input_query;
Just do this:
const char* input_query;
When reading, I expect that member variables have the leading
underscore so this was confusing.
This check should be done also for the POST case:
if ( _input_query == NULL || strcmp(_input_query, "") == 0 )
{
_input_query = "{}";
}
You could delete that one and then do this after the if/else block:
if ( _query == NULL || strcmp(_query, "") == 0 )
{
_query = "{}";
}
Please use the same text for both error messages. Also, in the below
sentences "specified" is grammatically correct. Also, use "must" when
something is required (http://www.ietf.org/rfc/rfc2119.txt)
_json_out["error_message"]= "Table isn't specify in URI query string";
_http_response_text = "Table should be specify in URI query string.";
So in other words:
_json_out["error_message"]= _http_response_text = "table must
be specified in URI query string.";
(non-capital t in table is intentional.)
Again, Drizzle custom is to return false on success and true on error
(yeah, it's crazy...):
return false;
}
return true;
}
Did you switch all the other ones I pointed out in previous review?
Same in validateJson() :
return retval;
}
Change to "return !retval"; (Ok, at this point it's starting to not
make sense anymore. But consistency is important...)
Btw. Notice now how we have achieved a state where most of the code
that used to be copy-pasted around the 3 separate GET/POST/DELETE
functions is now actually shared code, and you have only small
branches for handling POST or GET separately in HTTPHandler and
SQLToJsonGenerator, and of course the separate branches in
SQLGenerator.
This means that in json_server.cc you can just put the code to call
HTTPHandler and DBAccess directly in
process_api02_json_req()
...and you can delete
process_api02_json_get_req()
process_api02_json_post_req()
process_api02_json_put_req()
process_api02_json_delete_req()
...which will be a major re-factoring WIN!
henrik
--
henrik.i...@avoinelama.fi
+358-40-8211286 skype: henrik.ingo irc: hingo
www.openlife.cc
My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : drizzle-discuss@lists.launchpad.net
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
