Simple array operation using a Queue











up vote
2
down vote

favorite
1












Question



We are given an array (indexed from 1) of N elements on which we make M queries:



add(Left, Right, X) - all the elements between the position Left and Right (1<=Left<=Right<=N) are raising their values with X.



After all the operations are completed, print the array.



example



Array: 1 1 1 4 5 6



operations: (1, 5, 2), (2, 3, 10)



Final array: 3 13 13 6 7 6



Discussion



I managed to produce the expected output but I was wondering if this code can be simplified using a queue data structure somehow?



The complexity of this code is M*N (please correct me if I am wrong) - This naive approach is looping all the operations for every element of the input array.



Naive approach



#include <iostream>

using namespace std;

int Queue[1000], backInd;

void push(int i)
{
++backInd;
Queue[backInd] = i;
}

void applyOperations(int input, int operations, int N, int M)
{
for (int x = 0; x < N; ++x){
int item = input[x];
int offset = 0;
for (int op = 0; op < M; ++op)
{
if (x + 1 >= operations[op + offset] && x + 1 <= operations[op + offset + 1])
{
item += operations[op + offset + 2];
}
offset += 2;
}
push(item);
}
}

int main()
{
backInd = -1;
frontInd = 0;
int input = {1, 1, 1, 4, 5, 6};
int operations = {1, 5, 2, 2, 3, 10};
int N = 6;
int M = 2;

applyOperations(input, operations, N, M);

for (int x = 0; x < N; ++x) {
cout << Queue[x] << " ";
}

return 0;
}









