Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C++ > Thoughts on function

Reply
Thread Tools

Thoughts on function

 
 
Ebenezer
Guest
Posts: n/a
 
      09-19-2011
On Sep 18, 4:34*pm, Ian Collins <(E-Mail Removed)> wrote:
> On 09/19/11 08:30 AM, Ebenezer wrote:
>
>
>
>
>
>
>
>
>
> > On Sep 15, 7:23 pm, Ebenezer<(E-Mail Removed)> *wrote:

>
> >> I started by asking for ideas on how to improve it.
> >> Perhaps someone would show what they believe to be
> >> an improvement. *I realized after thinking about
> >> your earlier comments that the member variable
> >> partialFlush could be a static local variable.

>
> >> bool
> >> SendBuffer::Flush ()
> >> {
> >> * *uint32_t const all = buf_.size();
> >> * *uint32_t numWritten = 0;
> >> * *static bool partialFlush = false;
> >> * *if (partialFlush) {
> >> * * *::neolib::segmented_array<unsigned char, chunk_size>::segment&
> >> segment =
> >> * * * * * * * * segmented_iterator<unsigned char,
> >> chunk_size>(buf_.begin()).segment();

>
> >> * * *numWritten = Write(sock_,&buf_[0], segment.size());
> >> * * *if (numWritten< *segment.size()) goto ending;
> >> * * *partialFlush = false;
> >> * *}

>
> >> * *{
> >> * * *int32_t const chunks = (all - numWritten) / chunk_size;
> >> * * *for (int32_t ii = 0; ii< *chunks; ++ii) {
> >> * * * *uint32_t bytes = Write(sock_,&buf_[numWritten], chunk_size);
> >> * * * *numWritten += bytes;
> >> * * * *if (bytes< *chunk_size) goto ending;
> >> * * *}
> >> * *}

>
> >> * *if (numWritten< *all) {
> >> * * *numWritten += Write(sock_,&buf_[numWritten], all - numWritten);
> >> * *}

>
> >> ending:
> >> * *buf_.erase(buf_.begin(), buf_.begin() + numWritten);
> >> * *if (numWritten == all) return true;

>
> >> * *partialFlush = true;
> >> * *return false;
> >> }

>
> > How would you rate this function -- with a 1 being terrible
> > and a 10 being excellent?

>
> You don't appear to have headed any of the suggestions.


I'm not opposed to the size_t suggestion, but am wanting to
focus on the bigger picture.

>
> At review, I'd still give it a 1 because it isn't clear what it does,
> why it uses fixed width types and it still contains gratuitous gotos.
>


All of the code is free -- gratuitous. Meanwhile no one has
provided an alternative they think is better.

 
Reply With Quote
 
 
 
 
Geoff
Guest
Posts: n/a
 
      09-19-2011
On Sun, 18 Sep 2011 18:25:40 -0700 (PDT), Ebenezer
<(E-Mail Removed)> wrote:

>All of the code is free -- gratuitous. Meanwhile no one has
>provided an alternative they think is better.


Gratuitous in this context means unnecessary. Not gratis. But you knew
that.

Refactor your function, get rid of the goto's. They are bad practice.

Don't worry about writing redundant lines in your functions, don't try
to optimize them away with goto's as you have done in your function,
let the compiler optimize them away, it's much better at it than you
are. What you should be writing for is clarity and maintainability.

 
Reply With Quote
 
 
 
 
Ebenezer
Guest
Posts: n/a
 
      09-19-2011
On Sep 18, 9:08*pm, Geoff <(E-Mail Removed)> wrote:
> On Sun, 18 Sep 2011 18:25:40 -0700 (PDT), Ebenezer
>
> <(E-Mail Removed)> wrote:
> >All of the code is free -- gratuitous. *Meanwhile no one has
> >provided an alternative they think is better.

>
> Gratuitous in this context means unnecessary. Not gratis. But you knew
> that.
>
> Refactor your function, get rid of the goto's. They are bad practice.
>
> Don't worry about writing redundant lines in your functions, don't try
> to optimize them away with goto's as you have done in your function,
> let the compiler optimize them away, it's much better at it than you
> are. What you should be writing for is clarity and maintainability.


Still no code.
 
Reply With Quote
 
Geoff
Guest
Posts: n/a
 
      09-19-2011
On Sun, 18 Sep 2011 19:46:06 -0700 (PDT), Ebenezer
<(E-Mail Removed)> wrote:

