HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppgitModerate

Date of latest GitHub project release

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
latestgithubdateprojectrelease

Problem

I needed a clean, simple and robust piece of code to retrieve the date of publication of the latest release of a given GitHub Project.

My constraints were that it needed to be written in the C++11 subset supported by Visual Studio 2012, and that it would require as few dependencies as possible.

Here is what I came up with. The code uses the Windows Internet API to query the GitHub REST API, and RapidJSON to parse the JSON response.

Here are a few specific questions:

  • Am I using the Windows Internet API correctly?



  • Will my code work in the presence of proxies or other exotic settings?



  • Is there a simpler way to parse the ISO date returned by GitHub and pretty-print it?



  • Is my usage of TCHAR and _T() correct? Or should I use wchar_t and L prefixes?



I would appreciate any other comment or suggestion on how to improve (or fix) this code.

```
#include "rapidjson/document.h"

#include
#include
#include

#include
#include
#include
#include

DWORD query_release_information(std::string& response)
{
response.clear();

HINTERNET session =
InternetOpen(
_T("appleseed"),
INTERNET_OPEN_TYPE_DIRECT,
nullptr,
nullptr,
0);
if (session == nullptr)
return GetLastError();

HINTERNET connection =
InternetConnect(
session,
_T("api.github.com"),
INTERNET_DEFAULT_HTTPS_PORT,
nullptr,
nullptr,
INTERNET_SERVICE_HTTP,
0,
0);
if (connection == nullptr)
{
const DWORD result = GetLastError();
InternetCloseHandle(session);
return result;
}

static const TCHAR* AcceptTypes[] = { _T("application/json"), nullptr };
HINTERNET request =
HttpOpenRequest(
connection,
_T("GET"),
_T("/repos/appleseedhq/appleseed-max/releases"),
nullptr,
nullptr,
AcceptTypes,

Solution

You're actually writing in C but using some C++ features, I'd drop C style (and functions) in favor of their C++ counterparts:

1) Even if you're using a C API you should write C++ classes to hide them. Ratchet already addressed this issue in his answer then I won't repeat again here.

2) You're using sscanf to parse a string, you should use std::stringstream instead or - better - directly use std::get_time to parse it.

3) You're formatting your date using std::strftime but starting with C++ 11 (but supported from VC2K10) you have a better option: std::put_time (better because, among other things, it will directly return a std::string). Note that this will also simplify code around that.

4) You have many magic strings, move them to const fields in your class (see also point 1).

Moreover:

5) InternetReadFile reads line by line and it returns ERROR_INSUFFICIENT_BUFFER if lpBuffer is not big enough. It means that if one single line exceeds 4096 bytes then your code will fail. You should handle this case (checking for ERROR_INSUFFICIENT_BUFFER error code) and reiterating reading.

6) You may get extended error information (with useful server's messages) using InternetGetLastResponseInfo(). Win32 error alone won't help you to exactly understand what went wrong server-side (which parameter is wrong, access errors, bad request and so on).

7) I'd also use INTERNET_FLAG_NO_UI for HttpOpenRequest in case your application will run unattended. To be honest I'd drop WinINet functions in favor of WinHTTP...

8) You're not checking rapidjson::Document::Parse() return value, errors may go unnoticed.

9) Your main() function is pretty big, split code into more specific functions: download_and_parse_release_information(), extract_publishing_date() and print_publishing_date(). Code itself (especially after moving to C++) is pretty simple but you'll start building a toolbox of reusable functions.

10) Next step is to refactor them into a proper reusable class github::project with proper methods (github::project::find_releases()) and structs (github::release).

Context

StackExchange Code Review Q#112330, answer score: 12

Revisions (0)

No revisions yet.