Skip to content

Commit

Permalink
Fix linting issues
Browse files Browse the repository at this point in the history
  • Loading branch information
AlbertoEAF committed Feb 20, 2021
1 parent 6aa5e97 commit eff87d9
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 63 deletions.
2 changes: 1 addition & 1 deletion .ci/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ if [[ $TASK == "lint" ]]; then
echo "Linting R code"
Rscript ${BUILD_DIRECTORY}/.ci/lint_r_code.R ${BUILD_DIRECTORY} || exit -1
echo "Linting C++ code"
cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length --recursive ./src ./include ./R-package || exit -1
cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length --recursive ./src ./include ./R-package ./swig || exit -1
exit 0
fi

Expand Down
29 changes: 17 additions & 12 deletions swig/ChunkedArray.hpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
/*!
* Copyright (c) 2021 Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE file in the project root for license information.
*
* Author: Alberto Ferreira
*/
#ifndef __CHUNKED_ARRAY_H__
#define __CHUNKED_ARRAY_H__

Expand Down Expand Up @@ -50,8 +56,8 @@
*/
template <class T>
class ChunkedArray {
public:
ChunkedArray(size_t chunk_size)
public:
explicit ChunkedArray(size_t chunk_size)
: _chunk_size(chunk_size), _last_chunk_idx(0), _last_idx_in_last_chunk(0) {
new_chunk();
}
Expand All @@ -73,7 +79,7 @@ class ChunkedArray {
_last_idx_in_last_chunk = 0;
}

assert (setitem(_last_chunk_idx, _last_idx_in_last_chunk, value) == 0);
assert(setitem(_last_chunk_idx, _last_idx_in_last_chunk, value) == 0);
++_last_idx_in_last_chunk;
}

