Show Lecture.DRY as a slide show.

CS253 DRY

Definition

Definition

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.

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 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.

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.

Attempt #5

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 p=0; (p = s.find_first_of("aeiou",p)) != s.npos; p++)
        s[p] = '*';
for (const string &s : ducks)
    cout << s << '\n';

H**y (r*d)
D*w*y (bl**)
L**** (gr**n)

A different approach.

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.