2
\$\begingroup\$

I have following homework exercise from lecture "Variables, references and pointers":

Write the function set (...) so that the following code works correctly:

int x = 3;
set(x) = 3;
cout << x << endl;  // writes 0
set(x) = 4;
cout << x << endl;  // writes 3
set(x) = 5;
cout << x << endl;  // writes 4

This is my solution:

#include <iostream>

using namespace std;

int & set(int & num);

int main(void) {
    int x = 3;
    set(x) = 3;
    cout << x << endl;  // wypisze 0
    set(x) = 4;
    cout << x << endl;  // wypisze 3
    set(x) = 5;
    cout << x << endl;  // wypisze 4
    return 0;
}

int & set(int & num) {
    static int tmp;
    int & ret = tmp;
    num = ret;
    return ret;
}

What do you think about it?

\$\endgroup\$
4
  • \$\begingroup\$ Why do you use this static hack? Simply return the object you passed in. Be aware that this might result in UB. You could solve this issue with perfect forwarding... But that seems to much for this state of education. \$\endgroup\$
    – DNKpp
    Commented Dec 29, 2017 at 21:24
  • \$\begingroup\$ @CrisLuengo There is an assignment to tmp; it's just after set returns. \$\endgroup\$
    – Justin
    Commented Jan 1, 2018 at 22:49
  • \$\begingroup\$ @Justin My bad, I didn't parse correctly. :) \$\endgroup\$ Commented Jan 1, 2018 at 23:15
  • \$\begingroup\$ @CrisLuengo np. This code is actually quite hard to read and especially to understand. Unfortunately, the way the homework assignment is stated, there isn't a better way to do it \$\endgroup\$
    – Justin
    Commented Jan 1, 2018 at 23:16

2 Answers 2

1
\$\begingroup\$

You have too many variables. The ret is not needed.

Also, for testing I would like to see some actual testing, rather than just printing to the console and letting the user do it:

#include <cassert>

void test_set() {
    int x = 3;
    set(x) = 3;
    assert(x == 0);
    set(x) = 4;
    assert(x == 3);
    // etc.
}

(Or, you could use an actual unit testing framework. But assert works for this simple case.)

\$\endgroup\$
0
\$\begingroup\$

Your style is identical to your previous question. Good job at being consistent!


I mentioned it on the previous question, but it's important enough to mention again: avoid using namespace std;. See Why is “using namespace std” considered bad practice?


int main(void) {

Again, avoid this C-ism. Don't use void to indicate no arguments. Just write int main(). Also, you don't need to return 0; from main; it's done automatically.


int & set(int & num) {
    static int tmp;

static variables are initialized to 0 IIRC, but there's no harm in just adding the = 0 to make it clear what the initial value is. Also tmp is an odd name for a variable that is not temporary.


    int & ret = tmp;
    num = ret;
    return ret;

As mentioned by Austin Hastings, ret is completely unneeded; you could just write:

    num = tmp;
    return tmp;
\$\endgroup\$
1
  • \$\begingroup\$ @CrisLuengo The code does work. See OP's code vs my code. And you absolutely need to return something, otherwise it's impossible to have the syntax of set(x) = foo;. If it were set(x, foo), you wouldn't have to return anything \$\endgroup\$
    – Justin
    Commented Jan 1, 2018 at 22:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.