Expand Down Expand Up @@ -141,16 +147,16 @@ class ChunkedArray {

// Copy full chunks:
size_t i = 0;
for(size_t chunk = 0; chunk < full_chunks; ++chunk) {
for (size_t chunk = 0; chunk < full_chunks; ++chunk) {
T* chunk_ptr = _chunks[chunk];
for(size_t chunk_pos = 0; chunk_pos < _chunk_size; ++chunk_pos) {
for (size_t chunk_pos = 0; chunk_pos < _chunk_size; ++chunk_pos) {
other[i++] = chunk_ptr[chunk_pos];
}
}
// Copy filled values from last chunk only:
const size_t last_chunk_elems = this->get_last_chunk_add_count();
T* chunk_ptr = _chunks[full_chunks];
for(size_t chunk_pos = 0; chunk_pos < last_chunk_elems; ++chunk_pos) {
for (size_t chunk_pos = 0; chunk_pos < last_chunk_elems; ++chunk_pos) {
other[i++] = chunk_ptr[chunk_pos];
}
}
Expand All @@ -164,7 +170,7 @@ class ChunkedArray {
*
* @return pointer or nullptr if index is out of bounds.
*/
T getitem(size_t chunk_index, size_t index_within_chunk, T on_fail_value) noexcept {
T getitem(size_t chunk_index, size_t index_within_chunk, T on_fail_value) const noexcept {
if (within_bounds(chunk_index, index_within_chunk))
return _chunks[chunk_index][index_within_chunk];
else
Expand All @@ -181,8 +187,7 @@ class ChunkedArray {
* @return 0 = success, -1 = out of bounds access.
*/
int setitem(size_t chunk_index, size_t index_within_chunk, T value) noexcept {
if (within_bounds(chunk_index, index_within_chunk))
{
if (within_bounds(chunk_index, index_within_chunk)) {
_chunks[chunk_index][index_within_chunk] = value;
return 0;
} else {
Expand Down Expand Up @@ -225,7 +230,7 @@ class ChunkedArray {
* @param index_within_chunk index within that chunk
* @return true if that chunk is already allocated and index_within_chunk < chunk size.
*/
inline bool within_bounds(size_t chunk_index, size_t index_within_chunk) {
inline bool within_bounds(size_t chunk_index, size_t index_within_chunk) const {
return (chunk_index < _chunks.size()) && (index_within_chunk < _chunk_size);
}

Expand All @@ -238,11 +243,11 @@ class ChunkedArray {
// Check memory allocation success:
if (!_chunks[_chunks.size()-1]) {
release();
throw std::bad_alloc("Couldn't add more chunks to ChunkedArray. Released memory!");
throw std::bad_alloc();
}
}

private:
private:
const size_t _chunk_size;
std::vector<T*> _chunks;

Expand Down
52 changes: 21 additions & 31 deletions swig/StringArray.hpp
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
/*!
* Copyright (c) 2020 Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE file in the project root for license information.
*
* Author: Alberto Ferreira
*/
#ifndef __STRING_ARRAY_H__
#define __STRING_ARRAY_H__

#include <algorithm>
#include <new>
#include <string>
#include <vector>
#include <algorithm>

/**
* Container that manages an array of fixed-length strings.
Expand All @@ -22,18 +25,15 @@
* The class also takes care of allocation of the underlying
* char* memory.
*/
class StringArray
{
public:
class StringArray {
public:
StringArray(size_t num_elements, size_t string_size)
: _string_size(string_size),
_array(num_elements + 1, nullptr)
{
_array(num_elements + 1, nullptr) {
_allocate_strings(num_elements, string_size);
}

~StringArray()
{
~StringArray() {
_release_strings();
}

Expand All @@ -43,8 +43,7 @@ class StringArray
*
* @return char** pointer to raw data (null-terminated).
*/
char **data() noexcept
{
char **data() noexcept {
return _array.data();
}

Expand All @@ -56,8 +55,7 @@ class StringArray
* @param index Index of the element to retrieve.
* @return pointer or nullptr if index is out of bounds.
*/
char *getitem(size_t index) noexcept
{
char *getitem(size_t index) noexcept {
if (_in_bounds(index))
return _array[index];
else
Expand All @@ -77,11 +75,9 @@ class StringArray
* into the target string (_string_size), it errors out
* and returns -1.
*/
int setitem(size_t index, std::string content) noexcept
{
if (_in_bounds(index) && content.size() < _string_size)
{
std::strcpy(_array[index], content.c_str());
int setitem(size_t index, const std::string &content) noexcept {
if (_in_bounds(index) && content.size() < _string_size) {
std::strcpy(_array[index], content.c_str()); // NOLINT
return 0;
} else {
return -1;
Expand All @@ -91,22 +87,19 @@ class StringArray
/**
* @return number of stored strings.
*/
size_t get_num_elements() noexcept
{
size_t get_num_elements() noexcept {
return _array.size() - 1;
}

private:

private:
/**
* Returns true if and only if within bounds.
* Notice that it excludes the last element of _array (NULL).
*
* @param index index of the element
* @return bool true if within bounds
*/
bool _in_bounds(size_t index) noexcept
{
bool _in_bounds(size_t index) noexcept {
return index < get_num_elements();
}

Expand All @@ -120,15 +113,13 @@ class StringArray
* @param num_elements Number of strings to store in the array.
* @param string_size The size of each string in the array.
*/
void _allocate_strings(size_t num_elements, size_t string_size)
{
for (size_t i = 0; i < num_elements; ++i)
{
void _allocate_strings(size_t num_elements, size_t string_size) {
for (size_t i = 0; i < num_elements; ++i) {
// Leave space for \0 terminator:
_array[i] = new (std::nothrow) char[string_size + 1];

// Check memory allocation:
if (! _array[i]) {
if (!_array[i]) {
_release_strings();
throw std::bad_alloc();
}
Expand All @@ -138,13 +129,12 @@ class StringArray
/**
* Deletes the allocated strings.
*/
void _release_strings() noexcept
{
void _release_strings() noexcept {
std::for_each(_array.begin(), _array.end(), [](char* c) { delete[] c; });
}

const size_t _string_size;
std::vector<char*> _array;
};

#endif // __STRING_ARRAY_H__
#endif // __STRING_ARRAY_H__
44 changes: 25 additions & 19 deletions swig/test_ChunkedArray_manually.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
/*!
* Copyright (c) 2021 Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE file in the project root for license information.
*
* Author: Alberto Ferreira
*/
/**
* Tests for ChunkedArray.
*
Expand All @@ -12,18 +18,19 @@
#include <sstream>

#include "ChunkedArray.hpp"
using namespace std;
using std::cout;
using std::endl;

using intChunkedArray=ChunkedArray<int>;
using intChunkedArray = ChunkedArray<int>;
using doubleChunkedArray = ChunkedArray<double>;

// Test data
const int out_of_bounds = 4; // test get outside bounds.
const int out_of_bounds = 4; // test get outside bounds.
const size_t chunk_size = 3;
const std::vector<int> ref = {1, 2, 3, 4, 5, 6, 7};

template<class T>
size_t _get_merged_array_size(ChunkedArray<T> &ca) {
size_t _get_merged_array_size(const ChunkedArray<T> &ca) {
if (ca.empty()) {
return 0;
} else {
Expand All @@ -33,7 +40,7 @@ size_t _get_merged_array_size(ChunkedArray<T> &ca) {
}

template<class T>
void print_container_stats(ChunkedArray<T> &ca) {
void print_container_stats(const ChunkedArray<T> &ca) {
printf("\n\nContainer stats: %ld chunks of size %ld with %ld item(s) on last chunk (#elements=%ld).\n"
" > Should result in single array of size %ld.\n\n",
ca.get_chunks_count(),
Expand All @@ -44,7 +51,7 @@ void print_container_stats(ChunkedArray<T> &ca) {
}

template <typename T>
void _print_chunked_data(ChunkedArray<T> &x, T** data, std::ostream &o = std::cout) {
void _print_chunked_data(const ChunkedArray<T> &x, T** data, std::ostream &o = std::cout) {
int chunk = 0;
int pos = 0;

Expand All @@ -61,27 +68,26 @@ void _print_chunked_data(ChunkedArray<T> &x, T** data, std::ostream &o = std::co
}

template <typename T>
void print_data(ChunkedArray<T> &x) {
T **data = x.data();
void print_data(ChunkedArray<T> *x) {
T **data = x->data();
cout << "Printing from T** data(): \n";
_print_chunked_data(x, data);
_print_chunked_data(*x, data);
cout << "\n^ Print complete ^\n";
}

template <typename T>
void print_void_data(ChunkedArray<T> &x) {
T **data = reinterpret_cast<T**>(x.data_as_void());
void print_void_data(ChunkedArray<T> *x) {
T **data = reinterpret_cast<T**>(x->data_as_void());
cout << "Printing from reinterpret_cast<T**>(data_as_void()):\n";
_print_chunked_data(x, data);
_print_chunked_data(*x, data);
cout << "\n^ Print complete ^\n";
}

template <typename T>
void print_ChunkedArray_contents(ChunkedArray<T> &ca) {
void print_ChunkedArray_contents(const ChunkedArray<T> &ca) {
int chunk = 0;
int pos = 0;
for (int i = 0; i < ca.get_add_count() + out_of_bounds; ++i) {

bool within_added = i < ca.get_add_count();
bool within_bounds = ca.within_bounds(chunk, pos);
cout << "@(" << chunk << "," << pos << ") = " << ca.getitem(chunk, pos, 10)
Expand Down Expand Up @@ -113,7 +119,7 @@ void test_coalesce_to(const intChunkedArray &ca, const std::vector<int> &ref) {
* that the data was stored correctly and with the correct memory layout.
*/
template <typename T>
void test_data_layout(ChunkedArray<T> &ca, const std::vector<T> &ref, bool data_as_void) {
void test_data_layout(ChunkedArray<T> *ca, const std::vector<T> &ref, bool data_as_void) {
std::stringstream ss, ss_ref;
T **data = data_as_void? reinterpret_cast<T**>(ca.data_as_void()) : ca.data();
// Dump each chunk represented by a line with elements split by space:
Expand Down Expand Up @@ -162,16 +168,16 @@ int main() {
test_coalesce_to(ca, ref);

// Test chunked data layout for retrieval:
test_data_layout<int>(ca, ref, false);
test_data_layout<int>(ca, ref, true);
test_data_layout<int>(&ca, ref, false);
test_data_layout<int>(&ca, ref, true);

test_clear();

// For manual verification - useful outputs //////////////////////////////////////
print_container_stats(ca);
print_ChunkedArray_contents(ca);
print_data<int>(ca);
print_void_data<int>(ca);
print_data<int>(&ca);
print_void_data<int>(&ca);
ca.release(); ca.release(); print_container_stats(ca); // Check double free behaviour.
cout << "Done!" << endl;
return 0;
Expand Down

0 comments on commit eff87d9

Please sign in to comment.