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

Kinematic Equations Calculator

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

Problem

The program allows user to enter values for 3 of the following

  • initial velocity



  • final velocity



  • acceleration



  • displacement



  • time



and to specify an unknown (one of the previously mentioned variables). The program uses the Kinematic Equations to calculate the unknown.

I would like a review of the entire program. I have only studied programming online as a hobby so i have never received comments on my code. I want to become a better programmer and break any bad habits. Dont hold back on any criticism or praise, please comment anything you think of. What is inefficient/ unclear/ unnecessary? What can be improved or added? What is good? What approach would you have taken to the problem? Is the code understandable at all As far as I can tell the code at least works, does what it should.

OrderedArray.h, container class

```
#include "stdafx.h"
#include
#include

class OrderedArray
{
private:
double* m_pdValues;
int* m_pnOrderKeys;
int m_nValuesLength;
int m_nKeysLength;
void erase()
{
delete[] m_pdValues;
delete[] m_pnOrderKeys;
m_pdValues = 0;
m_pnOrderKeys = 0;
m_nValuesLength = 0;
m_nKeysLength = 0;
}
public:
OrderedArray() : m_pdValues(0), m_nValuesLength(0), m_pnOrderKeys(0), m_nKeysLength(0)
{

}
double operator[](int nIndex) //returns from values array
{
assert(nIndex >= 0 && nIndex = 0 && nIndex = 0 && nIndex = 0 && nIndex = 0 && nIndex = 3)
{
std::cout << "OrderedArray full" << std::endl;
return;
}
if(m_pdValues == 0)
{
m_pdValues = new double[1];
m_nValuesLength = 1;
m_pdValues[0] = dValue;
m_pnOrderKeys = new int[1];
m_nKeysLength = 1;
m_pnOrderKeys[0] = nKey;
}
else
{
int keysLengthTemp = m_nKeysLength;
for(int i = 0; i < keysLengthTemp; i++)
{
if(nKey < m_pnOrderKeys[i])
{
insertBefore(dValue, i);

Solution

This is pretty clever work for someone self taught! In my opinion, it's more work than is necessary, though. Your OrderedArray class looks like a cross between std::map and std::vector, but it also looks like it will never hold more than 4 elements, if I understand it correctly.

Simplify

I think you can do away with the OrderedArray class and the funcArray, and just use some enums and a switch statement. For example, something like this:

enum {
    kUnknown_Displacement,
    kUnknown_InitialVelocity,
    kUnknown_FinalVelocity,
    kUnknown_Acceleration,
    kUnknown_Time
};


Then, after collecting your knowns, just do something simple like:

switch(unknown) {
    case kUnknown_Displacement:
        calculateDisplacement(initialVelocity, Acceleration, time); // or whatever variables you need
    break;

    case kUnknown_InitialVelocity:
        // etc...
    break;
};


Naming

The other thing I think you could improve is your naming. Function names like vfvf, vfa, and ddd don't tell a reader of the code (including you 6 months from now) what these functions actually do. Renaming them as I did in my example above to things like calculateDisplacement(), calculateAcceleration(), etc. would go a long way towards helping this.

Types

Each of your functions returns 2 values, and so you're returning them as an array. It's not clear to me what the meaning of each value is, but if it's something like an x and y of a point, you should make a Point type and use that. If it's a magnitude and angle, then you should make a structure with those members and use that. Another alternative is to return a std::tuple, though when the values are clearly related (like for a Point), it's better to put it into a structure with named members.

Don't reinvent the wheel

You made your own constant PI. You should use M_PI because it's portable and will have enough digits for the precision you need.

Code Snippets

enum {
    kUnknown_Displacement,
    kUnknown_InitialVelocity,
    kUnknown_FinalVelocity,
    kUnknown_Acceleration,
    kUnknown_Time
};
switch(unknown) {
    case kUnknown_Displacement:
        calculateDisplacement(initialVelocity, Acceleration, time); // or whatever variables you need
    break;

    case kUnknown_InitialVelocity:
        // etc...
    break;
};

Context

StackExchange Code Review Q#106547, answer score: 8

Revisions (0)

No revisions yet.