>On Sep 18, 9:08*pm, Geoff <(E-Mail Removed)> wrote:
>> On Sun, 18 Sep 2011 18:25:40 -0700 (PDT), Ebenezer
>>
>> <(E-Mail Removed)> wrote:
>> >All of the code is free -- gratuitous. *Meanwhile no one has
>> >provided an alternative they think is better.

>>
>> Gratuitous in this context means unnecessary. Not gratis. But you knew
>> that.
>>
>> Refactor your function, get rid of the goto's. They are bad practice.
>>
>> Don't worry about writing redundant lines in your functions, don't try
>> to optimize them away with goto's as you have done in your function,
>> let the compiler optimize them away, it's much better at it than you
>> are. What you should be writing for is clarity and maintainability.

>
>Still no code.


Ah! So it's not about what do you think of this function but it's
about fix this function if you can. We gave you our thoughts on this
function. We are not obligated to fix it for you.
 
Reply With Quote
 
Miles Bader
Guest
Posts: n/a
 
      09-19-2011
Geoff <(E-Mail Removed)> writes:
> Refactor your function, get rid of the goto's. They are bad practice.


But let's be fair -- while goto is often[*] used badly, and in such
cases makes code harder to understand/maintain, it isn't somehow the
kiss of death. There are reasonable uses of goto.

So although the presence of "goto" in code is a big warning sign
(especially in C++, where RIAA tends to supplant "goto for error
cleanup" idioms), is seems unreasonable to thus grade code a "1"
without actually looking at the details of how its used.
[*] maybe less so these days than in the past, as it's acquired such a
huge negative reputation that non-clueful programmers are likely to
avoid it entirely on that basis.

-Miles

--
Learning, n. The kind of ignorance distinguishing the studious.
 
Reply With Quote
 
Ebenezer
Guest
Posts: n/a
 
      09-19-2011
On Sep 18, 10:12*pm, Geoff <(E-Mail Removed)> wrote:
> On Sun, 18 Sep 2011 19:46:06 -0700 (PDT), Ebenezer
>
>
>
>
>
> <(E-Mail Removed)> wrote:
> >On Sep 18, 9:08 pm, Geoff <(E-Mail Removed)> wrote:
> >> On Sun, 18 Sep 2011 18:25:40 -0700 (PDT), Ebenezer

>
> >> <(E-Mail Removed)> wrote:
> >> >All of the code is free -- gratuitous. Meanwhile no one has
> >> >provided an alternative they think is better.

>
> >> Gratuitous in this context means unnecessary. Not gratis. But you knew
> >> that.

>
> >> Refactor your function, get rid of the goto's. They are bad practice.

>
> >> Don't worry about writing redundant lines in your functions, don't try
> >> to optimize them away with goto's as you have done in your function,
> >> let the compiler optimize them away, it's much better at it than you
> >> are. What you should be writing for is clarity and maintainability.

>
> >Still no code.

>
> Ah! So it's not about what do you think of this function but it's
> about fix this function if you can. We gave you our thoughts on this
> function. We are not obligated to fix it for you.


I have said numerous times I'm looking for an alternative
that is thought to be better.
 
Reply With Quote
 
Ian Collins
Guest
Posts: n/a
 
      09-19-2011
On 09/19/11 01:25 PM, Ebenezer wrote:
> On Sep 18, 4:34 pm, Ian Collins<(E-Mail Removed)> wrote:
>> On 09/19/11 08:30 AM, Ebenezer wrote:
>>

Why does google put in these huge gaps? I didn't include it.
>>
>>> On Sep 15, 7:23 pm, Ebenezer<(E-Mail Removed)> wrote:

>>
>>>> ending:
>>>> buf_.erase(buf_.begin(), buf_.begin() + numWritten);
>>>> if (numWritten == all) return true;

>>
>>>> partialFlush = true;
>>>> return false;
>>>> }

>>
>>> How would you rate this function -- with a 1 being terrible
>>> and a 10 being excellent?

>>
>> You don't appear to have headed any of the suggestions.

>
> I'm not opposed to the size_t suggestion, but am wanting to
> focus on the bigger picture.
>
>> At review, I'd still give it a 1 because it isn't clear what it does,
>> why it uses fixed width types and it still contains gratuitous gotos.

>
> All of the code is free -- gratuitous. Meanwhile no one has
> provided an alternative they think is better.


Everything at after your ending label could be in a bool returning
member function.

--
Ian Collins
 
