C / C++ / MFC

int i = 1;
int j = (i) + ++i;

int i = 1;
i = i + 1;
int j = i + i;

j[i]=i;
i++;

const int src[] = {0, 1};
const size_t srcLen = sizeof(src) / sizeof(int);
int *const dst = new int[srcLen] - 1;
for (size_t i = 0; i < srcLen; ++i)
    dst[i + 1] = src[i];

for (size_t i = 0; i < srcLen; )
    dst[++i] = src[i];

for (size_t i = 0; i < srcLen; ) {
    const int temp = src[i];
    dst[++i] = temp;
}

Skippums wrote:

why not write efficient code when you can?

const size_t indexArray[] = {1, 0, 3, 2, 0};
DATA_TYPE *const dataArray = new DATA_TYPE[3] - 1; // This is a 1-indexed array and has three elements in it (since max(indexArray) == 3)

int *src_start = ... // from API
int *src_end = ... // from API

int *p = new int[ src_end - src_start ];
std::copy( src_start, src_end, p );
int *p_to_1_based_alias = p - 1;

template<std::size_t N, typename T>
class N_based_array
{
    public:
        N_based_array( T *data ) : data_( data ) {}

        T &operator[]( std::size_t index )
        {
            return data_[ index - N ];
        }

	private:
		T *data_;
};

// prhs is of type "mxArray const *"
size_t const indexCount = mxGetNumberOfElements(prhs[0]);
size_t const dataSize = mxGetNumberOfElements(prhs[1]);
double const ** data = new double const *[dataSize];
// Get the 1-based indices into the data array
unsigned __int32 const *const indices = static_cast<unsigned __int32 const *>(mxGetData(prhs[0]));
for (size_t i = 0; i < dataSize; ++i) {
    // Get a double* to the data in the cell array at 0-indexed location i
    data[i] = mxGetPr(mxGetCell(prhs[1], i));
}
--data; // Make data 1-indexed so I don't have to modify "indices" to be 0-based

Skippums wrote:

int *const dst = new int[srcLen] - 1;

Skippums wrote:

for (size_t i = 0; i < srcLen; )
dst[++i] = src[i];

Skippums wrote:

const int temp = src[i];

Skippums wrote:

(I don't really care timing-wise as the loop is only iterating over a couple hundred thousand elements one time per run, which is an insignificant amount of time as a fraction of the program's run-time, but why not write efficient code when you can?)

void Test1()
{
	for (size_t i = 0; i < len; ++i)
		pDst[i + 1] = pSrc[i];
}

void Test2()
{
	for (size_t i = 0; i < len; )
	{
		const int temp = pSrc[i];
		pDst[++i] = temp;
	}
}

void Test3()
{
	memcpy(&pDst[1], pSrc, len * sizeof(int));
}

void Test4()
{
	int *src_start = (int*) &pSrc[0];
	int *src_end = (int*) &pSrc[len];

	std::copy(src_start, src_end, &pDst[1]);
}

Joe Woodbury wrote:

const int temp = src[i];

The "const" here does nothing and ends up being slower, not just because you are confusing the compiler, but because it prevents the compiler from using an indexed assembly call.

for (size_t i = 0; i < srcLen; ++i)
    dst[i + 1] = src[i];

for (size_t i = 0; i < srcLen; )
{
    const int temp = src[i];
    dst[++i] = temp;
}

int* pDst = &dst[1];
int* pSrc = &src[0]; // just to be more clear

for (size_t i = 0; i < srcLen; ++i)
    *pDst++ = *pSrc++;

Joe Woodbury wrote:

This is seriously broken. You are allocating a pointer and then decrementing it. This is VERY bad (especially since you declare dst const and now can't delete it.) If you want to use 1 indexing, simply allocate one extra element and don't worry about element zero.

delete[] (dst + 1);

Joe Woodbury wrote:

The behavior here is technically undefined, however with VS 2008 it will increment i, then use the result as the index for both arrays.

Joe Woodbury wrote:

The "const" here does nothing

Joe Woodbury wrote:

Because memcpy() will very likely be faster in this case

Joe Woodbury wrote:

Assuming you have a sound algorithm, a general rule is writing sensible, plain C/C++ code

Skippums wrote:

I'm guessing VS2008 is smart enough not to allocate memory for it

void Test1()
{
	for (size_t i = 0; i < len; ++i)
		pDst[i + 1] = pSrc[i];
}

void Test2()
{
	for (size_t i = 0; i < len; )
	{
		const int temp = pSrc[i];
		pDst[++i] = temp;
	}
}

void Test3()
{
	memcpy(&pDst[1], pSrc, len * sizeof(int));
}

void Test4()
{
	int *src_start = (int*) &pSrc[0];
	int *src_end = (int*) &pSrc[len];

	std::copy(src_start, src_end, &pDst[1]);
}

Skippums wrote:

Why use a performance profiler when I can just guess which parts will be inefficient