There's a lot that can be improved in your code, especially if you are using C++. The more advanced features of C++ actually help you organize your code better, help you write less code to do what you want, and remove a lot of tedium that you find in C-style code.
Use proper names for variables and functions
Names should accurately reflect what the purpose is of a variable or a function. Of course, names should be concise, but apart from some well established conventions such as using i
for a loop index, or x
and y
for coordinates, don't abbreviate names.
Here are some suggestions for improvements in your code:
- Instead of
circle()
, name the function draw_circles()
.
- Instead of
square()
, name the function print_result()
.
- Instead of
comp
, name the variable angle
, or perhaps phi
.
Use '\n'
instead of std::endl
std::endl
is in fact equivalent to '\n'
, but in addition it also forces a flush of the output stream, which will slow down the output if it is used a lot.
Don't store unnecessary data
You have added a member variable circ
to struct vec2
. First of all, it is rather strange to name something vec2
when it contains 3 elements. But it is also unnecessary to store the radius of the circle in that struct at all.
Avoid using global variables
As already mentioned in the comments, don't use global variables if you can easily avoid them. Keep the array v
inside main()
, and pass it as a parameter to the other functions. It does not seem like a big deal now, but once your code grows you will appreciate keeping the global namespace tidy, and by passing references/pointers to your data to functions, you make those functions more reusable.
Stick to radians when possible
All trigonometric functions in <cmath>
use radians, and any calculations done involving angles are usually simpler when done in radians than in degrees. Instead of:
int comp = j*30;
double rad = comp * (pi / 180);
Write this:
double angle = j * (2 * pi / 12);
Use a proper value for π
If your <cmath>
library provides it, use M_PI
for the value of π, or if not, write double pi = std::acos(-1)
(see this question). Just don't make unnecessary approximations; it costs you nothing to do it properly, and you never know when an approximation might actually cause you problems.
Store the image, not the coordinates of the X'es.
Building a list of coordinates of where the X'es are, and then checking for each position in the output whether it matches the coordinates of one of the X'es is not efficient. This algorithm scales with O(width * height * number_of_Xes). You either want to write the X'es directly to the screen at the right position, or keep an array of width * height characters in memory, write the X'es to that array, and then at the end print the contents of that character array. Let's use the latter method, which brings us to:
Use a class to represent the output image
You are using C++, so make use of its features! Make a class that manages an ASCII-art image for you, like so:
#include <cassert>
#include <iostream>
#include <vector>
class Image {
std::vector<char> pixels;
public:
const unsigned int width;
const unsigned int height;
// Create a w * h image, fill it with spaces
Image(unsigned int w, unsigned int h):
width(w), height(h), pixels(w * h, ' ') {}
// Return a reference to the character at coordinates x, y
char &at(unsigned int x, unsigned int y) {
assert(x < width && y < height);
return pixels[y * width + x];
}
// Print the image
void print(std::ostream &out) {
for (unsigned int y = 0; y < height; y++) {
for (unsigned int x = 0; x < width; x++)
out << at(x, y);
out << '\n';
}
}
};
This class will store the characters that make up the image in a std::vector
, which acts mostly like an array, but it has the nice property that it also allocates and cleans up memory for you behind the scenes.
The function at()
will return a reference to the character in the vector at the given coordinates. You can use this to both read and write to that character.
The function print()
takes a std::ostream
reference as an argument, so you can have it print to any stream you want.
Using this class you can create an ASCII-art image, draw to it, and then print it like so:
int main() {
// Create a 10x10 character image
Image image(10, 10);
// Draw a diagonal line
for (int i = 0; i < 10; ++i)
image.at(i, i) = '\\';
// Print the results to the standard output
image.print(std::cout);
}
Now you can rewrite your circle()
function to write to a class Image
, like so:
void draw_circle(Image &image, double radius, int steps) {
static const double pi = std::acos(-1);
for (int i = 0; i < steps; ++i) {
double phi = i * (2 * pi / steps);
double x = image.width / 2 + radius * cos(phi);
double y = image.height / 2 + radius * sin(phi);
image.at(x, y) = 'X';
}
}
And then you can draw 3 circles with increasing radii like so:
int main() {
Image image(40, 40);
for (auto radius: {8, 12, 16})
draw_circle(image, radius, 12);
image.print(std::cout);
}
Of course, this will look like an ellipse on the screen, so you will have to find some way to correct the aspect ratio. You could create a draw_ellipse()
function which takes two parameters for the radius, one for the horizontal direction and another for the vertical.