Like Tree3Likes

Please suggest a better algorithm..

This is a discussion on Please suggest a better algorithm.. within the C++ Programming forums, part of the General Programming Boards category; Originally Posted by manasij7479 Wouldn't Code: for(VI it1=st[temp.v1[1]].begin();it1!=st[temp.v1[1]].end();it1++) look more cumbersome if written as Code: for ( VI it1 = ...

  1. #16
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    Quote Originally Posted by manasij7479 View Post
    Wouldn't
    Code:
    for(VI it1=st[temp.v1[1]].begin();it1!=st[temp.v1[1]].end();it1++)
    look more cumbersome if written as
    Code:
    for ( VI it1 = st[ temp.v1[1]].begin() ; it1 != st[temp.v1[1]].end() ;  it1++)
    ?
    Absolutely not. It provides visual separation of tokens. It makes it much easier to see where the three parts of the for head begin and end.
    Although you're overdoing it a bit. I'd write it like this:
    Code:
    for (VI it1 = st[temp.v1[1]].begin(); it1 != st[temp.v1[1]].end(); it1++)
    Whitespace placement is a matter of taste, really, but I don't know anyone who thinks that removing as much as the language allows is a good idea.

    Of course, you get a more effective readability improvement by factoring out that horrible nested indexing. Since you're using <array> I'm going to assume your compiler supports auto.

    Code:
        for (c = 10; (c * c) < 1000; c++)
        {
            temp.v1 = itotr(c * c);
            auto& a1 = st[temp.v1[1]];
            for (VI it1 = a1.begin(); it1 != a1.end(); it1++)
            {
                temp.v2=*it1;
                auto& a2 = st[temp.v1[2]];
                for(VI it2 = a2.begin(); it2 != st[temp.v1[2]].end(); it2++)
                {
                    temp.v3=*it2;
                    if(temp.v2[2]==temp.v3[1])
                    {
                        mc++;
                        storage.push_back(temp);
                    }
                }
            }
        }
    That still leaves your horrid naming. What is up with this?
    Code:
    class Tr //triad
    Seriously? Is your need to save three keystrokes per use of this class so big that you can't just call the class Triad? It's members are called disp() (What's wrong with display()? Or print(), which is shorter, and closer to typical computer terminology?), v1, v2, and v3. What are v1, v2 and v3? For that matter, what is a triad?

    Code:
    itotr // int to triad row
    You're basically confusing comments with names. In the above snippet, the comment should be the name, and the actual comment should say something like, "Given a three-digit number, return an array containing the three digits."
    Then add an assertion to the function that validates that the number is actually in the expected range.

    Why is storage not called "results"? Every variable stores something. The special thing about this one is that it stores the actual results of your program.
    What is mc? Inspection of your program tells me that it is the result count. Call it resultCount. And then remove it, because you realize that it is always equal to results.size().

    Typedefs would also help. A typedef of array<int, 3> to Row or TriadRow would go a long way towards making the type vector<array<int, 3>>[10] palatable. Of course, giving st a name that indicates what it is (a map from first digit to possible rows, try "possibleRowsByFirstDigit" or "rowsStartingWith") would go even further.

    Basically, your code is extremely cryptic because you want it to be compact. That's not a good habit.
    Last edited by CornedBee; 06-14-2011 at 07:42 AM.
    kmdv and iMalc like this.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  2. #17
    Registered User manasij7479's Avatar
    Join Date
    Feb 2011
    Location
    Kolkata@India
    Posts
    2,500
    understand your first code snippet
    I didn't post the first snippet to be thoroughly read... The sentence above it says that it was to show that the code looks really clumsy.
    Stick to n-column rule EmacsWiki: Eighty Column Rule.
    Could this be implemented with less than 3 levels of indentations?...+ It shows far less than 80 cols in my ide...with font set to 18..
    Whitespace placement is a matter of taste
    In my case.. a habit ..after using a netbook for a year.. Maybe I'll grow out of it when permanently using a larger screen..
    Last edited by manasij7479; 06-14-2011 at 07:28 AM.
    Manasij Mukherjee | gcc-4.8.2 @Arch Linux
    Slow and Steady wins the race... if and only if :
    1.None of the other participants are fast and steady.
    2.The fast and unsteady suddenly falls asleep while running !



  3. #18
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    I consider netbooks to be unusable for programming. But I guess you can't always choose your tools.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  4. #19
    Registered User manasij7479's Avatar
    Join Date
    Feb 2011
    Location
    Kolkata@India
    Posts
    2,500
    unusable for programming
    They are usable....at least when programming is a hobby....as in my case.....
    I really underestimated it first....It changed after I tried to run a few older games in it..
    + with Fedora ... it shows no sign of being slow.
    I've got eclipse, Kdesvn ,10 Firefox tabs and a pdf reader open ..and everything is snappy..

    The only problem seems to be the input and output devices

    btw..this thread is going totally off topic ..!
    Manasij Mukherjee | gcc-4.8.2 @Arch Linux
    Slow and Steady wins the race... if and only if :
    1.None of the other participants are fast and steady.
    2.The fast and unsteady suddenly falls asleep while running !



  5. #20
    Registered User
    Join Date
    Aug 2010
    Location
    Poland
    Posts
    682
    Programming on netbook?! Lol.
    They are usable....at least when programming is a hobby....as in my case.....
    As you can see, even on such a low level, they are ususable. You aren't even able to read 80-column lines.
    I never put signature, but I decided to make an exception.

  6. #21
    Registered User manasij7479's Avatar
    Join Date
    Feb 2011
    Location
    Kolkata@India
    Posts
    2,500
    You're basically confusing comments with names. In the above snippet, the comment should be the name, and the actual comment should say something like, "Given a three-digit number, return an array containing the three digits."
    Then add an assertion to the function that validates that the number is actually in the expected range.

    Why is storage not called "results"? Every variable stores something. The special thing about this one is that it stores the actual results of your program.
    What is mc? Inspection of your program tells me that it is the result count. Call it resultCount. And then remove it, because you realize that it is always equal to results.size().

    Typedefs would also help. A typedef of array<int, 3> to Row or TriadRow would go a long way towards making the type vector<array<int, 3>>[10] palatable. Of course, giving st a name that indicates what it is (a map from first digit to possible rows, try "possibleRowsByFirstDigit" or "rowsStartingWith") would go even further.

    Basically, your code is extremely cryptic because you want it to be compact. That's not a good habit.
    Thanks, I was really adhering to some bad principles.

    Could you elaborate on the "factoring out the indent" part ?
    according to wikipedia... auto seems to declare 'callable' types ...Wth is that ?
    Last edited by CornedBee; 06-14-2011 at 09:42 AM.
    Manasij Mukherjee | gcc-4.8.2 @Arch Linux
    Slow and Steady wins the race... if and only if :
    1.None of the other participants are fast and steady.
    2.The fast and unsteady suddenly falls asleep while running !



  7. #22
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    Basically auto means that the compiler determines the type of the variable based on the initializer. It's a new feature in C++11 and thus only supported by very new compilers.

    Without auto, it would look like this:
    Code:
    array<int, 3>& a1 = st[temp.v1[1]];
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  8. #23
    Registered User manasij7479's Avatar
    Join Date
    Feb 2011
    Location
    Kolkata@India
    Posts
    2,500
    Correct me if I'm wrong:
    Does
    Code:
    for (VI it1 = st[temp.v1[1]].begin(); it1 != st[temp.v1[1]].end(); it1++)
    become
    Code:
    for (auto it1 : st[temp.v1[1]] )
    ?
    Manasij Mukherjee | gcc-4.8.2 @Arch Linux
    Slow and Steady wins the race... if and only if :
    1.None of the other participants are fast and steady.
    2.The fast and unsteady suddenly falls asleep while running !



  9. #24
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,308
    Correctly applying spaces means that to most of us it should look like this:
    Code:
    for (VI it1 = st[temp.v1[1]].begin(); it1 != st[temp.v1[1]].end(); ++it1)
    You don't just put them in random places. Removing whitespace prior to review is ridiculously back-to-front.

    Edit: Hmm, I should have noticed the second page before posting.
    Last edited by iMalc; 06-14-2011 at 02:18 PM.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  10. #25
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    Quote Originally Posted by manasij7479 View Post
    Correct me if I'm wrong:
    Does
    Code:
    for (VI it1 = st[temp.v1[1]].begin(); it1 != st[temp.v1[1]].end(); it1++)
    become
    Code:
    for (auto it1 : st[temp.v1[1]] )
    ?
    Not quite. First, of course, your compiler has to support the for-range loop, and MSVC, at least, doesn't yet. (GCC and Clang do.)
    Second, the declaration in the for-range loop is for the element type, not the iterator type, so the name it1 is no longer appropriate, and you should probably use a reference. The auto in your code expands to array<int, 3>, and you probably don't want to copy the array. So you would do
    Code:
    for (const auto& row : st[temp..v1[1]])
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  11. #26
    Registered User manasij7479's Avatar
    Join Date
    Feb 2011
    Location
    Kolkata@India
    Posts
    2,500
    I'm using gcc-4.6(and once tried the incomplete 4.7-2011wxyz), I'd say that I'm almost well rounded in the support dept!..
    Why would
    Code:
    for ( const auto row : st[temp.v1[1]] )
    (without the &)
    copy the array/row ?
    Is the fact that I'd be assigning the 'row' to temp.v2 inside the body of the loop relevant here ?
    Manasij Mukherjee | gcc-4.8.2 @Arch Linux
    Slow and Steady wins the race... if and only if :
    1.None of the other participants are fast and steady.
    2.The fast and unsteady suddenly falls asleep while running !



  12. #27
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    With const auto row, auto resolve to array<int, 3>, so the code is equivalent to
    Code:
    for (const array<int, 3> row : st[...])
    Since this row is not a reference, there will be a copy. That's just how C++ works.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

Page 2 of 2 FirstFirst 12
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Please Suggest....
    By thecrazycoerian in forum Networking/Device Communication
    Replies: 1
    Last Post: 01-10-2004, 04:07 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21