3
\$\begingroup\$

Here is some of the code in my library. If there any inefficient things in my code, let me know. UPDATE: The following code is deprecated. All of your fixes have been incorporated. The project now lives at InitializeSahib/Inverse InverseAPI.cpp:

// InverseAPI (Cobalt)
// Developed by Sahibdeep Nann (@SahibdeepNann)
// https://github.com/InitializeSahib/InverseAPI
// Licensed under the MIT License
#include "stdafx.h"
#include "InverseCobalt.h"
namespace InverseCobalt {
    int InverseRandom::randomInteger(int minimumInteger, int maximumInteger) {
        std::mt19937 generateInteger(time(0));
        std::uniform_int_distribution<int> integerDistribution(minimumInteger, maximumInteger);
        return integerDistribution(generateInteger);
    }
    double InverseMath::inverseExponents(double baseNumber, double exponentNumber) {
        return pow(baseNumber, exponentNumber);
    }
    double InverseMath::inverseSquare(double numberToSquare) {
        return InverseMath::inverseExponents(numberToSquare, 2);
    }
    void InverseSystem::runSystemCommand(const char *commandToRun) {
        system(commandToRun);
    }
    time_t InverseSystem::getSystemTime() {
        return time(0);
    }
    int InverseRandom::randomDigit() {
        std::mt19937 generateDigit(time(0));
        std::uniform_int_distribution<int> integerDistribution(0, 9);
        return integerDistribution(generateDigit);
    }
    char InverseRandom::randomLetterUppercase() {
        std::mt19937 generateLetterInt(time(0));
        std::uniform_int_distribution<int> integerDistribution(1, 26);
        int randomLetterInt = integerDistribution(generateLetterInt);
        return (char)'A' - 1 + randomLetterInt;
    }
    char InverseRandom::randomLetterLowercase() {
        std::mt19937 generateLetterInt(time(0));
        std::uniform_int_distribution<int> integerDistribution(1, 26);
        int randomLetterInt = integerDistribution(generateLetterInt);
        return (char)'a' - 1 + randomLetterInt;
    }
    int InverseConversion::charDecimalValue(char characterToCheck) {
        return (int)characterToCheck;
    }
    char InverseConversion::decimalValueChar(int decimalValueToCheck) {
        return (char)decimalValueToCheck;
    }
    void InverseDebug::testAllFunctions() {
        std::cout << "This function will test all the other functions in InverseAPI. It will take a few moments.\n";
        std::cout << "Now testing: InverseRandom::randomInteger. If test succeeds, should output random integer between 1 and 100.\n";
        std::cout << InverseRandom::randomInteger(1, 100) << std::endl;
        std::cout << "Now testing: InverseMath::inverseExponents. If test succeeds, should output 1,000.\n";
        std::cout << InverseMath::inverseExponents(10, 3) << std::endl;
        std::cout << "Now testing: InverseSystem::runSystemCommand. If test succeeds, should open command-line window asking you to press a key.\n";
        InverseSystem::runSystemCommand("pause");
        std::cout << "Now testing: InverseSystem::getSystemTime. If test succeeds, should output current Unix Time.\n";
        std::cout << InverseSystem::getSystemTime() << std::endl;
        std::cout << "Now testing: InverseRandom::randomDigit, randomLetterUppercase, and randomLetterLowercase. If test succeeds, should output an integer between 0 and 9, then a random uppercase letter on the next line, and a random lowercase letter on the line after that.\n";
        std::cout << InverseRandom::randomDigit() << std::endl << InverseRandom::randomLetterUppercase() << std::endl << InverseRandom::randomLetterLowercase() << std::endl;
        std::cout << "Now testing: InverseMath::inverseSquare. If test succeeds, should output 81.\n";
        std::cout << InverseMath::inverseSquare(9) << std::endl;
        std::cout << "Now testing: InverseConversion::charDecimalValue and decimalValueChar. If test succeeds, should output 97, then \"a\" without quotes underneath it.\n";
        std::cout << InverseConversion::charDecimalValue('a') << std::endl << InverseConversion::decimalValueChar(97) << std::endl;
        std::cout << "All tests have finished.\n";
    }
    void InverseDebug::testCharFunctions() {
        std::cout << "This function will test all functions that return a char in InverseAPI. It will take a few moments.\n";
        std::cout << "Now testing: InverseConversion::decimalValueChar. If test succeeds, should output 97.\n";
        std::cout << InverseConversion::decimalValueChar('a') << std::endl;
        std::cout << "Now testing: InverseRandom::randomLetterUppercase. If test succeeds, should output a random uppercase letter.\n";
        std::cout << InverseRandom::randomLetterUppercase() << std::endl;
        std::cout << "Now testing: InverseRandom::randomLetterLowercase. If test succeeds, should output a random lowercase letter.\n";
        std::cout << InverseRandom::randomLetterLowercase() << std::endl;
        std::cout << "All tests finished." << std::endl;
    }
    int InverseLegacy::legacyRandomInt(int minimumInteger, int maximumInteger) {
        return rand() % ((maximumInteger - minimumInteger) + 1) + minimumInteger;
    }
    int InverseLegacy::legacyIntSquareRoot(int numberToSquareRoot) {
        return sqrt(numberToSquareRoot);
    }
    char InverseLegacy::legacyRandomLetterUppercase() {
        int randomLetterInt = InverseLegacy::legacyRandomInt(1, 26);
        if (randomLetterInt == 1) {
            return 'A';
        }
        if (randomLetterInt == 2) {
            return 'B';
        }
        if (randomLetterInt == 3) {
            return 'C';
        }
        if (randomLetterInt == 4) {
            return 'D';
        }
        if (randomLetterInt == 5) {
            return 'E';
        }
        if (randomLetterInt == 6) {
            return 'F';
        }
        if (randomLetterInt == 7) {
            return 'G';
        }
        if (randomLetterInt == 8) {
            return 'H';
        }
        if (randomLetterInt == 9) {
            return 'I';
        }
        if (randomLetterInt == 10) {
            return 'J';
        }
        if (randomLetterInt == 11) {
            return 'K';
        }
        if (randomLetterInt == 12) {
            return 'L';
        }
        if (randomLetterInt == 13) {
            return 'M';
        }
        if (randomLetterInt == 14) {
            return 'N';
        }
        if (randomLetterInt == 15) {
            return 'O';
        }
        if (randomLetterInt == 16) {
            return 'P';
        }
        if (randomLetterInt == 17) {
            return 'Q';
        }
        if (randomLetterInt == 18) {
            return 'R';
        }
        if (randomLetterInt == 19) {
            return 'S';
        }
        if (randomLetterInt == 20) {
            return 'T';
        }
        if (randomLetterInt == 21) {
            return 'U';
        }
        if (randomLetterInt == 22) {
            return 'V';
        }
        if (randomLetterInt == 23) {
            return 'W';
        }
        if (randomLetterInt == 24) {
            return 'X';
        }
        if (randomLetterInt == 25) {
            return 'Y';
        }
        if (randomLetterInt == 26) {
            return 'Z';
        }
    }
    char InverseLegacy::legacyRandomLetterLowercase() {
        int randomLetterInt = InverseLegacy::legacyRandomInt(1, 26);
        if (randomLetterInt == 1) {
            return 'a';
        }
        if (randomLetterInt == 2) {
            return 'b';
        }
        if (randomLetterInt == 3) {
            return 'c';
        }
        if (randomLetterInt == 4) {
            return 'd';
        }
        if (randomLetterInt == 5) {
            return 'e';
        }
        if (randomLetterInt == 6) {
            return 'f';
        }
        if (randomLetterInt == 7) {
            return 'g';
        }
        if (randomLetterInt == 8) {
            return 'h';
        }
        if (randomLetterInt == 9) {
            return 'i';
        }
        if (randomLetterInt == 10) {
            return 'j';
        }
        if (randomLetterInt == 11) {
            return 'k';
        }
        if (randomLetterInt == 12) {
            return 'l';
        }
        if (randomLetterInt == 13) {
            return 'm';
        }
        if (randomLetterInt == 14) {
            return 'n';
        }
        if (randomLetterInt == 15) {
            return 'o';
        }
        if (randomLetterInt == 16) {
            return 'p';
        }
        if (randomLetterInt == 17) {
            return 'q';
        }
        if (randomLetterInt == 18) {
            return 'r';
        }
        if (randomLetterInt == 19) {
            return 's';
        }
        if (randomLetterInt == 20) {
            return 't';
        }
        if (randomLetterInt == 21) {
            return 'u';
        }
        if (randomLetterInt == 22) {
            return 'v';
        }
        if (randomLetterInt == 23) {
            return 'w';
        }
        if (randomLetterInt == 24) {
            return 'x';
        }
        if (randomLetterInt == 25) {
            return 'y';
        }
        if (randomLetterInt == 26) {
            return 'z';
        }
    }
    void InverseDebug::testSysFunctions() {
        std::cout << "Testing all system functions of InverseAPI. This will take a few moments.\n";
        std::cout << "Testing InverseSystem::getSystemTime. If test succeeds, should output current Unix Time.\n";
        std::cout << InverseSystem::getSystemTime() << std::endl;
        std::cout << "Testing InverseSystem::runSystemCommand. If test succeeds, should open a command line window asking you to press a key.\n";
        InverseSystem::runSystemCommand("pause");
        std::cout << "All tests finished.\n";
    }
    void InverseDebug::testIntFunctions() {
        std::cout << "Testing all functions of InverseAPI that return an integer. This will take a few moments.\n";
        std::cout << "Testing InverseRandom::randomInteger. If test succeeds, should output a random integer between 1 and 100.\n";
        std::cout << InverseRandom::randomInteger(1, 100) << std::endl;
        std::cout << "Testing InverseRandom::randomDigit. If test succeeds, should output a random integer between 0 and 9.\n";
        std::cout << InverseRandom::randomDigit() << std::endl;
        std::cout << "Testing InverseConversion::charDecimalValue. If test succeeds, should output 97.\n";
        std::cout << InverseConversion::charDecimalValue('a');
        std::cout << "All tests finished.\n";
    }
    void InverseData::tempInt(int valueOfTempInt) {
        int tempInt = valueOfTempInt;
    }
    void InverseData::tempChar(char valueOfTempChar) {
        char tempChar = valueOfTempChar;
    }
    double InverseConversion::kilogramToPound(double amountInKilograms) {
        return amountInKilograms * 2.2046;
    }
    double InverseConversion::poundToKilogram(double amountInPounds) {
        return amountInPounds / 2.2046;
    }
    bool InverseMath::isEven(int numberToCheck) {
        if (numberToCheck % 2 == 0) {
            return true;
        }
        else {
            return false;
        }
    }
    bool InverseMath::isOdd(int numberToCheck) {
        if (numberToCheck % 2 == 0) {
            return false;
        }
        else {
            return true;
        }
    }
    double InverseConversion::yardToFoot(double amountInYards) {
        return amountInYards * 3;
    }
    double InverseConversion::footToYard(double amountInFeet) {
        return amountInFeet / 3;
    }
    double InverseMath::inverseCube(double numberToCube) {
        return InverseMath::inverseExponents(numberToCube, 3);
    }
    void InverseFileIO::readFile(std::string fileToRead) {
        std::ifstream readStream;
        readStream.open(fileToRead);
        if (!readStream.is_open()) {
            std::cout << "InverseCobalt: InverseFileIO: readFile: Encountered malfunction while processing file. File is either non-existent or damaged." << std::endl;
        } else {
            char currentCharacterFromFile;
            while (readStream.get(currentCharacterFromFile)) {
                std::cout << currentCharacterFromFile;
            }
        }
        readStream.close();
    }
    void InverseFileIO::writeStringToFile(std::string fileToWriteTo, std::string stringToWrite) {
        std::ofstream writeStream(fileToWriteTo, std::ios::app);
        if (!writeStream.is_open()) {
            std::cout << "InverseCobalt: InverseFileIO: writeStringToFile: Encountered malfunction while processing file. File is either non-existenet or damaged." << std::endl;
        }
        else {
            writeStream << stringToWrite;
            writeStream.close();
        }
    }
    void InverseFileIO::writeStringOverFile(std::string fileToWriteTo, std::string stringToWrite) {
        std::ofstream writeStream(fileToWriteTo, std::ios::trunc);
        if (!writeStream.is_open()) {
            std::cout << "InverseCobalt: InverseFileIO: writeStringOverFile: Encountered malfunction while processing file. File is either non-existenet or damaged." << std::endl;
        }
        else {
            writeStream << stringToWrite;
            writeStream.close();
        }
    }
}

