HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Find Triangle Triplets from a list of given numbers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
tripletsnumberstrianglefindlistfromgiven

Problem

If a triplet of segments A, B and C are triangle triplets if and only
if

  • A + B > C



  • A + C > B



  • B + C > A



Is there a better implementation for this problem?

#include
#include

void findTriangleTriplets( std::vector& vec_input, std::vector>& triplets)
{
    int size =  vec_input.size();
    for(int i = 0; i  vec_input.at( k ) ) &&
                    ( ( vec_input.at( i ) + vec_input.at( k ) ) > vec_input.at( j ) ) &&
                    ( ( vec_input.at( j ) + vec_input.at( k ) ) > vec_input.at( i ) ) )
                {               
                    std::vector temp;
                    temp.emplace_back( vec_input.at( i ) );
                    temp.emplace_back( vec_input.at( j ) ); 
                    temp.emplace_back( vec_input.at( k ) );
                    triplets.emplace_back( temp );
                }
            }
        }
    }
} 

int main()
{
    std::vector>  triplets;
    std::vector vec_input { 1, 2, 3, 4, 5, 6 };
   
    findTriangleTriplets( vec_input,  triplets);
    for ( auto &x : triplets)
    {
        std::cout<<x.at(0)<<" "<<x.at(1)<<" "<<x.at(2)<<"\n";
    }         

    return 0;
}

Solution

First thing to comment on after copying and pasting into my IDE, the following line gives a warning:

int size = vec_input.size();




Implicit conversion loses integer precision: 'size_type' (aka 'unsigned long') to 'int'

Apparently, the size() method of Vector returns an unsigned long, and we should use this type for our size, rather than int.

unsigned long size = input.size();


Functions that return void but accept (and modify) an argument passed by reference are always a warning sign for me. Here, I don't think there's a particularly good reason to do so.

Here, I can't see any good reason to be doing so. We create and return a vector within the function, and only accept the input argument.

Another problem I see: the result we're giving the caller is a vector of vectors, and the inner vectors are supposed to always have exactly 3 values in it. Why don't we create a struct to represent the triplets?

struct Triplet {
    int a;
    int b;
    int c;
}


Now we can return a vector of these.

I would also abstract that complex conditional into its own function.

bool isTriplet(int a, int b, int c) {
    return ((a + b) > c) && ((a + c) > b) && ((b + c) > a);
}


Now, putting all that together, without changing the algorithm or any of the logic, the code looks like this:

#include
#include

struct Triplet {
    int a;
    int b;
    int c;
};

bool isTriplet(int a, int b, int c) {
    return ((a + b) > c) && ((a + c) > b) && ((b + c) > a);
}

std::vector findTriangleTriplets(std::vector input) {
    std::vector triplets;
    unsigned long size = input.size();

    for(int i=0;i triplets;
    std::vector vec_input { 1, 2, 3, 4, 5, 6 };

    triplets = findTriangleTriplets(vec_input);
    for ( auto &x : triplets)
    {
        std::cout<<x.a<<" "<<x.b<<" "<<x.c<<"\n";
    }

    return 0;
}

Code Snippets

int size = vec_input.size();
unsigned long size = input.size();
struct Triplet {
    int a;
    int b;
    int c;
}
bool isTriplet(int a, int b, int c) {
    return ((a + b) > c) && ((a + c) > b) && ((b + c) > a);
}
#include<vector>
#include<iostream>

struct Triplet {
    int a;
    int b;
    int c;
};

bool isTriplet(int a, int b, int c) {
    return ((a + b) > c) && ((a + c) > b) && ((b + c) > a);
}

std::vector<Triplet> findTriangleTriplets(std::vector<int> input) {
    std::vector<Triplet> triplets;
    unsigned long size = input.size();

    for(int i=0;i<size;++i) {
        for(int j=i+1;j<size;++j) {
            for(int k=0;k<size;++k) {
                int a = input.at(i);
                int b = input.at(j);
                int c = input.at(k);

                if(isTriplet(a, b, c)) {
                    Triplet t = Triplet{a,b,c};
                    triplets.emplace_back(t);
                }
            }
        }
    }

    return triplets;
}

int main()
{
    std::vector<Triplet> triplets;
    std::vector<int> vec_input { 1, 2, 3, 4, 5, 6 };

    triplets = findTriangleTriplets(vec_input);
    for ( auto &x : triplets)
    {
        std::cout<<x.a<<" "<<x.b<<" "<<x.c<<"\n";
    }

    return 0;
}

Context

StackExchange Code Review Q#85862, answer score: 5

Revisions (0)

No revisions yet.