Вычислить случайные триплеты пар поплавка и средней площади образовавшихся треугольников


Я пытаюсь получить много практики для интервью, поэтому я стараюсь писать как можно больше кода, и получаете обратную связь от программистов, как "вы". Я могу сделать некоторые общие обратную связь на этот небольшой кусочек кода? Любые рекомендации по стилю, хорошей практики и т. д. очень ценится.

Обычно я бы поставил класс объявления в собственные заголовочные файлы и определения .cpp файл, писать файл и сборки с помощью make. Тем не менее, это небольшая программа, так что я просто кладу все в один модуль.

/********************************************************************
 * Define a data type for triangles in the unit square, including a
 * function that computes the area of a triangle. Then write a client
 * program that generates random triples of pairs of floats between 0
 * and 1 and computes the average area of the triangles generated.
 * *****************************************************************/
#include <iostream>
#include <math.h>
#include <time.h>
#include <stdlib.h>

using namespace std;

class Point {
    public:
    Point( ) { ;}
    Point( float x, float y ) : x( x ), y( y ){ ;}
    float getX( ) { return x; }
    float getY( ) { return y; }
    void setX( float X ) { x = X; }
    void setY( float Y ) { y = Y; }
    void print( ) { cout << "("<<x<<", "<<y<<") "<<endl; }
    private:
    float x;
    float y;
};

class Line {
    public:
    Line( Point a, Point b ) : a( a ), b( b ){ ;}
    void mid_point( Point &mid );
    float length( );
    Point getA( ) { return a; }
    Point getB( ) { return b; }
    void print( ) { a.print( ); b.print( ); }
    private:
    Point a;
    Point b;
};

void Line::mid_point( Point &mid ) {
    mid.setX( (a.getX( )+b.getX( ))/2.0 );
    mid.setY( (a.getY( )+b.getY( ))/2.0 );
}

float Line::length( ) {
    return sqrt( pow( a.getX( )-b.getX( ),2.0 ) +
                 pow( a.getY( )-b.getY( ),2.0 )  
               );
}

class Triangle {
    public:
    Triangle( Line A,Line B,Line C ) : A( A ), B( B ), C( C ){ ;}
    float calc_area( );
    void print( ) { 
        cout << "Line A formed from points\n"; A.print( ); 
        cout << "Line B formed from points\n"; B.print( ); 
        cout << "Line C formed from points\n"; C.print( );
    }
    private:
    Line A;
    Line B;
    Line C;
};

float Triangle::calc_area( ) {
    Point mid;
    C.mid_point( mid );
    Line height( mid, A.getB( ) );
    return 0.5*C.length( )*height.length( );
}

int main( int argc, char *argv[ ] ) {
    srand( time( NULL ) );
    //Calculate area of 10 triangles;
    for( int i=0;i<10;i++ ) {
        Point p1( (rand( )%10)/10.0,(rand( )%10)/10.0 );
        Point p2( (rand( )%10)/10.0,(rand( )%10)/10.0 );
        Point p3( (rand( )%10)/10.0,(rand( )%10)/10.0 );
        Line l1( p1,p2 );
        Line l2( p2,p3 );
        Line l3( p3,p1 );
        Triangle t( l1,l2,l3 );
        t.print( );
        cout << "Area = " << t.calc_area( ) << endl << endl;
    }
}

Выход: (в случае, если вы не хотите, чтобы скопировать и вставить код)

[mehoggan@desktop triangle]$ ./tri_area 
Line A formed from points
(0.4, 0.2) 
(0.6, 0.4) 
Line B formed from points
(0.6, 0.4) 
(0.8, 0.6) 
Line C formed from points
(0.8, 0.6) 
(0.4, 0.2) 
Area = 0

Line A formed from points
(0.5, 0.2) 
(0.6, 0.3) 
Line B formed from points
(0.6, 0.3) 
(0.5, 0.9) 
Line C formed from points
(0.5, 0.9) 
(0.5, 0.2) 
Area = 0.0942404

Line A formed from points
(0.5, 0.9) 
(0.5, 0.8) 
Line B formed from points
(0.5, 0.8) 
(0.1, 0.2) 
Line C formed from points
(0.1, 0.2) 
(0.5, 0.9) 
Area = 0.129059

Line A formed from points
(0.3, 0.1) 
(0.7, 0.2) 
Line B formed from points
(0.7, 0.2) 
(0.5, 0.4) 
Line C formed from points
(0.5, 0.4) 
(0.3, 0.1) 
Area = 0.0548293

Line A formed from points
(0.8, 0.1) 
(0.2, 0.5) 
Line B formed from points
(0.2, 0.5) 
(0.8, 0.3) 
Line C formed from points
(0.8, 0.3) 
(0.8, 0.1) 
Area = 0.067082