InverseAPI.h:

#pragma once
// InverseAPI (Cobalt)
// Developed by Sahibdeep Nann (@SahibdeepNann) 
// https://github.com/InitializeSahib/InverseAPI
// Licensed under the MIT License
#ifdef INVERSEAPI_EXPORTS
#define INVERSECOBALT_API __declspec(dllexport)
#else
#define INVERSECOBALT_API __declspec(dllimport)
#endif
#include <ctime>
#include <string>
namespace InverseCobalt {
    class InverseMath {
    public:
        static INVERSECOBALT_API double inverseExponents(double baseNumber, double exponentNumber);
        static INVERSECOBALT_API double inverseSquare(double numberToSquare);
        static INVERSECOBALT_API double inverseCube(double numberToCube);
        static INVERSECOBALT_API bool isEven(int numberToCheck);
        static INVERSECOBALT_API bool isOdd(int numberToCheck);
    };
    class InverseSystem {
    public:
        static INVERSECOBALT_API void runSystemCommand(const char *commandToRun);
        static INVERSECOBALT_API time_t getSystemTime();
    };
    class InverseRandom {
    public:
        static INVERSECOBALT_API int randomInteger(int minimumInteger, int maximumInteger);
        static INVERSECOBALT_API int randomDigit();
        static INVERSECOBALT_API char randomLetterUppercase();
        static INVERSECOBALT_API char randomLetterLowercase();
    };
    class InverseConversion {
    public:
        static INVERSECOBALT_API int charDecimalValue(char characterToCheck);
        static INVERSECOBALT_API char decimalValueChar(int decimalValueToCheck);
        static INVERSECOBALT_API double kilogramToPound(double amountInKilograms);
        static INVERSECOBALT_API double poundToKilogram(double amountInPounds);
        static INVERSECOBALT_API double yardToFoot(double amountInYards);
        static INVERSECOBALT_API double footToYard(double amountInFeet);
    };
    class InverseDebug {
    public:
        static INVERSECOBALT_API void testAllFunctions();
        static INVERSECOBALT_API void testCharFunctions();
        static INVERSECOBALT_API void testIntFunctions();
        static INVERSECOBALT_API void testSysFunctions();
    };
    class InverseLegacy {
    public:
        static INVERSECOBALT_API int legacyIntSquareRoot(int integerToSquareRoot);
        static INVERSECOBALT_API int legacyRandomInt(int minimumInteger, int maximumInteger);
        static INVERSECOBALT_API char legacyRandomLetterUppercase();
        static INVERSECOBALT_API char legacyRandomLetterLowercase();
    };
    class InverseData {
    public:
        static INVERSECOBALT_API void tempInt(int valueOfTempInt);
        static INVERSECOBALT_API void tempChar(char valueOfTempChar);
    };
    class InverseFileIO {
    public:
        static INVERSECOBALT_API void readFile(std::string fileToRead);
        static INVERSECOBALT_API void writeStringToFile(std::string fileToWriteTo, std::string stringToWrite);
        static INVERSECOBALT_API void writeStringOverFile(std::string fileToWriteTo, std::string stringToWrite);
    };
}
\$\endgroup\$
  • \$\begingroup\$ Perhaps instead of "Library for C++ programs," a better title would be a description of what the code does, i.e. what does inverse mean/do? \$\endgroup\$ – Der Kommissar Mar 26 '16 at 23:52
