Need help with logic errors



  • Hi
    I am designing a data structure than can efficiently store and check if total of any three successively added elements is equal to given total.

    For example MovingTotal() creates an empty container with no existing totals.append([1,2,3]) appends elements [1,2,3] which means that there is only one exisitng total (1+2+3 = 6) , append ([4]) appends element 4 and creates additional total from [2,3,4]. There would now be two totals (1+2+3 = 6 and 2+3+4 =9). At this point contains(6) and contains(9) should return True, while contains(7) should return False.

    `` cpp
    #include <vector>
    #include <stdexcept>
    #include <iostream>
    #include <string>

    class MovingTotal{
    private:
    std::vector<int> list = std::vector<int>();
    public:
    virtual void append(std::vector<int> &list);
    virtual bool contains(int total);
    static void main(std::vectorstd::wstring & args);
    };

    void MovingTotal::append(const std::vector<int> addAll& list)
    {
    list.addAll(Arrays::asList(list));
    throw std::logic_error("Waiting for implement");
    }

    bool MovingTotal:: contains(int total)
    {
    if((list[0] + list[1] + list[2]) == total)
    {

     return true;
    

    }
    return false;
    throw std::logic_error("Waiting for implement");
    }

    #ifndef RunTests
    int main()
    {
    MovingTotal *movingTotal = new MovingTotal();
    movingTotal->append(std::vector<int> {1,2,3});
    std::vector<int> first;
    first.push_back(1);
    first.push_back(2);
    first.push_back(3);

    movingTotal->append(first);
    std::wcout << movingTotal->contains(6) << std::endl;
    std::wcout << movingTotal->contains(9) << std::endl;

    std::vector<int> second;
    second.push_back(4);
    movingTotal->append(second);
    std::wcout << movingTotal->contains(9) << std::endl;
    }
    #endif
    ``

    Below are compilation errors:

    void MovingTotal::append(const std::vector<int> addAll& list)
    ^
    main.cpp:23:6: error: prototype for ‘void MovingTotal::append(std::vector)’ does not match any in class ‘MovingTotal’
    void MovingTotal::append(const std::vector<int> addAll& list)
    ^~~~~~~~~~~
    main.cpp:18:19: error: candidate is: virtual void MovingTotal::append(std::vector&)
    virtual void append(std::vector<int> &list);
    ^~~~~~
    main.cpp: In function ‘int main()’:
    main.cpp:44:29: error: cannot bind non-const lvalue reference of type ‘std::vector&’ to an rvalue of type ‘std::vector’
    movingTotal->append(std::vector<int> {1,2,3});
    ^~~~~~~~~~~~~~~~~~~
    main.cpp:18:19: note: initializing argument 1 of ‘virtual void MovingTotal::append(std::vector&)’
    virtual void append(std::vector<int> &list);
    ^~~~~~



  • @tampere2021 sagte in Need help with logic errors:

    void MovingTotal::append(const std::vector<int> addAll& list)

    What is the name of the parameter? addAll? list? Any of them? Both? => A parameter can have just one name.

    list.addAll(Arrays::asList(list));

    There is no addAll in a std::vector, there is no Arrays::asList. Are you thinking in Java?

    static void main(std::vectorstd::wstring & args);

    [public] static void main - looks like Java, not C++.

    bool MovingTotal:: contains(int total)
    {
    if((list[0] + list[1] + list[2]) == total)
    {
    
     return true;
    }
    return false;
    throw std::logic_error("Waiting for implement");
    }
    

    An if (condition) return true; [else] return false; can be rewritten as return condition;. The throw is unreachable.

    MovingTotal *movingTotal = new MovingTotal();

    Why a pointer? Avoid new (and memory leaks)!
    MonvingTotal movingTotal; - done!



  • @wob I have modified my code again. Also implemented the logic for "addAll" myself. please check it and suggest any changes needed? If I run below code, it has compilation errors:

    main.cpp:33:15: error: ‘class std::vector’ has no member named ‘addAll’
        this->list.addAll(Arrays::asList(list));
                   ^~~~~~
    main.cpp:33:22: error: ‘Arrays’ has not been declared
        this->list.addAll(Arrays::asList(list));
                          ^~~~~~
    
    #include <vector>
    #include <stdexcept>
    #include <iostream>
    #include <string>
    
    class MovingTotal{
    private:
          std::vector<int> list = std::vector<int>();
    public:
         virtual void append(const std::vector<int> &list);
         virtual bool contains(int total);
         static void main(std::vector<std::wstring> & args);
    };
    
    void addAll(std::vector<int>& vec, const std::vector<int>& addition)
    {
        for (const auto& element : addition)
        {
            vec.push_back(element);
        }
    }
    
    void MovingTotal::append(const std::vector<int> &list)
    {
       this->list.addAll(Arrays::asList(list));
       throw std::logic_error("Waiting for implement");
    }
    
    bool MovingTotal:: contains(int total)
    {
      if((list[0] + list[1] + list[2]) == total)
      {
    
         return true;
      }
      return false;
      throw std::logic_error("Waiting for implement");
    }
    
    #ifndef RunTests
    int main()
    {
       MovingTotal *movingTotal = new MovingTotal();
       movingTotal->append(std::vector<int> {1,2,3});
       std::vector<int> first;
       first.push_back(1);
       first.push_back(2);
       first.push_back(3);
       
       movingTotal->append(first);
       std::wcout << movingTotal->contains(6) << std::endl;
       std::wcout << movingTotal->contains(9) << std::endl;
    
       std::vector<int> second;
       second.push_back(4);
       movingTotal->append(second);
       std::wcout << movingTotal->contains(9) << std::endl;
    }
    #endif 
    


  • This is wrong in so many places 😱 Where did you get this assignment from?

    The best container to use for a moving total is probably a circular buffer. Unfortunately this is missing in the STL, but boost has one. If you want to stick to the STL you can use std::deque which offers fast insertion/removal from both start and end. std::list is okay if you just use it to keep track of elements, you have to implement some internal logic though.
    Naming a std::vector variable list is a bit unfortunate since it can be mistaken for std::list.
    Instead of correcting all your errors I present a solution the way I´d implement it (not templated version, just for int):

    Edit:
    This a pure C++ problem, why did you post it in this subforum?

    #include <deque>
    #include <cassert>
    
    class MovingTotal
    {
        std::deque<int> Elements_;
        std::size_t Capacity_ = 1;
        int Total_ = 0;
    public:
       MovingTotal() = default;
       MovingTotal( std::size_t capacity ) :
          Capacity_( capacity )
       {
          assert( capacity > 0 );
       }
    
       int total() const
       {
          return Total_;
       }
    
       bool empty() const
       {
          return Elements_.empty();
       }
    
       std::size_t size() const
       {
          return Elements_.size();
       }
    
       std::size_t capacity() const
       {
          return Capacity_;
       }
    
       int push_back( int value )
       {
          Total_ += value;
          if( capacity() == size() )
          {
             Total_ -= Elements_.front();
             Elements_.pop_front();
          }
          Elements_.push_back( value );
          return total();
       }
    };
    

    Edit: Fixed
    But honestly, those errors were trivial, you should be able to fix them yourself.



  • @DocShoe sagte in Need help with logic errors:

    #include <deque>

    class MovingTotal
    {
    std::deque<int> Elements_;
    std::size_t Capacity_ = 1;
    int Total_ = 0;
    public:
    MovingTotal() = default;
    MovingTotal( std::size_t capaity ) :
    Capacity_( capacity )
    {
    assert( capacity > 0 );
    }

    int total() const
    {
    return Total_;
    }

    bool empty() const
    {
    return Elements_.empty();
    }

    std::size_t size() const
    {
    return Elements_.size();
    }

    int push_back( int value )
    {
    Total_ += value;
    if( capacity() == size() )
    {
    Total_ -= Elements_.front();
    Elements_.pop_front();
    }
    Elements_.push_back( value );
    return total();
    }
    }

    After i run your code, it throws lot of compilation errors.

    
     }
      ^
      ;
    main.cpp: In constructor ‘MovingTotal::MovingTotal(std::size_t)’:
    main.cpp:19:18: error: ‘capacity’ was not declared in this scope
           Capacity_( capacity )
                      ^~~~~~~~
    main.cpp:19:18: note: suggested alternative: ‘capaity’
           Capacity_( capacity )
                      ^~~~~~~~
                      capaity
    main.cpp:21:7: error: ‘assert’ was not declared in this scope
           assert( capacity > 0 );
           ^~~~~~
    main.cpp:21:7: note: suggested alternative: ‘short’
           assert( capacity > 0 );
           ^~~~~~
           short
    main.cpp: In member function ‘int MovingTotal::push_back(int)’:
    main.cpp:42:11: error: ‘capacity’ was not declared in this scope
           if( capacity() == size() )
               ^~~~~~~~
    main.cpp:42:11: note: suggested alternative: ‘Capacity_’
           if( capacity() == size() )
               ^~~~~~~~
               Capacity_
    ``


  • @tampere2021 As you may see, theres obviously a typing in @DocShoe's code.
    replace capaity with capacity

    And, you have to include <assert.h>



  • Irgendetwas scheint doch nicht mit dem User (bzw. Account) zu stimmen: zuerst möchte er eine eigene TGA-Lib entwickeln, dann einen Konsolen-Shooter. Plötzlich die eigenartige Anfrage bzgl. der Perfomance bei virtuellen Funktionen sowie Umschwenken auf GLSL/HLSL-Programmierung - irgendwie alles ein bißchen zu viel in kurzer Zeit - und jetzt auf einmal kann er noch nicht einmal die einfachsten Codes programmieren (bzw. Fehler finden)?!!


  • Mod

    @Th69 sagte in Need help with logic errors:

    Irgendetwas scheint doch nicht mit dem User (bzw. Account) zu stimmen: zuerst möchte er eine eigene TGA-Lib entwickeln, dann einen Konsolen-Shooter. Plötzlich die eigenartige Anfrage bzgl. der Perfomance bei virtuellen Funktionen sowie Umschwenken auf GLSL/HLSL-Programmierung - irgendwie alles ein bißchen zu viel in kurzer Zeit - und jetzt auf einmal kann er noch nicht einmal die einfachsten Codes programmieren (bzw. Fehler finden)?!!

    Sozialwissenschaftliche Studie, wie lange man Hilfsbereitschaft ausnutzen kann? Dass einfachste Codes nicht verstanden werden, sollte man aber sofort bemerkt haben.



  • @Th69 Ein kleiner 😈 ?


Log in to reply