Homework1 #71
Homework1 #71
Conversation
| #pragma once | ||
|
|
||
| #include <cstring> | ||
| #include <iostream> |
There was a problem hiding this comment.
Why is iostream needed here? You should exclusively include what is needed , nothing else
| #include <cstring> | ||
| #include <iostream> | ||
| #include <memory> | ||
| #include <ostream> |
There was a problem hiding this comment.
In general IOs related header files are heavy and you should include declarations (iosfwd) in header files, definitions (like ostream) in cc. This will also force you to move your class method efinition to the cc, which is better.
|
|
||
| private: | ||
| const std::size_t mSize; | ||
| const std::unique_ptr<char[]> mContents; |
There was a problem hiding this comment.
It was supposed to be on the stack
| public: | ||
| String(const char s[]) : mSize(std::strlen(s)), mContents(std::make_unique<char[]>(mSize)) | ||
| { | ||
| std::strcpy(mContents.get(), s); |
There was a problem hiding this comment.
This might overflow, you should check the length and throw before copying. I wouldnt use strcpy, it's a C function exported to the std:: NS - we would always prefer algorithms like std::copy
There was a problem hiding this comment.
In general the convention would to pass const char* str, size_t len, not just a pointer or array
| { | ||
| std::strcpy(mContents.get(), s); | ||
| } | ||
| String() : String("") {} |
There was a problem hiding this comment.
Default ctor should be a no-op - this one allocates, which isnt great
| return (std::strncmp(c_str(), substr.c_str(), substr.Size()) == 0); | ||
| } | ||
|
|
||
| const bool EndsWith(const String substr) const |
| const char *c_str() const { return mContents.get(); }; | ||
|
|
||
| // mainly added these two to help with testing operator+ | ||
| const bool StartsWith(const String substr) const |
| const std::unique_ptr<char[]> mContents; | ||
| }; | ||
|
|
||
| std::ostream &operator<<(std::ostream &stream, const String &str) |
There was a problem hiding this comment.
spaces: std::ostream& .. not std::ostream &
| char tmpstr[str1.Size() + str2.Size()]; | ||
| std::strcpy(tmpstr, str1.c_str()); | ||
| std::strcat(tmpstr, str2.c_str()); | ||
| return String(tmpstr); |
There was a problem hiding this comment.
This should be in the cc file. Also all these C functions are unsafe (risk of buffer overflow).
| // extra letter at the start | ||
| assert(!addedstr.EndsWith("TThis is a different string")); | ||
|
|
||
| } |
Might add more if I think of anything interesting to add.