4
\$\begingroup\$

This seems redundant:

if (randomLetterInt == 1) {
        return 'a';
    }
    if (randomLetterInt == 2) {
        return 'b';
    }
    if (randomLetterInt == 3) {
        return 'c';
    }

.... etc.

Replace with an array lookup.

  static char randomLetter[] = "abcdefghtijklmnopqrstuvwxyz";
  return randomLetter[randomLetterInt-1];

You dont want to build a random nuber generator every time you want a random number. You byuild the generator once then ask for numbers,

int InverseRandom::randomDigit() {

    // build the generator once
    static   std::mt19937 generateDigit(time(0));
    ^^^^^^


    // It would be better to create one generator 
    // that is used for all calls


    std::uniform_int_distribution<int> integerDistribution(0, 9);
    return integerDistribution(generateDigit);
}

Lots of static methods is very OO. But C++ is not just OO.

class InverseMath {
public:
    static INVERSECOBALT_API double inverseExponents(double baseNumber, double exponentNumber);
    static INVERSECOBALT_API double inverseSquare(double numberToSquare);
    static INVERSECOBALT_API double inverseCube(double numberToCube);
    static INVERSECOBALT_API bool isEven(int numberToCheck);
    static INVERSECOBALT_API bool isOdd(int numberToCheck);
};

