double return vs single return

[ischuldt]

I wrote this distructor one way, but the person who code reviewed it seems to think it's better and more readable the other way. I disagree. It's probably more a matter of preference, but what do you think...

RfPrinter::~RfPrinter()
{
    if (!hub_.empty() && !name_.empty()) {
        clearbuf(rfunitlock);
        rul_hub = hub_;
        rul_id = name_;
        try {
            read(rfunitlock);
            clearbuf(rfunitlog);
            rlog_hub = hub_;
            rlog_id = name_;
            rlog_indate = rul_date;
            rlog_intime = rul_time;
            GETTIME();
            try {
                read(rfunitlog);
                rlog_outdate = date();
                rlog_outtime = curtime;
                write(rfunitlog);
            } catch (nsr&) {
                clearbuf(rfunitlog);
                rlog_hub = hub_;
                rlog_id = name_;
                rlog_indate = date();
                rlog_intime = curtime;
                rlog_user = user();
                rlog_outdate = rlog_indate;
                rlog_outtime = curtime;
                rlog_comm = "No IN-RECORD in Log.";
                insert(rfunitlog);
            }
            rul_status = "Available";
            rul_date = date();
            rul_time = curtime;
            write(rfunitlock);
        } catch (nsr&) {
        }
    }
}

or...

RfPrinter::~RfPrinter()
{
	if(hub_empty() || name_.empty())return;
    clearbuf(rfunitlock);
    rul_hub = hub_;
    rul_id = name_;
    try {
        read(rfunitlock);
    } catch (nsr&) {
      	return;
    }
    clearbuf(rfunitlog);
    rlog_hub = hub_;
    rlog_id = name_;
    rlog_indate = rul_date;
    rlog_intime = rul_time;
    GETTIME();
    try {     
        read(rfunitlog);
        rlog_outdate = date();
        rlog_outtime = curtime;
        write(rfunitlog);
     } catch (nsr&) {
        clearbuf(rfunitlog);
        rlog_hub = hub_;
        rlog_id = name_;
        rlog_indate = date();
        rlog_intime = curtime;
        rlog_user = user();
        rlog_outdate = rlog_indate;
        rlog_outtime = curtime;
        rlog_comm = "No IN-RECORD in Log.";
        insert(rfunitlog);
     }
     rul_status = "Available";
     rul_date = date();
     rul_time = curtime;
     write(rfunitlock);
}

mostly i'd like thoughts on the single return vs the triple return.

[Narue]

I think they're both hideous, but mostly because you didn't use code tags properly.

>mostly i'd like thoughts on the single return vs the triple return.
There are good arguments both ways. It's a preference thing unless you're bound by an employer's coding guidelines.

[Fbody]

I don't like it either way, because I can't read it. Code tags are used [CODE] ... your code here ... [/CODE]. Or, you can just click the CODE button in the posting toolbar.

I suggest you edit your post within the next 15-minutes to correct the tags.

[edit]
ACK!! Ninja Narue!! Run! :D

[ischuldt]

Sorry is that better?

[mattjbond]

SESE (single entry single exit) was quite a big thing at one point, more especially in C as it was felt that it made functions easier to maintain amongst other things.

In C++ where you have exceptions as another way in which a function can be exited it is difficult to enforce unless you prohibit the use of exceptions, so it becomes less of an issue.

I used to work in Aerospace simulation producing realtime simulation code in C. There it was a company standard to use SESE. As I moved to C++ I have found that I have dropped this style completely.

I recently read C++ coding standards - 101 Rules, Guidelines and Best Practices. It had this to say in its 'Don't sweat the small stuff" section (item 1) -

'Single entry, single exit (“SESE”). Historically, some coding standards have required that each function have exactly one exit, meaning one return statement. Such a requirement is obsolete in languages that support exceptions and destructors, where functions typically have numerous implicit exits.'

And goes on to suggest that it is better to produce entities that have a single defined purpose as such making them more understandable and maintainable, and then buildingn more complex behavioural classes and functions from these basic building blocks.

make of that what you will.

[Fbody]

Yes, that is better.

Without knowing the problem domain and the goals of the code, personally I would consider this the better version...

RfPrinter::~RfPrinter()
{
    if (!hub_.empty() && !name_.empty()) {
        clearbuf(rfunitlock);
        rul_hub = hub_;
        rul_id = name_;
        try {
            read(rfunitlock);
            clearbuf(rfunitlog);
            rlog_hub = hub_;
            rlog_id = name_;
            rlog_indate = rul_date;
            rlog_intime = rul_time;
            GETTIME();
            try {
                read(rfunitlog);
                rlog_outdate = date();
                rlog_outtime = curtime;
                write(rfunitlog);
            } catch (nsr&) {
                clearbuf(rfunitlog);
                rlog_hub = hub_;
                rlog_id = name_;
                rlog_indate = date();
                rlog_intime = curtime;
                rlog_user = user();
                rlog_outdate = rlog_indate;
                rlog_outtime = curtime;
                rlog_comm = "No IN-RECORD in Log.";
                insert(rfunitlog);
            }
            rul_status = "Available";
            rul_date = date();
            rul_time = curtime;
            write(rfunitlock);
        } catch (nsr&) {
        }
    }
}

1. It's formatted properly which makes it easier to read
2. If there is an exception/error it provides information about and/or attempts to correct the error, it doesn't just terminate seemingly for no reason.