Line A formed from points
(0.5, 0.8) 
(0.2, 0.2) 
Line B formed from points
(0.2, 0.2) 
(0.8, 0.3) 
Line C formed from points
(0.8, 0.3) 
(0.5, 0.8) 
Area = 0.166208

Line A formed from points
(0.6, 0.2) 
(0.8, 0.5) 
Line B formed from points
(0.8, 0.5) 
(0.4, 0.2) 
Line C formed from points
(0.4, 0.2) 
(0.6, 0.2) 
Area = 0.0424264

Line A formed from points
(0.1, 0.3) 
(0.1, 0) 
Line B formed from points
(0.1, 0) 
(0.4, 0.9) 
Line C formed from points
(0.4, 0.9) 
(0.1, 0.3) 
Area = 0.20744

Line A formed from points
(0.2, 0.4) 
(0.6, 0.9) 
Line B formed from points
(0.6, 0.9) 
(0.3, 0.9) 
Line C formed from points
(0.3, 0.9) 
(0.2, 0.4) 
Area = 0.109659

Line A formed from points
(0.1, 0.4) 
(0.1, 0.3) 
Line B formed from points
(0.1, 0.3) 
(0.8, 0.5) 
Line C formed from points
(0.8, 0.5) 
(0.1, 0.4) 
Area = 0.134629


662
6
задан 31 июля 2011 в 08:07 Источник Поделиться
Комментарии
4 ответа

с помощью пространства имен std; - это почти всегда плохая идея. Даже в небольшие программы, я хотел бы избежать его.

Ваш алгоритм calc_area это просто неправильно. Площадь треугольника равна половине длины стороны, умноженному на вертикальное расстояние от третьей вершины до базовой линии, а не на расстоянии от вершины к середине базы. (Представьте себе треугольник с вершинами в (-1,0), (1,0) и (1, 1). Площадь этого треугольника должна быть 1, а не функция sqrt(2).)

Рассматривать вашу точку класса, конструктор по умолчанию не инициализируются переменные-члены. Это может быть приемлемо для производительности причин - например - если вы сознательно хотите иметь возможность создавать большие массивы неинициализированных очков, но это часто безопаснее явно инициализировать все члены класса в конструкторе.

Имеющие как геттеры и сеттеры для Х и г эффективно делает их общие члены данных. Единственная функция, что точка имеет печати , но это может быть представлен в виде функции, не являющейся членом. После того как вы сделали этот класс обеспечивает функциональность, что простая структура не. Кроме того, вы можете использовать агрегатную инициализацию структуры , которые могут быть полезны.

Е. Г.

struct Point
{
float x;
float y;
};

// print functionality
std::ostream& operator( std::ostream& os, const Point& point )
{
os << "(" << point.x << ", " << point.y << ") " << std::endl;
}

Это намного проще, хотя вам придется изменить настройки.

// Point p( p1, p2 );
Point p = { p1, p2 };

Одним недостатком является то, что вы не можете легко создать временную точку с явными начальными значениями. Если вам нужно сделать это, вы могли бы рассмотреть вспомогательную функцию, аналогичную функции std::например, make_pair.

Е. Г.

inline Point MakePoint( int px, int py )
{
Point p = { px, py };
return p;
}

int main()
{
// FunctionTakingPoint( Point( 1, 2 ) );
FunctionTakingPoint( MakePoint( 1, 2 ) );
}

Будучи тривиальным стручок-структуры, большинство компиляторов будет иметь никаких трудностей в том, устраняя большинство подразумевает копий.

Есть какая-то аргументация, что линия заслуживает того, чтобы быть классом, как у вас нет наладчиков для своих членов, но, учитывая, что она маленькая поведение и поведение его может быть обеспечен свободной функции, я хотел бы сохранить его как стручок структуры. Клиенты могут выбрать, чтобы сделать константный экземпляр, если они так выбирают.

Кроме того, я не вижу никакой необходимости, чтобы сделать mid_point взять ссылку на структуру. Она может возвращать значение для более читабельный код.

struct Line
{
Point a;
Point b;
};

Point mid_point( const Line& line )
{
Point p = { ( line.a.x + line.b.x ) / 2.0,
( line.a.y + line.b.y ) / 2.0 };
return p;
}

double length( const Line& line )
{
double xdiff = line.b.x - line.a.x;
double ydiff = line.b.y - line.a.y;
return sqrt( xdiff * xdiff + ydiff * ydiff );
}

std::ostream& operator<<( std::ostream& os, const Line& line )
{
os << line.a << line.b;
}

8
ответ дан 31 июля 2011 в 02:07 Источник Поделиться