A better technique for C++ is to use namespace.

namespace InverseMath {
    INVERSECOBALT_API double inverseExponents(double baseNumber, double exponentNumber);
    INVERSECOBALT_API double inverseSquare(double numberToSquare);
    INVERSECOBALT_API double inverseCube(double numberToCube);
    INVERSECOBALT_API bool isEven(int numberToCheck);
    INVERSECOBALT_API bool isOdd(int numberToCheck);
};

These functions do nothing:

void InverseData::tempInt(int valueOfTempInt) {
    int tempInt = valueOfTempInt;
//  ^^^^ this is a local variable
//       Once scope is left the vale disappears.   
}
void InverseData::tempChar(char valueOfTempChar) {
    char tempChar = valueOfTempChar;
//  ^^^^
//  klop
}

Using an if condition to return true/false. You already have a boolean in the test.

    if (numberToCheck % 2 == 0) {
        return true;
    }
    else {
        return false;
    }

This is easier to write as:

    return (numberToCheck % 2 == 0);

Reading a file:

There is no point in using two lines. When the constructor will do it in one line.

    std::ifstream readStream;
    readStream.open(fileToRead);

Why not just

    std::ifstream readStream(fileToRead);

There is no reason to explicitly close a file.

    readStream.close();

In fact there are situations where this is bad. But in general you should let the destructor close the file. The destructor will close the file (and importantly catch and discard exceptions). You should only call close() explicitly if you want to explicitly handle error conditions (there are cases where this is important, but usually not).

