Czy mógłby mi ktoś sprawdzić zadanie? C+++

[RSon]

Witam,

mógłby mi ktoś sprawdzić kod? nie mam pojęcia czemu nie działa

 

TREŚĆ:

http://pl.spoj.com/problems/PP0506A/

 

KOD:

https://wklej.to/VsAZu

[michal1972]

Po pierwsze w funkcji wprowadzanie(); najpierw tworzysz tablice i alokujesz jej nieznaną ilość pamieci (k), a dopiero potem prosisz użytkownika o podanie ile ma pamięci (miejsca na punkty) zaalokować.

[VereX.]

1. Korzystaj z vectorów

Czemu main nie ma typu?

Od razu oblicz ich długość łączący punkt (x,y) i (0,0). I sortuj od razu po nazwie punktu, a w pomocniczej tablicy możesz trzymać współrzędne punktu.

Przemyślałbym ilość funkcji, które spokojnie mogłyby być w mainie.

c=a*a+b*b; - sprawdź jeszcze raz wzór na odległość między punktami w układzie kartezjańskim.

[Sparq]

...
obliczanie(tab1,tab2,k);
	
return *tab1;
return *tab2;
	
delete [] tab1;
delete [] tab2;
...

!?!?!?!?!?!???????? To jest źle... bardzo! 

Ogólnie cały flow programu to przykład jak sie nie powinno tego robić.

 

 

 

EDIT: Na szybko coś napisałem powinno być dobrze.

 

 

#include <iostream>
#include <string>
#include <cmath>

using std::cout;
using std::cin;
using std::string;
// using namespace std is bad practice

struct point_t
{
	string name;
	int x;
	int y;
	float dist;
};

float CalcDist(int x, int y)
{
	return sqrt(x*x + y*y);
}

void Sort(point_t* tab, int size)
{
	for (int i = 0; i < size; i++)
	{
		for (int j = 0; j < size - 1; j++)
		{
			if (tab[j].dist > tab[j + 1].dist)
				std::swap(tab[j], tab[j + 1]);
		}
	}
}

int main()
{
	int iTest = 0;
	int iPoints[2];

	cin >> iTest;
	if (iTest > 99)
		return false;

	point_t** point = new point_t*[iTest];
	for (size_t i = 0; i < iTest; i++)
	{
		cin >> iPoints[i];
		if (iPoints[i] < 1 || iPoints[i] > 1000)
			return false;

		point[i] = new point_t[iPoints[i]];
		for (size_t j = 0; j < iPoints[i]; j++)
		{
			cin >> point[i][j].name >> point[i][j].x >> point[i][j].y;
			point[i][j].dist = CalcDist(point[i][j].x, point[i][j].y);
		}

		Sort(point[i], iPoints[i]);
	}

	for (size_t i = 0; i < iTest; i++)
	{
		for (size_t j = 0; j < iPoints[i]; j++)
		{
			cout << point[i][j].name << " " << point[i][j].x << " " << point[i][j].y << "\n";
		}

		delete[] point[i];	// important
	}
	delete[] point;	// important memory leaks etc.

	system("pause");
	return true;
}

 

 

[RSon]

Dziękuje wszystkim za odpowiedzi. Jestem początkującym programistą. Krytyka z takimi argumentami jak powyżej jest bardzo motywująca. Dziękuje jeszcze raz

[RSon]

https://wklej.to/BbqEK

 

poprawiłem program. Jedyne co, to wyświetla mi miejsca w ramie zamiast wartości. Ale to już kwestia wyświetlania. Jak oceniacie ten kod?

[mirraseq]

To nie jest kwestia wyświetlania, kod po prostu nie działa.

 

Odpowiednikiem C++ dla math.h jest cmath

#include <iostream>
#include <math.h>
#include <cstdlib>

 

Skorzystaj z rady Sparq a propos tego:

using namespace std;

 

Fajnie gdyby nazwa zmiennej odzwierciedlała cel jej istnienia

int main()
{
    int k;

Druga instrukcja może zostać przeoczona, unika się dwóch instrukcji w jednej linii

    cout<<"Podaj ilosc wprowadzanych punktow: "; cin>>k;

std::vector<int> tab_x; jest lepszym wyborem od tablicy dynamicznej, dzięki RAII nie martwisz się o zwalnianie pamięci (#include <vector>)

    int *tab_x=new int [k];
    int *tab_y=new int [k];
    float *tab_pom=new float [k];

 

 

Gdybyś użył wektora, wyświetlanie mógłbyś zrobić następująco:

    for (const auto& i: tab_pom)
    {
        cout << i << endl;
    }

 

 

I nie martwiłbyś się tym, bo dzięki RAII, wektory same się sprzątają.

    delete [] tab_x;
    delete [] tab_y;
    delete [] tab_pom;

 

 

Deklarowanie wielu zmiennych w jednej linijce jest nieestetyczne i może powodować pomyłki podczas deklarowania wskaźników czy referencji.

void obliczanie(int *x,int *y,int ile,float *tab)
{
    int j1=0,j2=0,j3=0,pom=0;

 

 

Deklarujesz tu tablice o rozmiarze 0! Pisanie po takiej tablicy powoduje tzw. "undefined behaviour". Poza tym VLA (Variable Length Array) są niezgodne ze standardem C++.

    float pomocnicza1[j1],pomocnicza2[j2],pomocnicza3[j3],a,b,c;