share|improve this question


























    up vote
    2
    down vote

    favorite
    1












    Question



    We are given an array (indexed from 1) of N elements on which we make M queries:



    add(Left, Right, X) - all the elements between the position Left and Right (1<=Left<=Right<=N) are raising their values with X.



    After all the operations are completed, print the array.



    example



    Array: 1 1 1 4 5 6



    operations: (1, 5, 2), (2, 3, 10)



    Final array: 3 13 13 6 7 6



    Discussion



    I managed to produce the expected output but I was wondering if this code can be simplified using a queue data structure somehow?



    The complexity of this code is M*N (please correct me if I am wrong) - This naive approach is looping all the operations for every element of the input array.



    Naive approach



    #include <iostream>

    using namespace std;

    int Queue[1000], backInd;

    void push(int i)
    {
    ++backInd;
    Queue[backInd] = i;
    }

    void applyOperations(int input, int operations, int N, int M)
    {
    for (int x = 0; x < N; ++x){
    int item = input[x];
    int offset = 0;
    for (int op = 0; op < M; ++op)
    {
    if (x + 1 >= operations[op + offset] && x + 1 <= operations[op + offset + 1])
    {
    item += operations[op + offset + 2];
    }
    offset += 2;
    }
    push(item);
    }
    }

    int main()
    {
    backInd = -1;
    frontInd = 0;
    int input = {1, 1, 1, 4, 5, 6};
    int operations = {1, 5, 2, 2, 3, 10};
    int N = 6;
    int M = 2;

    applyOperations(input, operations, N, M);

    for (int x = 0; x < N; ++x) {
    cout << Queue[x] << " ";
    }

    return 0;
    }









    share|improve this question
























      up vote
      2
      down vote

      favorite
      1









      up vote
      2
      down vote

      favorite
      1






      1





      Question



      We are given an array (indexed from 1) of N elements on which we make M queries:



      add(Left, Right, X) - all the elements between the position Left and Right (1<=Left<=Right<=N) are raising their values with X.



      After all the operations are completed, print the array.



      example



      Array: 1 1 1 4 5 6



      operations: (1, 5, 2), (2, 3, 10)



      Final array: 3 13 13 6 7 6



      Discussion



      I managed to produce the expected output but I was wondering if this code can be simplified using a queue data structure somehow?



      The complexity of this code is M*N (please correct me if I am wrong) - This naive approach is looping all the operations for every element of the input array.



      Naive approach



      #include <iostream>

      using namespace std;

      int Queue[1000], backInd;

      void push(int i)
      {
      ++backInd;
      Queue[backInd] = i;
      }

      void applyOperations(int input, int operations, int N, int M)
      {
      for (int x = 0; x < N; ++x){
      int item = input[x];
      int offset = 0;
      for (int op = 0; op < M; ++op)
      {
      if (x + 1 >= operations[op + offset] && x + 1 <= operations[op + offset + 1])
      {
      item += operations[op + offset + 2];
      }
      offset += 2;
      }
      push(item);
      }
      }

      int main()
      {
      backInd = -1;
      frontInd = 0;
      int input = {1, 1, 1, 4, 5, 6};
      int operations = {1, 5, 2, 2, 3, 10};
      int N = 6;
      int M = 2;

      applyOperations(input, operations, N, M);

      for (int x = 0; x < N; ++x) {
      cout << Queue[x] << " ";
      }

      return 0;
      }









      share|improve this question













      Question



      We are given an array (indexed from 1) of N elements on which we make M queries:



      add(Left, Right, X) - all the elements between the position Left and Right (1<=Left<=Right<=N) are raising their values with X.



      After all the operations are completed, print the array.



      example



      Array: 1 1 1 4 5 6



      operations: (1, 5, 2), (2, 3, 10)



      Final array: 3 13 13 6 7 6



      Discussion



      I managed to produce the expected output but I was wondering if this code can be simplified using a queue data structure somehow?



      The complexity of this code is M*N (please correct me if I am wrong) - This naive approach is looping all the operations for every element of the input array.



      Naive approach



      #include <iostream>

      using namespace std;

      int Queue[1000], backInd;

      void push(int i)
      {
      ++backInd;
      Queue[backInd] = i;
      }

      void applyOperations(int input, int operations, int N, int M)
      {
      for (int x = 0; x < N; ++x){
      int item = input[x];
      int offset = 0;
      for (int op = 0; op < M; ++op)
      {
      if (x + 1 >= operations[op + offset] && x + 1 <= operations[op + offset + 1])
      {
      item += operations[op + offset + 2];
      }
      offset += 2;
      }
      push(item);
      }
      }

      int main()
      {
      backInd = -1;
      frontInd = 0;
      int input = {1, 1, 1, 4, 5, 6};
      int operations = {1, 5, 2, 2, 3, 10};
      int N = 6;
      int M = 2;

      applyOperations(input, operations, N, M);

      for (int x = 0; x < N; ++x) {
      cout << Queue[x] << " ";
      }

      return 0;
      }






      c++ algorithm queue






      share|improve this question













      share|improve this question











      share|improve this question




      share|improve this question










      asked Nov 16 at 12:16









      Michael

      19016




      19016






















          2 Answers
          2






          active

          oldest

          votes

















          up vote
          3
          down vote



          accepted










          I'm not really sure if your code matches the intention of the task, albeit you possibly might get the expected results.



          At first, you are producing a new array, whereas the task is 'all elements raise their values', which I read that you should modify the array in place.



          Then I'd rather assume that you should implement a single operation in a function which then gets called M times, example:



          void applyOperation(int array, size_t length, size_t begin, size_t end, int operator);

          int array = { 1, 1, 1, 4, 5, 6 };
          applyOperation (array, sizeof(array)/sizeof(*array), 1, 5, 2);
          applyOperation (array, sizeof(array)/sizeof(*array), 2, 3, 10);
          // print result


          M is 2, function is called twice (I renamed N to length...).



          Whether you now implement the function to execute one operation at a time or all at once, you shouldn't iterate over the entire array and then check if the index is in the range, instead you'd iterate over the range directly:



          void applyOperation(int array, size_t length, size_t begin, size_t end, int operator)
          {
          --begin; --end; // they want one-based indices? bad idea for C++,
          // but OK, then let's adjust them to zero-based...

          for(;; begin != end; ++begin)
          {
          array[begin] += operator;
          }
          }


          Looks much easier, doesn't it? What's actually lacking yet is checking the range of begin and end, assuming you manage this yourself...



          Now if you want to have all operations in one single go, then it is much more comprehensive if you iterate over the operations in first place:



          void applyOperations(int input, int operations, int N, int M)
          {
          for(int i = 0; i < M; ++i, operations += 3)
          // this assumes: each operation occupies three subsequent values
          {
          // now using operations[0] as begin, operations[1] as end
          // and operations[2] as operator just like above

          // it might be unacceptable to modify the operations array, then you'd
          // need a separate loop variable compared to above; but IF it is,
          // then you should keep the operations const anyway:
          //int const operations
          }
          }


          Finally: You tagged the question C++ – your code could as well be good old C, though. In C++, you'd rather use std::array or std::vector than raw arrays. Whichever you use, you don't have to provide size/length separately, both come with their own member function size() giving the number of elements (not size in bytes as sizeof operator does, so you'd avoid the ugly sizeof(x)/sizeof(*x) at the same time – even more important: you could provide a bad value for length, this cannot happen with the C++ containers as these manage their length/size on their own).



          void applyOperation(std::vector<int> values, size_t begin, size_t end, int operator)
          {
          --begin; --end;
          for(;; begin != end; ++begin)
          {
          values[begin] += operator;
          }
          }


          The loop still looks similar, but range checking would now use values.size() instead of a length parameter.



          One final note: Did you notice that I changed the parameter types to size_t? Two reasons for this:





          1. size_t is an unsigned type. Negative indices are meaningless in the given task, so why allow them at all?


          2. size_t mapped to the native type large enough to hold an object of maximum size on given system (which means at the same time that it will always be large enough to index any member in an array). This is obviously not relevant in given case, as your arrays' sizes most likely won't ever exceed the range of ordinary int - but you simply cannot fail this way, so if you get used to it, you'll be well prepared for the future...






          share|improve this answer










          New contributor




          Aconcagua is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
          Check out our Code of Conduct.


















          • This is a great answer, not only you improve the code by re-formulating it in a better way but also you elaborate some c++ trick - I am not a c++ developer - this is very helpful! Thanks a lot.
            – Michael
            Nov 16 at 13:51


















          up vote
          2
          down vote













          frontInd is never declared. It's only ever assigned to, so that assignment can just be removed.



          Avoid using namespace std - that's not a namespace that's designed to be imported wholesale, and you could introduce surprising bugs that way. Given that we have a single use of std::cout and nothing else from the standard library, it's actually costing more code lines/characters (as well as more cognitive overhead) than just writing the prefix where needed.



          Inseparable data should be contained in a class. Here, backInd and Queue belong together; no outside code ever needs to use backInd, so it could be a private member. That said, backInd is always equal to x when writing, so we could just index that way.



          Consider an alternative algorithm - the current approach will be slow when the number of operations gets large. Consider looping over the operations just once, recording the delta of value at each position (so each operation will update just two values - an increment at the beginning of its range and a corresponding decrement at the end). Then track the cumulative total when writing the output.






          share|improve this answer























            Your Answer





            StackExchange.ifUsing("editor", function () {
            return StackExchange.using("mathjaxEditing", function () {
            StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
            StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
            });
            });
            }, "mathjax-editing");

            StackExchange.ifUsing("editor", function () {
            StackExchange.using("externalEditor", function () {
            StackExchange.using("snippets", function () {
            StackExchange.snippets.init();
            });
            });
            }, "code-snippets");

            StackExchange.ready(function() {
            var channelOptions = {
            tags: "".split(" "),
            id: "196"
            };
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function() {
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled) {
            StackExchange.using("snippets", function() {
            createEditor();
            });
            }
            else {
            createEditor();
            }
            });

            function createEditor() {
            StackExchange.prepareEditor({
            heartbeatType: 'answer',
            convertImagesToLinks: false,
            noModals: true,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            imageUploader: {
            brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
            contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
            allowUrls: true
            },
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            });


            }
            });














             

            draft saved


            draft discarded


















            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f207799%2fsimple-array-operation-using-a-queue%23new-answer', 'question_page');
            }
            );

            Post as a guest















            Required, but never shown

























            2 Answers
            2






            active

            oldest

            votes








            2 Answers
            2






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes








            up vote
            3
            down vote



            accepted










            I'm not really sure if your code matches the intention of the task, albeit you possibly might get the expected results.



            At first, you are producing a new array, whereas the task is 'all elements raise their values', which I read that you should modify the array in place.



            Then I'd rather assume that you should implement a single operation in a function which then gets called M times, example:



            void applyOperation(int array, size_t length, size_t begin, size_t end, int operator);

            int array = { 1, 1, 1, 4, 5, 6 };
            applyOperation (array, sizeof(array)/sizeof(*array), 1, 5, 2);
            applyOperation (array, sizeof(array)/sizeof(*array), 2, 3, 10);
            // print result


            M is 2, function is called twice (I renamed N to length...).



            Whether you now implement the function to execute one operation at a time or all at once, you shouldn't iterate over the entire array and then check if the index is in the range, instead you'd iterate over the range directly:



            void applyOperation(int array, size_t length, size_t begin, size_t end, int operator)
            {
            --begin; --end; // they want one-based indices? bad idea for C++,
            // but OK, then let's adjust them to zero-based...

            for(;; begin != end; ++begin)
            {
            array[begin] += operator;
            }
            }


            Looks much easier, doesn't it? What's actually lacking yet is checking the range of begin and end, assuming you manage this yourself...



            Now if you want to have all operations in one single go, then it is much more comprehensive if you iterate over the operations in first place:



            void applyOperations(int input, int operations, int N, int M)
            {
            for(int i = 0; i < M; ++i, operations += 3)
            // this assumes: each operation occupies three subsequent values
            {
            // now using operations[0] as begin, operations[1] as end
            // and operations[2] as operator just like above

            // it might be unacceptable to modify the operations array, then you'd
            // need a separate loop variable compared to above; but IF it is,
            // then you should keep the operations const anyway:
            //int const operations
            }
            }


            Finally: You tagged the question C++ – your code could as well be good old C, though. In C++, you'd rather use std::array or std::vector than raw arrays. Whichever you use, you don't have to provide size/length separately, both come with their own member function size() giving the number of elements (not size in bytes as sizeof operator does, so you'd avoid the ugly sizeof(x)/sizeof(*x) at the same time – even more important: you could provide a bad value for length, this cannot happen with the C++ containers as these manage their length/size on their own).



            void applyOperation(std::vector<int> values, size_t begin, size_t end, int operator)
            {
            --begin; --end;
            for(;; begin != end; ++begin)
            {
            values[begin] += operator;
            }
            }


            The loop still looks similar, but range checking would now use values.size() instead of a length parameter.



            One final note: Did you notice that I changed the parameter types to size_t? Two reasons for this:





            1. size_t is an unsigned type. Negative indices are meaningless in the given task, so why allow them at all?


            2. size_t mapped to the native type large enough to hold an object of maximum size on given system (which means at the same time that it will always be large enough to index any member in an array). This is obviously not relevant in given case, as your arrays' sizes most likely won't ever exceed the range of ordinary int - but you simply cannot fail this way, so if you get used to it, you'll be well prepared for the future...






            share|improve this answer










            New contributor




            Aconcagua is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.


















            • This is a great answer, not only you improve the code by re-formulating it in a better way but also you elaborate some c++ trick - I am not a c++ developer - this is very helpful! Thanks a lot.
              – Michael
              Nov 16 at 13:51















            up vote
            3
            down vote



            accepted










            I'm not really sure if your code matches the intention of the task, albeit you possibly might get the expected results.



            At first, you are producing a new array, whereas the task is 'all elements raise their values', which I read that you should modify the array in place.



            Then I'd rather assume that you should implement a single operation in a function which then gets called M times, example:



            void applyOperation(int array, size_t length, size_t begin, size_t end, int operator);

            int array = { 1, 1, 1, 4, 5, 6 };
            applyOperation (array, sizeof(array)/sizeof(*array), 1, 5, 2);
            applyOperation (array, sizeof(array)/sizeof(*array), 2, 3, 10);
            // print result


            M is 2, function is called twice (I renamed N to length...).



            Whether you now implement the function to execute one operation at a time or all at once, you shouldn't iterate over the entire array and then check if the index is in the range, instead you'd iterate over the range directly:



            void applyOperation(int array, size_t length, size_t begin, size_t end, int operator)
            {
            --begin; --end; // they want one-based indices? bad idea for C++,
            // but OK, then let's adjust them to zero-based...

            for(;; begin != end; ++begin)
            {
            array[begin] += operator;
            }
            }


            Looks much easier, doesn't it? What's actually lacking yet is checking the range of begin and end, assuming you manage this yourself...



            Now if you want to have all operations in one single go, then it is much more comprehensive if you iterate over the operations in first place:



            void applyOperations(int input, int operations, int N, int M)
            {
            for(int i = 0; i < M; ++i, operations += 3)
            // this assumes: each operation occupies three subsequent values
            {
            // now using operations[0] as begin, operations[1] as end
            // and operations[2] as operator just like above

            // it might be unacceptable to modify the operations array, then you'd
            // need a separate loop variable compared to above; but IF it is,
            // then you should keep the operations const anyway:
            //int const operations
            }
            }


            Finally: You tagged the question C++ – your code could as well be good old C, though. In C++, you'd rather use std::array or std::vector than raw arrays. Whichever you use, you don't have to provide size/length separately, both come with their own member function size() giving the number of elements (not size in bytes as sizeof operator does, so you'd avoid the ugly sizeof(x)/sizeof(*x) at the same time – even more important: you could provide a bad value for length, this cannot happen with the C++ containers as these manage their length/size on their own).



            void applyOperation(std::vector<int> values, size_t begin, size_t end, int operator)
            {
            --begin; --end;
            for(;; begin != end; ++begin)
            {
            values[begin] += operator;
            }
            }


            The loop still looks similar, but range checking would now use values.size() instead of a length parameter.



            One final note: Did you notice that I changed the parameter types to size_t? Two reasons for this:





            1. size_t is an unsigned type. Negative indices are meaningless in the given task, so why allow them at all?


            2. size_t mapped to the native type large enough to hold an object of maximum size on given system (which means at the same time that it will always be large enough to index any member in an array). This is obviously not relevant in given case, as your arrays' sizes most likely won't ever exceed the range of ordinary int - but you simply cannot fail this way, so if you get used to it, you'll be well prepared for the future...






            share|improve this answer










            New contributor




            Aconcagua is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.


















            • This is a great answer, not only you improve the code by re-formulating it in a better way but also you elaborate some c++ trick - I am not a c++ developer - this is very helpful! Thanks a lot.
              – Michael
              Nov 16 at 13:51













            up vote
            3
            down vote



            accepted







            up vote
            3
            down vote



            accepted






            I'm not really sure if your code matches the intention of the task, albeit you possibly might get the expected results.



            At first, you are producing a new array, whereas the task is 'all elements raise their values', which I read that you should modify the array in place.



            Then I'd rather assume that you should implement a single operation in a function which then gets called M times, example:



            void applyOperation(int array, size_t length, size_t begin, size_t end, int operator);

            int array = { 1, 1, 1, 4, 5, 6 };
            applyOperation (array, sizeof(array)/sizeof(*array), 1, 5, 2);
            applyOperation (array, sizeof(array)/sizeof(*array), 2, 3, 10);
            // print result


            M is 2, function is called twice (I renamed N to length...).



            Whether you now implement the function to execute one operation at a time or all at once, you shouldn't iterate over the entire array and then check if the index is in the range, instead you'd iterate over the range directly:



            void applyOperation(int array, size_t length, size_t begin, size_t end, int operator)
            {
            --begin; --end; // they want one-based indices? bad idea for C++,
            // but OK, then let's adjust them to zero-based...

            for(;; begin != end; ++begin)
            {
            array[begin] += operator;
            }
            }


            Looks much easier, doesn't it? What's actually lacking yet is checking the range of begin and end, assuming you manage this yourself...



            Now if you want to have all operations in one single go, then it is much more comprehensive if you iterate over the operations in first place:



            void applyOperations(int input, int operations, int N, int M)
            {
            for(int i = 0; i < M; ++i, operations += 3)
            // this assumes: each operation occupies three subsequent values
            {
            // now using operations[0] as begin, operations[1] as end
            // and operations[2] as operator just like above

            // it might be unacceptable to modify the operations array, then you'd
            // need a separate loop variable compared to above; but IF it is,
            // then you should keep the operations const anyway:
            //int const operations
            }
            }


            Finally: You tagged the question C++ – your code could as well be good old C, though. In C++, you'd rather use std::array or std::vector than raw arrays. Whichever you use, you don't have to provide size/length separately, both come with their own member function size() giving the number of elements (not size in bytes as sizeof operator does, so you'd avoid the ugly sizeof(x)/sizeof(*x) at the same time – even more important: you could provide a bad value for length, this cannot happen with the C++ containers as these manage their length/size on their own).



            void applyOperation(std::vector<int> values, size_t begin, size_t end, int operator)
            {
            --begin; --end;
            for(;; begin != end; ++begin)
            {
            values[begin] += operator;
            }
            }


            The loop still looks similar, but range checking would now use values.size() instead of a length parameter.



            One final note: Did you notice that I changed the parameter types to size_t? Two reasons for this:





            1. size_t is an unsigned type. Negative indices are meaningless in the given task, so why allow them at all?


            2. size_t mapped to the native type large enough to hold an object of maximum size on given system (which means at the same time that it will always be large enough to index any member in an array). This is obviously not relevant in given case, as your arrays' sizes most likely won't ever exceed the range of ordinary int - but you simply cannot fail this way, so if you get used to it, you'll be well prepared for the future...






            share|improve this answer










            New contributor




            Aconcagua is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.









            I'm not really sure if your code matches the intention of the task, albeit you possibly might get the expected results.



            At first, you are producing a new array, whereas the task is 'all elements raise their values', which I read that you should modify the array in place.



            Then I'd rather assume that you should implement a single operation in a function which then gets called M times, example:



            void applyOperation(int array, size_t length, size_t begin, size_t end, int operator);

            int array = { 1, 1, 1, 4, 5, 6 };
            applyOperation (array, sizeof(array)/sizeof(*array), 1, 5, 2);
            applyOperation (array, sizeof(array)/sizeof(*array), 2, 3, 10);
            // print result


            M is 2, function is called twice (I renamed N to length...).



            Whether you now implement the function to execute one operation at a time or all at once, you shouldn't iterate over the entire array and then check if the index is in the range, instead you'd iterate over the range directly:



            void applyOperation(int array, size_t length, size_t begin, size_t end, int operator)
            {
            --begin; --end; // they want one-based indices? bad idea for C++,
            // but OK, then let's adjust them to zero-based...

            for(;; begin != end; ++begin)
            {
            array[begin] += operator;
            }
            }


            Looks much easier, doesn't it? What's actually lacking yet is checking the range of begin and end, assuming you manage this yourself...



            Now if you want to have all operations in one single go, then it is much more comprehensive if you iterate over the operations in first place:



            void applyOperations(int input, int operations, int N, int M)
            {
            for(int i = 0; i < M; ++i, operations += 3)
            // this assumes: each operation occupies three subsequent values
            {
            // now using operations[0] as begin, operations[1] as end
            // and operations[2] as operator just like above

            // it might be unacceptable to modify the operations array, then you'd
            // need a separate loop variable compared to above; but IF it is,
            // then you should keep the operations const anyway:
            //int const operations
            }
            }


            Finally: You tagged the question C++ – your code could as well be good old C, though. In C++, you'd rather use std::array or std::vector than raw arrays. Whichever you use, you don't have to provide size/length separately, both come with their own member function size() giving the number of elements (not size in bytes as sizeof operator does, so you'd avoid the ugly sizeof(x)/sizeof(*x) at the same time – even more important: you could provide a bad value for length, this cannot happen with the C++ containers as these manage their length/size on their own).



            void applyOperation(std::vector<int> values, size_t begin, size_t end, int operator)
            {
            --begin; --end;
            for(;; begin != end; ++begin)
            {
            values[begin] += operator;
            }
            }


            The loop still looks similar, but range checking would now use values.size() instead of a length parameter.



            One final note: Did you notice that I changed the parameter types to size_t? Two reasons for this:





            1. size_t is an unsigned type. Negative indices are meaningless in the given task, so why allow them at all?


            2. size_t mapped to the native type large enough to hold an object of maximum size on given system (which means at the same time that it will always be large enough to index any member in an array). This is obviously not relevant in given case, as your arrays' sizes most likely won't ever exceed the range of ordinary int - but you simply cannot fail this way, so if you get used to it, you'll be well prepared for the future...







            share|improve this answer










            New contributor




            Aconcagua is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.









            share|improve this answer



            share|improve this answer








            edited Nov 16 at 23:20









            Cris Luengo

            2,389319




            2,389319






            New contributor




            Aconcagua is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.









            answered Nov 16 at 13:17









            Aconcagua

            2265




            2265




            New contributor




            Aconcagua is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.





            New contributor





            Aconcagua is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.






            Aconcagua is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.












            • This is a great answer, not only you improve the code by re-formulating it in a better way but also you elaborate some c++ trick - I am not a c++ developer - this is very helpful! Thanks a lot.
              – Michael
              Nov 16 at 13:51


















            • This is a great answer, not only you improve the code by re-formulating it in a better way but also you elaborate some c++ trick - I am not a c++ developer - this is very helpful! Thanks a lot.
              – Michael
              Nov 16 at 13:51
















            This is a great answer, not only you improve the code by re-formulating it in a better way but also you elaborate some c++ trick - I am not a c++ developer - this is very helpful! Thanks a lot.
            – Michael
            Nov 16 at 13:51




            This is a great answer, not only you improve the code by re-formulating it in a better way but also you elaborate some c++ trick - I am not a c++ developer - this is very helpful! Thanks a lot.
            – Michael
            Nov 16 at 13:51












            up vote
            2
            down vote













            frontInd is never declared. It's only ever assigned to, so that assignment can just be removed.



            Avoid using namespace std - that's not a namespace that's designed to be imported wholesale, and you could introduce surprising bugs that way. Given that we have a single use of std::cout and nothing else from the standard library, it's actually costing more code lines/characters (as well as more cognitive overhead) than just writing the prefix where needed.



            Inseparable data should be contained in a class. Here, backInd and Queue belong together; no outside code ever needs to use backInd, so it could be a private member. That said, backInd is always equal to x when writing, so we could just index that way.



            Consider an alternative algorithm - the current approach will be slow when the number of operations gets large. Consider looping over the operations just once, recording the delta of value at each position (so each operation will update just two values - an increment at the beginning of its range and a corresponding decrement at the end). Then track the cumulative total when writing the output.






            share|improve this answer



























              up vote
              2
              down vote













              frontInd is never declared. It's only ever assigned to, so that assignment can just be removed.



              Avoid using namespace std - that's not a namespace that's designed to be imported wholesale, and you could introduce surprising bugs that way. Given that we have a single use of std::cout and nothing else from the standard library, it's actually costing more code lines/characters (as well as more cognitive overhead) than just writing the prefix where needed.



              Inseparable data should be contained in a class. Here, backInd and Queue belong together; no outside code ever needs to use backInd, so it could be a private member. That said, backInd is always equal to x when writing, so we could just index that way.



              Consider an alternative algorithm - the current approach will be slow when the number of operations gets large. Consider looping over the operations just once, recording the delta of value at each position (so each operation will update just two values - an increment at the beginning of its range and a corresponding decrement at the end). Then track the cumulative total when writing the output.






              share|improve this answer

























                up vote
                2
                down vote










                up vote
                2
                down vote









                frontInd is never declared. It's only ever assigned to, so that assignment can just be removed.



                Avoid using namespace std - that's not a namespace that's designed to be imported wholesale, and you could introduce surprising bugs that way. Given that we have a single use of std::cout and nothing else from the standard library, it's actually costing more code lines/characters (as well as more cognitive overhead) than just writing the prefix where needed.



                Inseparable data should be contained in a class. Here, backInd and Queue belong together; no outside code ever needs to use backInd, so it could be a private member. That said, backInd is always equal to x when writing, so we could just index that way.



                Consider an alternative algorithm - the current approach will be slow when the number of operations gets large. Consider looping over the operations just once, recording the delta of value at each position (so each operation will update just two values - an increment at the beginning of its range and a corresponding decrement at the end). Then track the cumulative total when writing the output.






                share|improve this answer














                frontInd is never declared. It's only ever assigned to, so that assignment can just be removed.



                Avoid using namespace std - that's not a namespace that's designed to be imported wholesale, and you could introduce surprising bugs that way. Given that we have a single use of std::cout and nothing else from the standard library, it's actually costing more code lines/characters (as well as more cognitive overhead) than just writing the prefix where needed.



                Inseparable data should be contained in a class. Here, backInd and Queue belong together; no outside code ever needs to use backInd, so it could be a private member. That said, backInd is always equal to x when writing, so we could just index that way.



                Consider an alternative algorithm - the current approach will be slow when the number of operations gets large. Consider looping over the operations just once, recording the delta of value at each position (so each operation will update just two values - an increment at the beginning of its range and a corresponding decrement at the end). Then track the cumulative total when writing the output.







                share|improve this answer














                share|improve this answer



                share|improve this answer








                edited Nov 16 at 13:51

























                answered Nov 16 at 13:45









                Toby Speight

                21.9k536108




                21.9k536108






























                     

                    draft saved


                    draft discarded



















































                     


                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function () {
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f207799%2fsimple-array-operation-using-a-queue%23new-answer', 'question_page');
                    }
                    );

                    Post as a guest















                    Required, but never shown





















































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown

































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown







                    Popular posts from this blog

                    Quarter-circle Tiles

                    build a pushdown automaton that recognizes the reverse language of a given pushdown automaton?

                    Mont Emei