Thread: Making function smaller and more efficient.

Does that even compile? I note that you are re-declaring data multiple times in the scope of the switch statement: the case labels and (presumably unshown) break statements do not create inner scopes. You should use an "extra" pair of braces immediately after the case label and before the break to create a block with that inner scope, or perhaps move each snippet of code to its own (inline) function (possibly declared in an unnamed namespace).

Oh, I think I understand now. Perhaps you meant to write:

Code:

void processData(int index, int format, void *handler) {


#ifdef LITTLE_ENDIAN


    switch (format) {
    case S_INT_8_LSB_1:
    case S_INT_8_MSB_1:
    case S_INT_8_NER_8:
    case S_INT_16_LSB:
    case S_INT_16_MSB:
    case S_INT_24_LSB:
    case S_INT_24_MSB:
    case S_INT_32_LSB_16:
    case S_INT_32_MSB_16:
    case S_INT_32_LSB_18:
    case S_INT_32_MSB_18:
    case S_INT_32_LSB_20:
    case S_INT_32_MSB_20:
    case S_INT_32_LSB_24:
    case S_INT_32_MSB_24:
        {
            Int32* data = new Int32[AudioStream::input->bufferSize];
            // Convert and create convert back function pointer
            DataProcessor<Int32>* handler = (DataProcessor<Int32>*)handler;
            handler->process(data, AudioStream::input->bufferSize);
        }
        break;
    case S_INT_32_LSB:
    case S_INT_32_MSB:
        {
            Int64* data = new Int64[AudioStream::input->bufferSize];
            // Convert and create convert back function pointer
            DataProcessor<Int64>* handler = (DataProcessor<Int64>*)handler;
            handler->process(data, AudioStream::input->bufferSize);
        }
        break;
    case FLOAT_32_LSB:
    case FLOAT_32_MSB:
        {
            Float32 *data; new Float32[AudioStream::input->bufferSize];
            // Convert and create convert back function pointer
            DataProcessor<Float32>* handler = (DataProcessor<Float32>*)handler;
            handler->process(data, AudioStream::input->bufferSize);
        }
        break;
    case FLOAT_64_LSB:
    case FLOAT_64_MSB:
        {
            Float64 *data = new Float64[AudioStream::input->bufferSize];
            // Convert and create convert back function pointer
            DataProcessor<Float64>* handler = (DataProcessor<Float64>*)handler;
            handler->process(data, AudioStream::input->bufferSize);
        }
        break;
    }


    // Call function pointer


#else
#error endianness not supported
#endif
}

But what is with the hanging "Call function pointer" comment?

EDIT:

Originally Posted by cmajor28

It does compile. On both Visual Studio and GCC. But I'm a little confused on what you are saying. I know what I have above it BADDDD programming practice but I really don't have a choice.

Really? Is that actually your code?

It is. I had your code before but it actually didn't compile on Visual Studio. Surprised me a lot because my old code/your code is ALOT prettier and easier to read.
One thing different about mine is in each case statement I have the comment. Each SEPERATE case has a different conversion function.

And that comment means I need to call a function to convert the samples back and then but it back into the correct buffer.

EDIT: But I see what you mean about the scope thing now.

Originally Posted by cmajor28

It is. I had your code before but it actually didn't compile on Visual Studio. Surprised me a lot because my old code/your code is ALOT prettier and easier to read.

Including the "extra" braces?

Out of curiosity, but does this program compile for you?

Code:

#include <iostream>

int main(int argc, char* argv[])
{
    switch (argc)
    {
    case 1:
        {
            int n = 1;
    case 2:
        int n = 2;
    case 3:
        int n = 3;
        std::cout << n << std::endl;
        break;
    }
}

EDIT:

Originally Posted by cmajor28

One thing different about mine is in each case statement I have the comment. Each SEPERATE case has a different conversion function.

Sigh. Then please either post your actual code, or make it obvious that there is code that is omitted, e.g., include a "// ..." comment along with the break; statement that is actually there and which affects the logic.

Originally Posted by cmajor28

And that comment means I need to call a function to convert the samples back and then but it back into the correct buffer.

Have you actually written that code? What variables are involved?

Oh, sorry, I made a typo. Try:

Code:

#include <iostream>

int main(int argc, char* argv[])
{
    switch (argc)
    {
    case 1:
        int n = 1;
    case 2:
        int n = 2;
    case 3:
        int n = 3;
        std::cout << n << std::endl;
        break;
    }
}

Even in g++? What version of MSVC and g++ are you using?

1800 MSVC. I didn't try it on GCC. It may have to deal with my compiler settings. For this project they are changed ALOT because of some fixes I had to make for an SDK I'm using.

Edit: Looking at the code I have NO IDEA how it compiles for me. Maybe I have some errors disabled because the program acts weird when it runs. I put some gotos and it all worked perfect.

Originally Posted by cmajor28

1800 MSVC. I didn't try it on GCC. It may have to deal with my compiler settings. For this project they are changed ALOT because of some fixes I had to make for an SDK I'm using.

I don't have MSVC on this computer, but checking with this purported online version of MSVC shows that it does give the error messages that I expect.

Originally Posted by cmajor28

Edit: Looking at the code I have NO IDEA how it compiles for me. Maybe I have some errors disabled because the program acts weird when it runs. I put some gotos and it all worked perfect.

Err... that's horrible. I know we do joke about code that works and we don't know why, and in reality that does happen especially when we take over maintenance of a code base for which the previous developer(s) are no longer available for consultation, but you do need to know why the code you just wrote works, otherwise you're programming blind. Putting in some gotos (which should not be done in the first place without very, very good reason in C++) and seeing that it "all worked perfect" is just asking for trouble: in such a case I dare not suggest anything to you because my suggestions could well break the code that nobody understands, and in fact even a change of say, optimisation level could well cause such brittle code to break.