Skip to content

Commit

Permalink
[swig] Refactor StringArray* with unique_ptr
Browse files Browse the repository at this point in the history
  • Loading branch information
alberto.ferreira committed Mar 8, 2020
1 parent cd0a3fa commit 9e74b99
Showing 1 changed file with 18 additions and 20 deletions.
38 changes: 18 additions & 20 deletions swig/StringArray_API_extensions.i
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
%apply char **STRING_ARRAY {char **StringArrayHandle_get_strings};

#include <iostream>
#include <memory>

%inline %{

Expand Down Expand Up @@ -225,21 +226,20 @@ class StringArray
int eval_counts,
StringArrayHandle *strings)
{
strings = nullptr;
std::unique_ptr<StringArray> strings_ptr(nullptr);

try {
// TODO: @reviewer, 128 was the chosen size, any particular reason for this constraint?
*strings = new StringArray(eval_counts, 128);
} catch (std::bad_alloc &e) {
*strings = nullptr;
LGBM_SetLastError("Failure to allocate memory."); // TODO: @reviewer is not setting the message.
strings_ptr.reset(new StringArray(eval_counts, 128));
} catch (std::bad_alloc &e) {
LGBM_SetLastError("Failure to allocate memory."); // TODO: @reviewer - why is this not setting the message?
return -1;
}

int api_return = LGBM_BoosterGetEvalNames(handle, &eval_counts,
static_cast<StringArray*>(*strings)->data());
if (api_return == -1) {
// Call failed, no point in returning memory to free later:
StringArrayHandle_free(*strings);
*strings = nullptr;
int api_return = LGBM_BoosterGetEvalNames(handle, &eval_counts, strings_ptr->data());
if (api_return == 0) {
*strings = strings_ptr.release();
}

return api_return;
Expand All @@ -259,32 +259,30 @@ class StringArray
int max_feature_name_size,
StringArrayHandle * strings)
{
std::unique_ptr<StringArray> strings_ptr(nullptr);
int api_return;

// 1) To preallocate memory extract number of features first:
int num_features;
api_return = LGBM_BoosterGetNumFeature(handle, &num_features);
if (api_return == -1)
if (api_return == -1) {
return -1;
}

// 2) Allocate an array of strings:
try {
// TODO: @reviewer should we also figure out the size of the biggest string and remove parameter max_feature_name_size?
*strings = new StringArray(num_features, max_feature_name_size);
strings_ptr.reset(new StringArray(num_features, max_feature_name_size));
} catch (std::bad_alloc &e) {
*strings = nullptr;
LGBM_SetLastError("Failure to allocate memory."); // TODO: @reviewer is not setting the message.
LGBM_SetLastError("Failure to allocate memory."); // TODO: @reviewer - why is this not setting the message?
return -1;
}

// 3) Extract feature names:
int _dummy_out_num_features; // already know how many there are (to allocate memory).
api_return = LGBM_BoosterGetFeatureNames(handle, &_dummy_out_num_features,
static_cast<StringArray*>(*strings)->data());
if (api_return == -1)
{
StringArrayHandle_free(*strings);
*strings = nullptr;
api_return = LGBM_BoosterGetFeatureNames(handle, &_dummy_out_num_features, strings_ptr->data());
if (api_return == 0) {
*strings = strings_ptr.release();
}

return api_return;
Expand Down

0 comments on commit 9e74b99

Please sign in to comment.