Пересмотрен и отредактирован под Windows 7, компилируется в ВС10 с пакетом обновления 1. Взять комментариями и изменениями с недоверием и использовать то, что вы считаете хорошим переменам :)

/********************************************************************
* Define a data type for triangles in the unit square, including a
* function that computes the area of a triangle. Then write a client
* program that generates random triples of pairs of floats between 0
* and 1 and computes the average area of the triangles generated.
* *****************************************************************/
// .h headers are deprecated, use the headers with the "c" prefix like so.
#include <iostream>
#include <cmath>
#include <ctime>
#include <ostream>

// No using namespace std. Since this is a .cpp this is acceptable, but
// here is a rule of thumb I found useful that lets me avoid namespace std
// even in .cpp files:
// - Write "using std::xxx" locally if you are using xxx more than three times.
// - Write "using std::xxx" globally if many functions are using xxx.
// - Write "using namespace std" locally if you are using 3 or more std:: functions more than three times.
// - Write "using namespace std" globally if you are lazy :D

// Reviewing your class design, I would like to stress some things:
// You are using the space R^2 (two dimensions, real numbers).
// I do not see this constraint in your problem definition above. Consider how you would expand this class
// to multiple dimensions as well as the realm of complex numbers (for which the length and other functions is slightly
// different).

// You did not define any operator overloads for things such as addition, subtraction, etc.
// Consider if it's worth implementing these functions, especially with vectors.

class Point {
public:
// You could default-initialize the second ctor and remove the need for the first one.
Point( float x = 0, float y = 0 )
: x_( x ), y_( y )
{ } // You do not need semicolons inside an empty function body.

// C++ usually doesn't prefix getters with "get". Again, depends on the style guide and personal preference.
float x( ) const
{ return x_; } // Getters do not mutate your members, make it const!

float y( ) const
{ return y_; }

void setX( float x ) { x_ = x; } // Parameters are usually lower-case in C++.
void setY( float y ) { y_ = y; }

// Personal preference, but give me a space between all those symbols! Otherwise it's hard to read.
void print( ) const
{ }

private:
float x_; // Members are usually of the form m_X, m_x, or x_ to distinguish them from local or parameter variables.
float y_;
};

std::ostream& operator<<(std::ostream& os, Point const& point)
{
os << "(" << point.x() << ", " << point.y() << ") " << std::endl;
return os;
}

class Line {
public:
// Consider a default constructor.
// We would then have, essentially, a line of length zero, or a point.
Line( Point a = Point(), Point b = Point() )
: a_( a ), b_( b )
{ }

// Why is this function outside the class body whereas all others are inside?
// You are mutating mid, something a user of your library might not be aware of.
// I recommend giving this function a return value of the midpoint instead.
Point midpoint( ) const
{
return Point ((a_.x( ) + b_.x( )) / 2.0,
(a_.y( ) + b_.y( )) / 2.0);
}

// Again, why have this function outside the class and not the others?
float length( ) const
{
// Length of a line is of the form:
// sqrt( (P2_x - P1_x)^2 + (P2_y - P1_y)^2 + ... + (P2_n - P1_n)^2 )
// where n is the number of coordinates of the point.
// I mention this because you are using only two dimensions, but many more are possible and used.

// This will not work, think about arithmetic operators for your Point class.
// return sqrt((a - b) * (a - b));

// (P2_x - P1_x)^2 = (P2_x - P1_x) * (P2_x - P1_x)
// Do not use pow in this case. We are using integers and pow() uses doubles or floats.

// sqrt uses doubles or floats. Depending on the accuracy, you want to change the cast (and the program) to a double.
return sqrt (static_cast<float>((b_.x() - a_.x()) * (b_.x() - a_.x()) +
(b_.y() - a_.y()) * (b_.y() - a_.y())));
}

Point const& a( ) const
{ return a_; }

Point const& b( ) const
{ return b_; }

private:
Point a_;
Point b_;
};

std::ostream& operator<<(std::ostream& os, Line const& line)
{
os << line.a() << line.b();
return os;
}

class Triangle {
public:
// Triangle of size zero? :D
Triangle( Line a = Line(), Line b = Line(), Line c = Line() )
: a_( a ), b_( b ), c_( c )
{ }

// Since the area of a triangle is just called "area" in mathematics,
// why not call the function the same?
float area( ) {
// We are not modifying mid or height past the initialization.
// Might as well const it.
// I also changed the ctor call for height, this prevents the "most vexing parse" problem
// as well as me being able to align the "=" :3
const Point mid = c_.midpoint();
const Line height = Line( mid, a_.b( ) );

return static_cast<float>(0.5 * c_.length( ) * height.length( ));
}

// You used accessors for Line and Point, but why not for Triangle?
Line const& a() const { return a_; }
Line const& b() const { return b_; }
Line const& c() const { return c_; }

private:
Line a_;
Line b_;
Line c_;
};