\$\endgroup\$
  • \$\begingroup\$ it is marked as legacy code, so I presumed it was not worth mentioning as he has a better function already built \$\endgroup\$ – spyr03 Sep 5 '15 at 3:31
  • \$\begingroup\$ I would have padded the array so that a was at index 1. \$\endgroup\$ – Hurkyl Sep 5 '15 at 4:08
  • \$\begingroup\$ Did you mean "Lots of static methods is not very OO"? \$\endgroup\$ – 200_success Sep 5 '15 at 5:06
  • 1
    \$\begingroup\$ No I mean that putting everything in a class is OO. It based on languages that don't have other other paradigms (like Java). \$\endgroup\$ – Martin York Sep 5 '15 at 6:03
  • \$\begingroup\$ The array look-up that saved 50 lines of code.. like a boss \$\endgroup\$ – Caridorc Sep 5 '15 at 15:04
0
\$\begingroup\$
int InverseRandom::randomDigit() {
    std::mt19937 generateDigit(time(0));
    std::uniform_int_distribution<int> integerDistribution(0, 9);
    return integerDistribution(generateDigit);
}

Just make use of the already written method and pass in a range of 0 - 9


char InverseRandom::randomLetterUppercase() {
    std::mt19937 generateLetterInt(time(0));
    std::uniform_int_distribution<int> integerDistribution(1, 26);
    int randomLetterInt = integerDistribution(generateLetterInt);
    return (char)'A' - 1 + randomLetterInt;
}

If you generate from 0 to 25, you wont need that - 1


bool InverseMath::isEven(int numberToCheck) {
    if (numberToCheck % 2 == 0) {
        return true;
    }
    else {
        return false;
    }
}

This is just

return !(numberToCheck % 2);

And it is really similar with the check if a number is odd


int InverseLegacy::legacyIntSquareRoot(int numberToSquareRoot) {
    return sqrt(numberToSquareRoot);

If it is legacy, why is it here? Why is it legacy code when there is no replacement for it?


double InverseMath::inverseSquare(double numberToSquare) {
    return InverseMath::inverseExponents(numberToSquare, 2);
}

Or

return numberToSquare * numberToSquare;

I think keeping it simple is the way to go. When would you really need a function to square a number though?

That's all i've got, its not much, so hopefully someone can comment on the c++ itself

\$\endgroup\$
  • \$\begingroup\$ You mean "is just return numberToCheck % 2 == 0;". \$\endgroup\$ – Hurkyl Sep 5 '15 at 4:10
  • \$\begingroup\$ @Hurkyl Ah, I was checking for odd, I'll edit it \$\endgroup\$ – spyr03 Sep 5 '15 at 14:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.