Show Lecture.DRY as a slide show.
CS253 DRY
Definition
D R Y
Don’t Repeat Yourself
Definition
W E T
Write Everything Twice
Repetition is bad
- Repetition of code is a bad thing.
- Copy & paste are quite tempting, like the Dark Side.
- Dual versions fall out of sync. One version gets a bug fix,
the other doesn’t. Later readers won’t know which version is right.
- If function
a()
does something quite similar to function b()
,
don’t just copy & paste a()
to b()
, and tweak b()
.
- Instead, perhaps:
b()
could call a()
- add another argument to
a()
to make it do the other thing.
- getline() has an optional end-of-line argument,
default is
'\n'
.
- the real work could be done in
c()
,
and both a()
and b()
call c()
.
- “You expect me to write another function‽ I’m scared of functions!”
Example
Here’s code to see if a number is present in a vector.
Apparently, the programmer didn’t know about set or the
find() algorithm.
bool present(const vector<int> &vec, int target) {
for (auto v : vec)
if (v == target)
return true;
return false;
}
int main() {
vector a = {11,22,33};
cout << boolalpha << present(a, 22) << '\n'
<< present(a, 44) << "\n\n";
}
true
false
Example
We also need to find if a value is absent.
That’s easy—copy & paste, change ==
to !=
.
🦡
bool present(const vector<int> &vec, int target) {
for (auto v : vec)
if (v == target)
return true;
return false;
}
bool absent(const vector<int> &vec, int target) {
for (auto v : vec)
if (v != target) // Changed == to !=
return true;
return false;
}
int main() {
vector a = {11,22,33};
cout << boolalpha << present(a, 22) << '\n'
<< present(a, 44) << '\n'
<< absent(a, 33) << "\n\n";
}
true
false
true
absent()
is incorrect; 33 is not absent.
Try, try, again
Let’s try this:
bool present(const vector<int> &vec, int target) {
for (auto v : vec)
if (v == target)
return true;
return false;
}
bool absent(const vector<int> &vec, int target) {
for (auto v : vec)
if (v == target)
return false; // changed true to false
return true; // changed false to true
}
int main() {
vector a = {11,22,33};
cout << boolalpha << present(a, 22) << '\n'
<< present(a, 44) << '\n'
<< absent(a, 33) << "\n\n";
}
true
false
false
Better, but WET.
Not good enough
Sure, the previous solution is correct, but it’s not
maintainable. What if we have to modify present()
,
say, to treat negative values as equal to positive values?
Will we remember to also modify absent()
?
Better
bool present(const vector<int> &vec, int target) {
for (auto v : vec)
if (v == target)
return true;
return false;
}
bool absent(const vector<int> &vec, int target) {
return !present(vec, target);
}
int main() {
vector a = {11,22,33};
cout << boolalpha << present(a, 22) << '\n'
<< present(a, 44) << '\n'
<< absent(a, 33) << "\n\n";
}
true
false
false
- Correct, and DRY.
- You think it’s slower?
Do you remember our talk about inlining?
Using references
As we all know, a reference creates an alias for another thing.
For example:
int a = 42;
int &r = a;
cout << "r=" << r << " a=" << a << '\n';
a = 99;
cout << "r=" << r << " a=" << a << '\n';
r = 56;
cout << "r=" << r << " a=" << a << '\n';
r=42 a=42
r=99 a=99
r=56 a=56
Loops
Let’s say that we want to change all the fives in this
vector to negative fives:
vector values = {3,1,4,1,5,9,2,6,5,3,5};
for (size_t i=0; i<values.size(); i++)
if (values[i] == 5)
values[i] = -5;
for (size_t i=0; i<values.size(); i++)
cout << values[i] << ' ';
3 1 4 1 -5 9 2 6 -5 3 -5
That works, but it’s a lot of repetition—quite a few instances of
values[i]
.
Better loops
Let’s use a more modern loop to write out the numbers:
vector values = {3,1,4,1,5,9,2,6,5,3,5};
for (size_t i=0; i<values.size(); i++)
if (values[i] == 5)
values[i] = -5;
for (int v : values)
cout << v << ' ';
3 1 4 1 -5 9 2 6 -5 3 -5
Hooray! One fewer values[i]
!
Betterer loops
Let’s use that same technique to change 5 to −5:
vector values = {3,1,4,1,5,9,2,6,5,3,5};
for (int v : values)
if (v == 5)
v = -5;
for (int v : values)
cout << v << ' ';
3 1 4 1 5 9 2 6 5 3 5
That didn’t work. Why?
Best loops
REFERENCES!
vector values = {3,1,4,1,5,9,2,6,5,3,5};
for (int &v : values)
if (v == 5)
v = -5;
for (int v : values)
cout << v << ' ';
3 1 4 1 -5 9 2 6 -5 3 -5
I need &v
in the first loop, because I’m changing the number.
I’m not changing anything in the second loop, so a reference
isn’t required.
C🙈ns🙈rsh🙈p
- Bored people get really worried about what other people
say, write, draw, or film.
- Mind your own business. It’s fine to control your own actions,
but don’t try to tell me what to do. Control your own “offense”.
- One way to censor words is to obscure certain letters.
- This way, we don’t actually write the scary word, but everybody
knows what we mean.
- This is viewed as better, somehow.
- It is, in fact, a pile of ✖️✖️✖️✖️.
Ducks
Consider this file:
% cat ~cs253/pub/ducks
Huey (red)
Dewey (blue)
Louie (green)
I want code that reads the lines of that file into a vector<string>
, and
converts the lower-case vowels to asterisks, so it looks like censorship.
Attempt #1
const string home = getpwnam("cs253")->pw_dir;
ifstream in(home+"/pub/ducks");
vector<string> ducks;
for (string line; getline(in, line); ducks.push_back(line));
for (size_t i=0; i<ducks.size(); i++)
for (size_t j=0; j<ducks[i].size(); j++)
if (ducks[i][j]=='a' || ducks[i][j]=='e' ||
ducks[i][j]=='i' || ducks[i][j]=='o' ||
ducks[i][j]=='u')
ducks[i][j] = '*';
for (size_t i=0; i<ducks.size(); i++)
cout << ducks[i] << '\n';
H**y (r*d)
D*w*y (bl**)
L**** (gr**n)
That was horrible. So soon, we’ve forgotten all that we’ve learned.
Attempt #2
const string home = getpwnam("cs253")->pw_dir;
ifstream in(home+"/pub/ducks");
vector<string> ducks;
for (string line; getline(in, line); ducks.push_back(line));
for (string &s : ducks)
for (size_t j=0; j<s.size(); j++)
if (s[j]=='a' || s[j]=='e' ||
s[j]=='i' || s[j]=='o' ||
s[j]=='u')
s[j] = '*';
for (string s : ducks)
cout << s << '\n';
H**y (r*d)
D*w*y (bl**)
L**** (gr**n)
Better, but still awful. What do we see a lot of?
Attempt #3
const string home = getpwnam("cs253")->pw_dir;
ifstream in(home+"/pub/ducks");
vector<string> ducks;
for (string line; getline(in, line); ducks.push_back(line));
for (string &s : ducks)
for (char &c : s)
if (c=='a' || c=='e' || c=='i' || c=='o' || c=='u')
c = '*';
for (string s : ducks)
cout << s << '\n';
H**y (r*d)
D*w*y (bl**)
L**** (gr**n)
Finally, it’s DRY! Still, we’re copying strings in the last loop.
Attempt #4
const string home = getpwnam("cs253")->pw_dir;
ifstream in(home+"/pub/ducks");
vector<string> ducks;
for (string line; getline(in, line); ducks.push_back(line));
for (string &s : ducks)
for (char &c : s)
if (c=='a' || c=='e' || c=='i' || c=='o' || c=='u')
c = '*';
for (const string &s : ducks)
cout << s << '\n';
H**y (r*d)
D*w*y (bl**)
L**** (gr**n)
DRY and efficient.
Classic WET code
Java programmers used to .getFirst()
and .getNext()
often produce code like this:
ifstream input("/etc/hostname");
string line;
getline(input, line); // 🦡
while (input) {
cout << "Read this: " << line;
getline(input, line); // 🦡
}
Read this: beethoven
The duplicated calls to getline() are not very DRY, are they?
DRY version
In this DRY version, the two calls to getline() were merged
into the condition:
ifstream input("/etc/hostname");
string line;
while (getline(input, line))
cout << "Read this: " << line;
Read this: beethoven
Or, even better, restrict the scope of line
:
ifstream input("/etc/hostname");
for (string line; getline(input, line); )
cout << "Read this: " << line;
Read this: beethoven
Summary
- If you find that you have common code, code that occurs several
times, then you have a problem.
- Don’t feel bad—this just happens naturally, as code evolves.
- If the code is repeated too many times, deal with it:
- If a value is fetched several times, maybe save it in a variable.
- You might use a reference to refer to an oft-used location.
- Perhaps the common code deserves its own function.