// Non-member non-friend for greatest encapsulation power :3
// Using an operator<< allows us to not just write to console, but moar! D:
std::ostream& operator<<(std::ostream& os, Triangle const& triangle)
{
os << "Line A formed from points\n" << triangle.a()
<< "Line B formed from points\n" << triangle.b()
<< "Line C formed from points\n" << triangle.c();

return os;
}

int main( int argc, char *argv[ ] ) {
srand( time( NULL ) );

// Calculate area of 10 triangles;
// For non-pod types (e.g. iterators), it is better to use post-increment (++i).
// I like to use it even on POD types just so I'm in the habit of using post-increment.
for( int i = 0; i < 10; ++i ) {
// Remember that this is a non-uniform distribution that gets worse as you
// increase the range of the rand (unless you remove the modulo division, of course).
// With such small numbers and for the intents of your program, this is negligible.
Point p1( (rand( ) % 10) / 10.0,(rand( ) % 10) / 10.0 );
Point p2( (rand( ) % 10) / 10.0,(rand( ) % 10) / 10.0 );
Point p3( (rand( ) % 10) / 10.0,(rand( ) % 10) / 10.0 );

Line l1( p1,p2 );
Line l2( p2,p3 );
Line l3( p3,p1 );

Triangle t( l1,l2,l3 );
std::cout << t;

std::cout << "Area = " << t.area( ) << "\n\n";
}

return 0;
}

5
ответ дан 31 июля 2011 в 12:07 Источник Поделиться

Пару моментов:

#include <math.h>
#include <time.h>
#include <stdlib.h>

Есть c++ версий этих заголовков:

#include <cmath>
#include <ctime>
#include <cstdlib>

Я знаю, что каждый руководство для начинающих в мире говорит:

using namespace std;

Не делай этого. Если вы хотите импортировать что-то специфическая польза:

using std::cin;
using std::cout; // or whatever you to import.

Если вы собираетесь сделать это, то попробуйте и области , используя в ближайшей области, которая имеет смысл. Но нет никаких реальных оснований даже для этого. Просто префикс вещи с СТД::.

std::cout << "Print a line\n";

Не нарушаем инкапсуляцию с GET и set

float getX( ) { return x; }
float getY( ) { return y; }
void setX( float X ) { x = X; }
void setY( float Y ) { y = Y; }

Это просто плохая идея. Я знаю, что это выглядит, как вы защищаете инкапсуляции, но вы не. Что вы делаете на самом деле является обязательным для реализации в ожидание того, что он должен обеспечить. Что вы должны делать-это предоставляет методы, которые используют внутренние элементы (то, что заключение о).

Действительно треугольника 3 линии?

Triangle( Line A,Line B,Line C ) : A( A ), B( B ), C( C ){ ;}

Я думаю, что треугольник действительно лучше представить как 3 очка. Что если A и B параллельны!
Это также делает несколько других методов легче писать. Кажется, вы сможете получить расстояние между двумя точками путем вычитания их или дистанционным методом (таким образом, вы не должны подвергать их внутреннего представления с getX() и gety()).

Это ОК, чтобы написать метод печати. Но вы должны proably также обеспечить выход оператора потока.

void print( ) { cout << "("<<x<<", "<<y<<") "<<endl; }

Я хотел бы сделать это:

class Point
{
public:
void print(std::ostream& str) const // Notice the cost
{
str << "(" << x << ", " << y << ") " << std::endl;
}
};
std::ostream& operator(std::ostream& str, Point const& data)
{
data.print(str);
return str;
}

ПС. Это не правильно:

float Triangle::calc_area( ) {
Point mid;
C.mid_point( mid );
Line height( mid, A.getB( ) );
return 0.5*C.length( )*height.length( );
}

Линия высота должна быть перпендикулярна к линии.

2
ответ дан 31 июля 2011 в 10:07 Источник Поделиться

Я не специалист по современному C++, но это намек, Я бы включил:

Вместо того, метод печати, который всегда выводит в cout, я также будет включать в себя метод, где он получает поток для печати.
Что-то вроде этого для пункт (не проверял):

void print(std::ostream &s){ s << "("<<x<<", "<<y<<") "<<endl;}

и для строка:

void print(std::ostream &s){ a.print(s); b.print(s);}

Таким образом, вы можете печатать в cout, cerr и файл, если вы хотите :)

Надеюсь, что это помогает

1
ответ дан 31 июля 2011 в 08:07 Источник Поделиться