Skip to content

Commit

Permalink
Changed allocation of memory for arrays, in response to #23
Browse files Browse the repository at this point in the history
  • Loading branch information
TommyKaneko committed Feb 22, 2018
1 parent 5092224 commit ffc704c
Show file tree
Hide file tree
Showing 12 changed files with 209 additions and 177 deletions.
42 changes: 22 additions & 20 deletions SUAPI-CppWrapper/String.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "SUAPI-CppWrapper/String.hpp"

#include <cassert>
#include <vector>

namespace CW {

Expand All @@ -36,32 +37,31 @@ SUStringRef m_string;
String::String():
m_string(create_string_ref()),
m_encoding(StringEncoding::UTF8)
{
}
{}


String::String(SUStringRef string_ref):
m_string(string_ref),
m_encoding(StringEncoding::UTF8)
{
}
{}


String::String(const std::string &string_input, StringEncoding enc):
m_string(create_string_ref(string_input, enc)),
m_encoding(enc)
{
}
{}


String::String(const char string_input[]):
m_string(create_string_ref(&string_input[0])),
m_encoding(StringEncoding::UTF8)
{
}
{}


String::String(const unichar string_input[]):
m_string(create_string_ref(&string_input[0])),
m_encoding(StringEncoding::UTF16)
{
}
{}


String::String(const String& other):
Expand Down Expand Up @@ -104,6 +104,7 @@ SUStringRef String::create_string_ref() {
return string_ref;
}


SUStringRef String::create_string_ref(std::string string_input, StringEncoding enc) {
SUStringRef string_ref = SU_INVALID;
if (enc == StringEncoding::UTF8) {
Expand All @@ -118,19 +119,21 @@ SUStringRef String::create_string_ref(std::string string_input, StringEncoding e
return string_ref;
}


SUStringRef String::create_string_ref(const char string_input[]) {
SUStringRef string_ref = SU_INVALID;
SUStringCreateFromUTF8(&string_ref, &string_input[0]);
return string_ref;
}


SUStringRef String::create_string_ref(const unichar string_input[]) {
SUStringRef string_ref = SU_INVALID;
// TODO UTF16 to be supported.
//SUStringCreateFromUTF16(&string_ref, &string_input[0]);
return string_ref;
}


String::~String() {
if (SUIsValid(m_string)) {
Expand All @@ -139,39 +142,38 @@ String::~String() {
}
}


SUStringRef String::ref() const {
return m_string;
}


std::string String::std_string() const {
size_t out_length = 0;
SUResult res = SUStringGetUTF8Length(m_string, &out_length);
assert(res == SU_ERROR_NONE);
out_length++; // Allow for null termianted string
char* char_array = new char[out_length];

This comment has been minimized.

Copy link
@thomthom

thomthom Feb 22, 2018

Contributor

Might be worth explicitly filling it with NULL; std::vector<char> char_array(out_length, 0);

This comment has been minimized.

Copy link
@TommyKaneko

TommyKaneko Feb 22, 2018

Author Owner

From what I understand from the standards, the default initialisation for char would be called if the default value is called. In this case, I think it is NULL.

This comment has been minimized.

Copy link
@thomthom

thomthom Feb 22, 2018

Contributor

Have you considered a unit test framework?
We use Googletest, works on both platforms.

This comment has been minimized.

Copy link
@thomthom

thomthom Feb 22, 2018

Contributor

I can set up a PR with a test unit project using GoogleTest if you want.

This comment has been minimized.

Copy link
@jimfoltz

jimfoltz Feb 22, 2018

Contributor

Yes please.

res = SUStringGetUTF8(m_string, out_length, &char_array[0], &out_length);
std::vector<char> char_array(out_length);
res = SUStringGetUTF8(m_string, out_length, char_array.data(), &out_length);
assert(res == SU_ERROR_NONE);
std::string str(char_array);
delete[] char_array;
std::string str(char_array.begin(),char_array.end());
return str;
}


String::operator std::string() const {
return std_string();
}

//char& String::operator [](size_t i) {

//}


size_t String::size() const {
size_t out_length = 0;
SUResult res = SUStringGetUTF8Length(m_string, &out_length);
assert(res == SU_ERROR_NONE);
return out_length;
}



bool String::empty() const {
// size of 1 is empty due to the \n character at the end.
if (size() == 1) {
Expand Down
22 changes: 10 additions & 12 deletions SUAPI-CppWrapper/model/AttributeDictionary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,18 +180,16 @@ std::vector<std::string> AttributeDictionary::get_keys() const {
size_t num_keys = 0;
SUResult res = SUAttributeDictionaryGetNumKeys(m_dict, &num_keys);
assert(res == SU_ERROR_NONE);
SUStringRef* keys_ref = new SUStringRef[num_keys];
for (size_t i=0; i < num_keys; i++) {
keys_ref[i] = SU_INVALID;
SUStringCreate(&keys_ref[i]);
}
SUAttributeDictionaryGetKeys(m_dict, num_keys, &keys_ref[0], &num_keys);
std::vector<std::string> keys;
keys.reserve(num_keys);
for (size_t i=0; i < num_keys; i++) {
keys.push_back(String(keys_ref[i]));
}
delete[] keys_ref;
std::vector<SUStringRef> keys_ref(num_keys, SU_INVALID);
//for (size_t i=0; i < num_keys; i++) {
// SUStringCreate(&keys_ref[i]);

This comment has been minimized.

Copy link
@thomthom

thomthom Feb 22, 2018

Contributor

It wasn't needed to call SUStringCreate? If not, any reason to keep the commented code around? (I'll be in source control history.)

This comment has been minimized.

Copy link
@TommyKaneko

TommyKaneko Feb 22, 2018

Author Owner

Actually, I am not sure why I removed SUStringCreate - should have tested it first. I must have had the TypedValueRef controversy in my head. I will test.

This comment has been minimized.

Copy link
@jimfoltz

jimfoltz Feb 22, 2018

Contributor

I get an exception without - the keys_ref element ptrs are all 0.

00511

This comment has been minimized.

Copy link
@TommyKaneko

TommyKaneko Feb 23, 2018

Author Owner

Ah, then the answer is to uncomment the lines that do SUStringCreate.

This comment has been minimized.

Copy link
@TommyKaneko

TommyKaneko Feb 23, 2018

Author Owner

fixed with 0f8ef5c

//}
SUAttributeDictionaryGetKeys(m_dict, num_keys, keys_ref.data(), &num_keys);
std::vector<std::string> keys(num_keys);
std::transform(keys_ref.begin(), keys_ref.end(), keys.begin(),
[](const SUStringRef& value) {
return String(value).std_string();
});
return keys;
}

Expand Down
9 changes: 7 additions & 2 deletions SUAPI-CppWrapper/model/ComponentInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,16 @@ SUComponentInstanceRef ComponentInstance::copy_reference(const ComponentInstance
/*****************************
* Constructors / Destructor **
******************************/
ComponentInstance::ComponentInstance():
DrawingElement(),
m_instance(SU_INVALID)
{}


ComponentInstance::ComponentInstance(SUComponentInstanceRef instance, bool attached):
DrawingElement(SUComponentInstanceToDrawingElement(instance), attached),
m_instance(instance)
{
}
{}


ComponentInstance::ComponentInstance(const ComponentInstance& other):
Expand Down
5 changes: 5 additions & 0 deletions SUAPI-CppWrapper/model/ComponentInstance.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ class ComponentInstance :public DrawingElement {
SUComponentInstanceRef copy_reference(const ComponentInstance& other);

public:
/**
* Constructor for null object
*/
ComponentInstance();

ComponentInstance(SUComponentInstanceRef instance, bool attached = true);

/** Copy constructor */
Expand Down
15 changes: 7 additions & 8 deletions SUAPI-CppWrapper/model/Curve.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,14 @@ std::vector<Edge> Curve::get_edges() const{
size_t num_edges = 0;
SUResult res = SUCurveGetNumEdges(m_curve, &num_edges);
assert(res == SU_ERROR_NONE);
SUEdgeRef* ref_edges = new SUEdgeRef[num_edges];
res = SUCurveGetEdges(m_curve, num_edges, &ref_edges[0], &num_edges);
std::vector<SUEdgeRef> ref_edges(num_edges);

This comment has been minimized.

Copy link
@thomthom

thomthom Feb 22, 2018

Contributor

Remember to initialize Ref objects to SU_INVALID.

std::vector<SUEdgeRef> ref_edges(num_edges, SU_INVALID);

res = SUCurveGetEdges(m_curve, num_edges, ref_edges.data(), &num_edges);
assert(res == SU_ERROR_NONE);
std::vector<Edge> edges;
edges.reserve(num_edges);
for (size_t i=0; i < num_edges; ++i) {
edges.push_back(Edge(ref_edges[i]));
}
delete ref_edges;
std::vector<Edge> edges(num_edges);
std::transform(ref_edges.begin(), ref_edges.end(), edges.begin(),
[](const SUEdgeRef& value) {
return Edge(value);
});
return edges;
}

Expand Down
17 changes: 8 additions & 9 deletions SUAPI-CppWrapper/model/Edge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,20 +200,19 @@ std::vector<Face> Edge::faces() const {
}
size_t count = 0;
SUResult res = SUEdgeGetNumFaces(m_edge, &count);
SUFaceRef* faces = new SUFaceRef[count];
assert(res == SU_ERROR_NONE);
if (count == 0) {
return std::vector<Face>();
}
res = SUEdgeGetFaces(m_edge, count, &faces[0], &count);
std::vector<SUFaceRef> face_refs(count, SU_INVALID);
res = SUEdgeGetFaces(m_edge, count, face_refs.data(), &count);
assert(res == SU_ERROR_NONE);
std::vector<Face> return_faces;
return_faces.reserve(count);
for (size_t i=0; i < count; ++i) {
return_faces.push_back(Face(faces[i]));
}
delete faces;
return return_faces;
std::vector<Face> faces(count);
std::transform(face_refs.begin(), face_refs.end(), faces.begin(),
[](const SUFaceRef& value) {
return Face(value);
});
return faces;
}

Vector3D Edge::vector() const {
Expand Down
45 changes: 21 additions & 24 deletions SUAPI-CppWrapper/model/Entities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,14 @@ std::vector<Face> Entities::faces() const {
if (count == 0) {
return std::vector<Face>(0);
}
SUFaceRef* face_refs = new SUFaceRef[count];
res = SUEntitiesGetFaces(m_entities, count, &face_refs[0], &count);
std::vector<SUFaceRef> face_refs(count, SU_INVALID);
res = SUEntitiesGetFaces(m_entities, count, face_refs.data(), &count);
assert(res == SU_ERROR_NONE);
std::vector<Face> faces;
faces.reserve(count);
for (size_t i=0; i < count; ++i) {
faces.push_back(Face(face_refs[i]));
}
delete[] face_refs;
std::vector<Face> faces(count);
std::transform(face_refs.begin(), face_refs.end(), faces.begin(),
[](const SUFaceRef& value) {
return Face(value);
});
return faces;
}

Expand All @@ -86,15 +85,14 @@ std::vector<Edge> Entities::edges(bool stray_only) const {
if (count == 0) {
return std::vector<Edge>(0);
}
SUEdgeRef* edge_refs = new SUEdgeRef[count];
res = SUEntitiesGetEdges(m_entities, stray_only, count, &edge_refs[0], &count);
std::vector<SUEdgeRef> edge_refs(count, SU_INVALID);
res = SUEntitiesGetEdges(m_entities, stray_only, count, edge_refs.data(), &count);
assert(res == SU_ERROR_NONE);
std::vector<Edge> edges;
edges.reserve(count);
for (size_t i=0; i < count; ++i) {
edges.push_back(Edge(edge_refs[i]));
}
delete[] edge_refs;
std::vector<Edge> edges(count);
std::transform(edge_refs.begin(), edge_refs.end(), edges.begin(),
[](const SUEdgeRef& value) {
return Edge(value);
});
return edges;
}

Expand All @@ -109,15 +107,14 @@ std::vector<ComponentInstance> Entities::instances() const {
if (count == 0) {
return std::vector<ComponentInstance>{};
}
SUComponentInstanceRef* instance_refs = new SUComponentInstanceRef[count];
res = SUEntitiesGetInstances(m_entities, count, &instance_refs[0], &count);
std::vector<SUComponentInstanceRef> instance_refs(count, SU_INVALID);
res = SUEntitiesGetInstances(m_entities, count, instance_refs.data(), &count);
assert(res == SU_ERROR_NONE);
std::vector<ComponentInstance> instances;
instances.reserve(count);
for (size_t i=0; i < count; ++i) {
instances.push_back(ComponentInstance(instance_refs[i]));
}
delete[] instance_refs;
std::vector<ComponentInstance> instances(count);
std::transform(instance_refs.begin(), instance_refs.end(), instances.begin(),
[](const SUComponentInstanceRef& value) {
return ComponentInstance(value);
});;
return instances;
}

Expand Down
12 changes: 6 additions & 6 deletions SUAPI-CppWrapper/model/Entity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,14 @@ std::vector<AttributeDictionary> Entity::attribute_dictionaries() const {
if (num_dicts == 0) {
return std::vector<AttributeDictionary>{};
}
SUAttributeDictionaryRef* dicts_ref = new SUAttributeDictionaryRef[num_dicts];
res = SUEntityGetAttributeDictionaries(m_entity, num_dicts, &dicts_ref[0], &num_dicts);
std::vector<SUAttributeDictionaryRef> dicts_ref(num_dicts, SU_INVALID);
res = SUEntityGetAttributeDictionaries(m_entity, num_dicts, dicts_ref.data(), &num_dicts);
assert(res == SU_ERROR_NONE);
std::vector<AttributeDictionary> dicts(num_dicts);
for (size_t i=0; i < num_dicts; i++) {
dicts[i] = AttributeDictionary(dicts_ref[i]);
}
delete[] dicts_ref;
std::transform(dicts_ref.begin(), dicts_ref.end(), dicts.begin(),
[](const SUAttributeDictionaryRef& value) {
return AttributeDictionary(value);
});;
return dicts;
}

Expand Down
Loading

0 comments on commit ffc704c

Please sign in to comment.