This a collection of tips and feedbacks for COMP2710 Software Construction Spring 2018.
-
-
Save gongzhitaao/f73396dbe1e6b76e060e5e17de155176 to your computer and use it in GitHub Desktop.
Some comments about HW1.
- MAKE SURE YOU PROGRAM COMPILES ON LINUX. For example,
system("pause")
is Windows specific. - implicit conversion. Please be aware of implicit conversion in C++.
For example
int a = 0, b = 1, c = 2; a = b / c; // a is 0 a = 2.0 * (b / c); // a is 0 a = 2.0 * b / c; // a is 1
- File encoding. Make sure that your cpp files are in ASCII encoding. If you copy and paste code snippets from website (which is fine), chances are you might be copying some non-printable Unicode characters as well.
- Over-engineering is bad. I could see that some students are really trying to modularize the code. Instead of a 20-line code, we see a file of about 100 lines. I'm not saying there is anything wrong about the added functionality and encapsulation (if any). However more thoughts are definitely needed over simplicity, functionality, code structure, and etc. For this simple problem whatever you do probably does not matter much. In a large code base, however, these trade-offs should be considered with caution.
Some feedbacks for HW2.
- MAKE SURE YOUR SOURCE FILE COMPILES.
- There is NO dollar sign before the rate.
- Be aware of the edge cases where the monthly payment is lower than the interest.
General tips on C++.
-
bool
andint
values can be implicitly converted to each other.bool
toint
:false
is 0, andtrue
is 1.int
tobool
: 0 isfalse
, non-zero istrue
.
So
at_least_two_alive
can be implemented asbool at_least_two_alive(bool a, bool b, bool c) { return a + b + c >= 2; }
-
When testing a boolean value, do not use
if (a == true)
orif (true == a)
, simply useif (a)
instead. -
When returning a boolean value from a function, instead of
if (a) return true; else return false;
Use
return a;
-
Configure your editor to use tabs or spaces consistently. DO NOT MIX TABS AND SPACES.
#include <fstream> | |
#include <iostream> | |
#include <vector> | |
using namespace std; | |
vector<int> merge(const vector<int>& v0, const vector<int>& v1) | |
{ | |
vector<int> ret(v0.size() + v1.size(), 0); | |
int i, j, k; | |
for (i = 0, j = 0, k = 0; i < v0.size() && j < v1.size(); ++k) | |
if (v0[i] < v1[j]) | |
ret[k] = v0[i++]; | |
else | |
ret[k] = v1[j++]; | |
for (; i < v0.size(); ++i, ++k) ret[k] = v0[i]; | |
for (; j < v1.size(); ++j, ++k) ret[k] = v1[j]; | |
return ret; | |
} | |
int main() | |
{ | |
string fname; | |
ifstream fin; | |
int val; | |
cin >> fname; | |
fin.open(fname); | |
vector<int> arr1; | |
while (fin >> val) arr1.push_back(val); | |
fin.close(); | |
cin >> fname; | |
fin.open(fname); | |
vector<int> arr2; | |
while (fin >> val) arr2.push_back(val); | |
fin.close(); | |
vector<int> res = merge(arr1, arr2); | |
cin >> fname; | |
ofstream fout(fname); | |
for (int n : res) fout << n << endl; | |
fout.close(); | |
return 0; | |
} |
Since we are using C++, please upgrade your code from C to C++. And C++11 has been around for quite a while, since 2011!! Even C++14 was approved back in 2014. GCC 6.1 supports C++14 with GNU extensions by default. And GCC 6.1 was released on April 27, 2016. Please upgrade your code from C++03 to C++11 as well.
- Avoid
.h
headers. C++ headers are like<iostream>
,<algorithm>
,<vector>
, etc. You probably will never need something like<something.h>
. - Prefer
vector
over raw array.vector
is a dynamic-sized array, which automatically grows as needed, you don’t need to worry about the maximum size.- Accessing individual elements is the same as the raw array.
- Many convenient functions, e.g.,
size()
,empty()
, etc.
- Prefer
string
over raw char array.std::basic_fstream::open()
takesstring
as input since C++11, so you do not need thec_str()
conversion if you use C++11. - Mixing
cin
andgetline
is MESSY. A simple solution iscin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
after you docin
, which ignores everything before a newline and the newline itself. - Avoid
eof()
to test the end of input. Just Google “eof c++ not working”.eof()
does not return true until after a failed reading.- You may run into other problems when reading a stream, e.g., incompatible types.
Just test the stream itself, e.g.,
while (cin >> a >> b)
.
- Define structure.
struct Node { int val; Node *next; Node(int v = 0) : val(v), next(nullptr) {} };
- Segment fault usually means you are visiting an address that does not belong to you program.
- Memory leak problem.
struct Node { int val; Node *next; Node(int v = 0) : val(v), next(nullptr) {} }; int main() { Node *node = new Node(13); // do something with node delete node; return 0; }
Use shared_ptr instead.
#include <iostream> | |
using namespace std; | |
class PrimeNumber | |
{ | |
public: | |
PrimeNumber(int n = 2) : n_(n) {} | |
friend ostream &operator<<(ostream &os, const PrimeNumber &pn); | |
PrimeNumber &operator++(); // prefix | |
PrimeNumber operator++(int); // postfix | |
PrimeNumber &operator--(); // prefix | |
PrimeNumber operator--(int); // postfix | |
private: | |
bool isprime(int n); | |
int next_prime(int n); | |
int prev_prime(int n); | |
private: | |
int n_; | |
}; | |
ostream &operator<<(ostream &os, const PrimeNumber &pn) | |
{ | |
os << pn.n_; | |
return os; | |
} | |
PrimeNumber &PrimeNumber::operator++() | |
{ | |
n_ = next_prime(n_); | |
return *this; | |
} | |
PrimeNumber PrimeNumber::operator++(int) | |
{ | |
PrimeNumber p = *this; | |
++*this; | |
return p; | |
} | |
PrimeNumber &PrimeNumber::operator--() | |
{ | |
n_ = prev_prime(n_); | |
return *this; | |
} | |
PrimeNumber PrimeNumber::operator--(int) | |
{ | |
PrimeNumber p = *this; | |
--*this; | |
return p; | |
} | |
int PrimeNumber::next_prime(int n) | |
{ | |
if (2 == n) return 3; | |
for (n += 2; !isprime(n); n += 2) | |
; | |
return n; | |
} | |
int PrimeNumber::prev_prime(int n) | |
{ | |
if (3 == n) return 2; | |
for (n -= 2; n > 1 && !isprime(n); n -= 2) | |
; | |
return n > 1 ? n : 2; | |
} | |
bool PrimeNumber::isprime(int n) | |
{ | |
for (int i = 2; i * i <= n; ++i) | |
if (n % i == 0) return false; | |
return true; | |
} | |
int main() | |
{ | |
PrimeNumber pn; | |
cout << pn << endl; | |
for (int i = 0; i < 10; ++i) | |
cout << pn++ << endl; | |
for (int i = 0; i < 10; ++i) | |
cout << pn-- << endl; | |
for (int i = 0; i < 10; ++i) | |
cout << ++pn << endl; | |
for (int i = 0; i < 10; ++i) | |
cout << pn-- << endl; | |
cout << pn << endl; | |
return 0; | |
} |