#Missing headers #include #include #include #include

#Interface

• I don't think operator<<() can be noexcept, unless you're not using the std::ostream argument!

• Is there a good reason that the initializer_list constructor requires std::vector arguments? The code only requires arguments that are enumerable with range-based for.

• I don't think the findIndex() method ought to be public. It's an implementation detail that client code shouldn't need to be aware of; making it public may be a sign of missing functionality that clients need. I don't know if Dim() and nnz() need to be public, but either way, they could have better names.

• Consider making operator()() return a (possibly const-qualified) reference, to make the interface more like standard collections.

• The return types of public methods should be complete (auto without a trailing return type prevents the methods being called before their definitions are in scope).

• There's no benefit to const on arguments passed by value in the interface. Go ahead and use them in the definitions, of course:

    class MSRmatrix
    {
    public:
        // declaration
        const element_type operator()(itype, itype) const noexcept;
    };
  
    // definition
    const element_type MSRmatrix::operator()(const itype, const itype) const noexcept
    {
        // code...
    }
  

• It's probably worth declaring using value_type = element_type like the standard containers - it makes it easier to use in generic code. And consider renaming itype to size_type for the same reason.

• It should be possible to implement print() and toFile() as non-member non-friend functions. If not, consider adding suitable public methods to implement them.

• operator+() wouldn't need to be a friend if you provide operator+= as a member.

• The value enum is never used.

#Implementation ##Constructor

• Don't exit() from within the constructor. Prefer to throw an exception instead - that way the calling code can choose whether to emit output (and if so, by what means) and whether it can recover from the error.
• There's a redundant assignment this->dim = row.size() - the initializer has done its job by the time we get here.
• The temp vector is assigned but never used.
• The index variables can be declared with smaller scope.

#Missing headers #include #include #include #include

#Interface

• I don't think operator<<() can be noexcept, unless you're not using the std::ostream argument!

• Is there a good reason that the initializer_list constructor requires std::vector arguments? The code only requires arguments that are enumerable with range-based for.

• I don't think the findIndex() method ought to be public. It's an implementation detail that client code shouldn't need to be aware of; making it public may be a sign of missing functionality that clients need. I don't know if Dim() and nnz() need to be public, but either way, they could have better names.

• Consider making operator()() return a (possibly const-qualified) reference, to make the interface more like standard collections.

• The return types of public methods should be complete (auto without a trailing return type prevents the methods being called before their definitions are in scope).

• There's no benefit to const on arguments passed by value in the interface. Go ahead and use them in the definitions, of course:

    class MSRmatrix
    {
    public:
        // declaration
        const element_type& operator()(itype, itype) const noexcept;
    };
  
    // definition
    const element_type& MSRmatrix::operator()(const itype, const itype) const noexcept
    {
        // code...
    }
  

• It's probably worth declaring using value_type = element_type like the standard containers - it makes it easier to use in generic code. And consider renaming itype to size_type for the same reason.

• It should be possible to implement print() and toFile() as non-member non-friend functions. If not, consider adding suitable public methods to implement them.

• operator+() wouldn't need to be a friend if you provide operator+= as a member.

• The value enum is never used.

#Implementation ##Constructor

• Don't exit() from within the constructor. Prefer to throw an exception instead - that way the calling code can choose whether to emit output (and if so, by what means) and whether it can recover from the error.
• There's a redundant assignment this->dim = row.size() - the initializer has done its job by the time we get here.
• The temp vector is assigned but never used.
• The index variables can be declared with smaller scope.

#Missing headers #include #include #include #include

#Interface

• I don't think operator<<() can be noexcept, unless you're not using the std::ostream argument!

• Is there a good reason that the initializer_list constructor requires std::vector arguments? The code only requires arguments that are enumerable with range-based for.

• I don't think the findIndex() method ought to be public. It's an implementation detail that client code shouldn't need to be aware of; making it public may be a sign of missing functionality that clients need. I don't know if Dim() and nnz() need to be public, but either way, they could have better names.

• Consider making operator()() return a (possibly const-qualified) reference, to make the interface more like standard collections.

• There's no benefit to const on arguments passed by value in the interface. Go ahead and use them in the definitions, of course:

    class MSRmatrix
    {
    public:
        // declaration
        const element_type operator()(itype, itype) const noexcept;
    };
  
    // definition
    const element_type MSRmatrix::operator()(const itype, const itype) const noexcept
    {
        // code...
    }
  

• It's probably worth declaring using value_type = element_type like the standard containers - it makes it easier to use in generic code. And consider renaming itype to size_type for the same reason.

• It should be possible to implement print() and toFile() as non-member non-friend functions. If not, consider adding suitable public methods to implement them.

• operator+() wouldn't need to be a friend if you provide operator+= as a member.

• The value enum is never used.

#Implementation ##Constructor

• Don't exit() from within the constructor. Prefer to throw an exception instead - that way the calling code can choose whether to emit output (and if so, by what means) and whether it can recover from the error.
• There's a redundant assignment this->dim = row.size() - the initializer has done its job by the time we get here.
• The temp vector is assigned but never used.
• The index variables can be declared with smaller scope.

#Missing headers #include #include #include #include

#Interface

• I don't think operator<<() can be noexcept, unless you're not using the std::ostream argument!

• Is there a good reason that the initializer_list constructor requires std::vector arguments? The code only requires arguments that are enumerable with range-based for.

• I don't think the findIndex() method ought to be public. It's an implementation detail that client code shouldn't need to be aware of; making it public may be a sign of missing functionality that clients need. I don't know if Dim() and nnz() need to be public, but either way, they could have better names.

• Consider making operator()() return a (possibly const-qualified) reference, to make the interface more like standard collections.

• The return types of public methods should be complete (auto without a trailing return type prevents the methods being called before their definitions are in scope).

• There's no benefit to const on arguments passed by value in the interface. Go ahead and use them in the definitions, of course:

    class MSRmatrix
    {
    public:
        // declaration
        const element_type operator()(itype, itype) const noexcept;
    };
  
    // definition
    const element_type MSRmatrix::operator()(const itype, const itype) const noexcept
    {
        // code...
    }
  

• It's probably worth declaring using value_type = element_type like the standard containers - it makes it easier to use in generic code. And consider renaming itype to size_type for the same reason.

• It should be possible to implement print() and toFile() as non-member non-friend functions. If not, consider adding suitable public methods to implement them.

• operator+() wouldn't need to be a friend if you provide operator+= as a member.

• The value enum is never used.

#Implementation ##Constructor

• Don't exit() from within the constructor. Prefer to throw an exception instead - that way the calling code can choose whether to emit output (and if so, by what means) and whether it can recover from the error.
• There's a redundant assignment this->dim = row.size() - the initializer has done its job by the time we get here.
• The temp vector is assigned but never used.
• The index variables can be declared with smaller scope.