Reply With Quote
 
Ian Collins
Guest
Posts: n/a
 
      09-19-2011
On 09/19/11 04:26 PM, Miles Bader wrote:
> Geoff<(E-Mail Removed)> writes:
>> Refactor your function, get rid of the goto's. They are bad practice.

>
> But let's be fair -- while goto is often[*] used badly, and in such
> cases makes code harder to understand/maintain, it isn't somehow the
> kiss of death. There are reasonable uses of goto.
>
> So although the presence of "goto" in code is a big warning sign
> (especially in C++, where RIAA tends to supplant "goto for error
> cleanup" idioms), is seems unreasonable to thus grade code a "1"
> without actually looking at the details of how its used.


Every coding standard I've written or had to follow has prohibited goto
except in exceptional (and well commented) cases.

--
Ian Collins
 
Reply With Quote
 
Miles Bader
Guest
Posts: n/a
 
      09-19-2011
Ian Collins <(E-Mail Removed)> writes:
>> But let's be fair -- while goto is often[*] used badly, and in such
>> cases makes code harder to understand/maintain, it isn't somehow the
>> kiss of death. There are reasonable uses of goto.
>>
>> So although the presence of "goto" in code is a big warning sign
>> (especially in C++, where RIAA tends to supplant "goto for error
>> cleanup" idioms), is seems unreasonable to thus grade code a "1"
>> without actually looking at the details of how its used.

>
> Every coding standard I've written or had to follow has prohibited
> goto except in exceptional (and well commented) cases.


That isn't inconsistent with what I said.

It isn't unreasonable for a coding standard, especially those targeted
at non-expert programmers, to outlaw goto with documented exceptions
like that -- the entire point of a coding standard, after all, is to
try and model subjective judgement with something simpler, more
consistent (within the organization), and easier to apply; this is
very valuable when many programmers of varying skill-levels are
working together.

But they aren't an end in themselves, and are not, in the end,
some sort of authoritative statement.

So if the OP was saying "goto gets your code grade 1 _in our
organization, because it violates our coding standards_", that would
be reasonable. But of course that isn't the scenario here.

[Remember, I'm _not_ defending the OP's use of goto, I'm simply saying
that goto isn't _universally_ bad, and a nuanced discussion of code
quality shouldn't use simplistic shortcuts like that, even if they're
justified in many more practical applications.]

-Miles

--
On a bad day, I see brewers still talking as though all beer were
consumed in the pub, by the pint -- by insatiably thirsty Yorkshire
steelworkers who have in reality long ago sought work as striptease
artists. [Michael Jackson]
 
Reply With Quote
 
hanukas
Guest
Posts: n/a
 
      09-19-2011
On Sep 15, 8:01*am, Ebenezer <(E-Mail Removed)> wrote:
> Shalom
>
> How to improve this function?
>
> bool
> SendBuffer::Flush ()
> {
> * uint32_t const all = buf_.size();
> * uint32_t numWritten = 0;
> * if (partialFlush_) {
> * * ::neolib::segmented_array<unsigned char, chunk_size>::segment&
> segment =
> * * * * * * * *segmented_iterator<unsigned char,
> chunk_size>(buf_.begin()).segment();
>
> * * numWritten = Write(sock_, &buf_[0], segment.size());
> * * if (numWritten < segment.size()) goto ending;
> * * partialFlush_ = false;
> * }


Someone suggested that "goto" isn't optimum solution, so you could
restructure the control flow:

-->

if (numWritten >= segment.size())
{
partialFlush_ = false;
}
}

if (!partialFlush)
{
...
}

//ending:

The first goto would leave the "partialFlush" into state "true", so
this control flow change would take care of that. The second one is a
break from the for-loop which is easy to reproduce with a signal
variable.

What's the problem you are having with this code? You feel that it
should be more satisfying to the craft or something more functional
than that?
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
thoughts on a more generic Array#partition function Rudi Cilibrasi Ruby 13 07-04-2008 03:31 PM
New Hash Function workshop thoughts ShadowEyez Computer Security 0 11-04-2005 04:59 AM
Please share your thoughts on Member Function Templates Rajan C++ 4 05-27-2005 04:09 PM
write a function such that when ever i call this function in some other function .it should give me tha data type and value of calling function parameter komal C++ 6 01-25-2005 11:13 AM
Passing a C++ object's member function to a C function expecing a function pointer! James Vanns C++ 7 01-21-2004 02:39 AM



Advertisments