Skip to main content
added 11 characters in body
Source Link
Bizkit
  • 1.7k
  • 10
  • 17
std::string numToBin(unsigned num, unsigned depth)
{
    std::string s;
    s.reserve(depth);
    unsigned bits = sizeof(unsigned) * 8 - 1;
    for (unsigned i = 0, shift = bits; i < depth && shift <= bits; 
                  ++i, --shift){
        s.push_back('0' + ((num & (1 << shift)) >> shift));
    }
    return s;
}
std::string numToBin(unsigned num, unsigned depth)
{
    std::string s;
    s.reserve(depth);
    unsigned bits = sizeof(unsigned) * 8 - 1;
    for (unsigned i = 0, shift = bits; i < depth && shift <= bits; 
                  ++i, --shift){
        s.push_back('0' + (num & (1 << shift)));
    }
    return s;
}
std::string numToBin(unsigned num, unsigned depth)
{
    std::string s;
    s.reserve(depth);
    unsigned bits = sizeof(unsigned) * 8 - 1;
    for (unsigned i = 0, shift = bits; i < depth && shift <= bits; 
                  ++i, --shift){
        s.push_back('0' + ((num & (1 << shift)) >> shift));
    }
    return s;
}
deleted 144 characters in body
Source Link
Bizkit
  • 1.7k
  • 10
  • 17

I'm almost certain this program won't compile because of these lines:

assignCodes(map, *rootNode.left, currWord << 1, depth + 1);
assignCodes(map, *rootNode.right, (currWord << 1) | 1, depth + 1);

rootNode is not a pointer so it cannot be dereferenced. Also, while we're at it, there'sThere's really no need to do all that bit manipulation when you can simply do:

assignCodes(map, *(rootNode.left), currWord * 2, depth + 1);
assignCodes(map, *(rootNode.right), currWord * 2 + 1, depth + 1);

EDIT: I would also put parenthesis around the expression that the * operator is supposed to dereference to make it clearer as I've done above. A similar syntactical fix needs to be done in decodeMessage() too.

std::string ret;
ret.reserve(message.size()); // reserve memory up front to avoid deallocations repeated allocations
for (const auto& c : message) {
    ret += map[c];
}

I'm almost certain this program won't compile because of these lines:

assignCodes(map, *rootNode.left, currWord << 1, depth + 1);
assignCodes(map, *rootNode.right, (currWord << 1) | 1, depth + 1);

rootNode is not a pointer so it cannot be dereferenced. Also, while we're at it, there's really no need to do all that bit manipulation when you can simply do:

assignCodes(map, rootNode.left, currWord * 2, depth + 1);
assignCodes(map, rootNode.right, currWord * 2 + 1, depth + 1);

A similar syntactical fix needs to be done in decodeMessage() too.

std::string ret;
ret.reserve(message.size()); // reserve memory up front to avoid deallocations
for (const auto& c : message) {
    ret += map[c];
}

There's really no need to do all that bit manipulation when you can simply do:

assignCodes(map, *(rootNode.left), currWord * 2, depth + 1);
assignCodes(map, *(rootNode.right), currWord * 2 + 1, depth + 1);

EDIT: I would also put parenthesis around the expression that the * operator is supposed to dereference to make it clearer as I've done above. A similar syntactical fix needs to be done in decodeMessage() too.

std::string ret;
ret.reserve(message.size()); // reserve memory up front to avoid  repeated allocations
for (const auto& c : message) {
    ret += map[c];
}
added 265 characters in body
Source Link
Bizkit
  • 1.7k
  • 10
  • 17

EDIT:

I can't believe I failed to mention this:

Your class dynamically creates Nodes using new but I don't see delete anywhere; as a result, your class will leak all that memory. Use std::unique_ptr to avoid having to deal with memory management issues.

EDIT:

I can't believe I failed to mention this:

Your class dynamically creates Nodes using new but I don't see delete anywhere; as a result, your class will leak all that memory. Use std::unique_ptr to avoid having to deal with memory management issues.

added 5 characters in body
Source Link
Bizkit
  • 1.7k
  • 10
  • 17
Loading
added 445 characters in body
Source Link
Bizkit
  • 1.7k
  • 10
  • 17
Loading
Source Link
Bizkit
  • 1.7k
  • 10
  • 17
Loading