6
\$\begingroup\$

I originally made this as a way to use character strings as a base-256 number, but then realized that I could turn it into a method for encryption instead. The only thing is that since I'm using a basic addition algorithm for the encryption process, I'm iterating through a string by one character at a time (which seems kind of slow). Are there any other ways to improve the performance of this process or am I pretty much stuck with simple iteration?

Here's the encryption code:

void encodeStr(string encryptKey, string& inputToEncrypt) {
    assert(minVal < maxVal);
    if (inputToEncrypt.length() == 0) return;

    //preliminary remainder check initialization
    int remainder = minVal; //zero value
    int keyNumCount = 0;
    int keyCount = 0;

    //variables used to determine when to stop counting numbers
    size_t keyPos = 0;
    size_t count = 0;
    size_t countEnd = inputToEncrypt.length();

    //begin numerical addition. stop once the end of both strings is reached
    while (count < countEnd) {
        //convert current characters to unsigned chars
        keyNumCount = encryptKey[keyPos];
        if (keyNumCount < minVal) keyNumCount += minVal;

        keyCount = (count < inputToEncrypt.length()) ? inputToEncrypt[count] : 0;
        if (keyCount < minVal) keyCount += minVal;

        //take advantage of signed-char overflow to calculate current value
        remainder += keyNumCount + keyCount;
        inputToEncrypt[count] = remainder;

        //check if there is a remainder to be tabulated
        remainder = (remainder > maxVal) ? 1 : 0;
        ++count;
        ++keyPos;
        if (keyPos == encryptKey.size()) keyPos = 0;

        //add 1 to the count if there is a remainder at the end of keyNum
        if (count == countEnd && remainder != 0) {
            inputToEncrypt.push_back('\0');
            ++countEnd; //variables used to avoid integer overflow
        }
    }
}

And the decryption function:

void decodeStr(string decryptKey, string& inputToDecrypt) {
    assert(minVal < maxVal);
    if (inputToDecrypt.length() == 0) return;

    //preliminary remainder check initialization
    int remainder = minVal;
    int keyNumCount = 0;
    int keyCount = 0;

    //variables used to determine when to stop counting numbers
    size_t keyPos = 0;
    size_t count = 0;
    size_t countEnd = inputToDecrypt.length();

    //make sure the program exits before the number necomes negative
    while (count < countEnd) {
        //convert current characters to unsigned chars
        keyNumCount = decryptKey[keyPos];
        if (keyNumCount < minVal) keyNumCount += minVal;

        keyCount = inputToDecrypt[count];
        if (keyCount < minVal) keyCount += minVal;

        //take advantage of signed-char overflow to calculate current value
        remainder += keyCount - keyNumCount;
        inputToDecrypt[count] = remainder;

        //check if there is a remainder to be tabulated
        remainder = (remainder > maxVal) ? 1 : 0;
        ++count;
        ++keyPos;
        if (keyPos == decryptKey.size()) keyPos = 0;
    }
}
\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

Since you key is const it would be nice to mark it as such (and avoid accidents of changing your key):

void encodeStr(string const& encryptKey, string& inputToEncrypt) {
//                    ^^^^^

To save the cost of the copy I would also make it a reference.

No idea what these are:

assert(minVal < maxVal);

The extra set of variables seems like a good idea but actually makes the code harder to read as you need to understand what the variable represents:

size_t countEnd = inputToEncrypt.length();
while (count < countEnd) {

// Simpler to

while (count < inputToEncrypt.length()) {

When is this condition every going to fail and give you 0?

keyCount = (count < inputToEncrypt.length()) ? inputToEncrypt[count] : 0;

If it was going to fail then you should have the same precautions on this:

inputToEncrypt[count] = remainder;

This is also bound to result in truncation of reminder. As inputToEncrypt as a string and thus each position is a char while remainder is the sum of 2 integers both greater than minval.

inputToEncrypt[count] = remainder;
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the help. The first two things you pointed out were remnants of when I was debugging. The "keycount" variable is used to determine what character in the encryption key should be used to do the encrypting. If the thing you want to encrypt is longer than the encryption key, then the encryption key becomes repeated. \$\endgroup\$
    – icdae
    Jun 3, 2012 at 18:20
4
\$\begingroup\$

If you're only interested in implementing this specific form of encryption, then what I'm going to post is almost certainly overkill. On the other hand, if you want to play around with different forms of encryption, it may be interesting. This attempts to factor the the job into a few distinct pieces. I'm not sure it entirely succeeds at doing that, but perhaps the attempt will be interesting anyway.

First, a little tool that's probably useful for a fair number of algorithms like this: a cyclic_iterator that provides an imitation of an infinite-length input by cycling through a finite-length input as often as needed. Right now, this is quite (excessively?) minimal -- for example, it doesn't even attempt to deal with post-increment, only pre-increment.

#ifndef CYCLIC_ITERATOR_H_INC_
#define CYCLIC_ITERATOR_H_INC_
#include <iterator>

template <class FwdIt>
class cyclic_iterator_t : public std::iterator<std::input_iterator_tag, typename FwdIt::value_type> {
    FwdIt begin;
    FwdIt end;
    FwdIt current;
public:
    cyclic_iterator_t(FwdIt begin, FwdIt end) : begin(begin), end(end), current(begin) {}

    cyclic_iterator_t operator++() { 
        if (++current == end) 
            current = begin; 
        return *this; 
    }
    typename FwdIt::value_type operator *() const { return *current; }
};

template <class Container>
cyclic_iterator_t<typename Container::iterator> cyclic_iterator(Container &c) { 
    return cyclic_iterator_t<typename Container::iterator>(c.begin(), c.end());
}

#endif

Then a little program to implement something that's at least similar to your algorithm (I couldn't regression test, because you didn't specify you minVal, among other things). It currently makes no attempt at dealing sensibly with negative inputs either.

#include <limits.h>
#include <vector>
#include <iostream>
#include <string>
#include <algorithm>

#include "cyclic_iterator.h"

// first a class to encrypt one character at a time:
// 
class encrypt { 
    bool carry;
public:
    encrypt() : carry(0) {}
    char operator()(char ina, char inb) { 
        int ret = ina + inb + static_cast<int>(carry);
        carry = ret > CHAR_MAX;
        return ret % CHAR_MAX;
    }
    operator bool() { return carry; }
};

// Then a little algorithm to apply that encryptor to an entire string of input:
//    
template<class InIt1, class InIt2, class OutIt>
void encode_string(InIt1 begin1, InIt1 end1, InIt2 begin2, OutIt result) { 
    encrypt e;
    // The main loop is almost like two-input std::transform:
    while (begin1 != end1) {
        *result = e(*begin1, *begin2);
        ++result;
        ++begin1;
        ++begin2;
    }
    // but then we handle the "carry" bit (if any):
    if (e)
        *result++ = '\0';
}

// finally a simple main to test out the preceding:    
int main() {
    std::string input("This is some text");
    std::string key("TheKey");
    std::string result;

    encode_string(input.begin(), input.end(), cyclic_iterator(key), std::back_inserter(result));
    std::cout << result;
    return 0;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.