#I'm a beginner and I wanted to know if my code is decently written

11 messages · Page 1 of 1 (latest)

thick prism
#

It get environmental variables and displays them

#include <iostream>
#include <string>
#include <filesystem>
#include <Windows.h>

int main() {
    std::string keep_open;
    // Check to see if env variables were successfully retrieved 
    if (std::getenv("USERNAME") != nullptr && std::getenv("NUMBER_OF_PROCESSORS") != nullptr && std::getenv("PROCESSOR_LEVEL") != nullptr && std::getenv("OS") != nullptr) {
        // Create the array to hold the info gathered from the env variables
        std::string info_array[4] = {
            std::getenv("USERNAME"),
            std::getenv("NUMBER_OF_PROCESSORS"),
            std::getenv("PROCESSOR_LEVEL"),
            std::getenv("OS")
        };
        // Display the contents of the array one by one
        std::cout << info_array[0] << "\n";
        std::cout << info_array[1] << "\n";
        std::cout << info_array[2] << "\n";
        std::cout << info_array[3] << "\n";
        std::cin >> keep_open;
    } else {
        // Display the error if it couldn't correctly get one of the env variables
        DWORD error = GetLastError();
        std::cout << "ERROR: " << error;
        std::cin >> keep_open;
        std::exit(0);
    }
}
urban estuary
#

You can also improve the readability by adding some empty lines

#

I also don't think that you need "#include <filesystem>"

#

And instead of std::cin >> keep_open; you can do system("pause") since this is windows only or cin.get()

#

No need to declare a variable then

thick prism
#

Ohh ok, thanks

#

@urban estuary Is this better?

#include <iostream>
#include <string>
#include <Windows.h>

int main() {
    // Check to see if env variable were successfully retrieved 
    if (std::getenv("USERNAME") != nullptr && std::getenv("NUMBER_OF_PROCESSORS") != nullptr && std::getenv("PROCESSOR_LEVEL") != nullptr && std::getenv("OS") != nullptr) {
        // Create the array to hold the info gathered from the env variables
        std::string info_array[4] = {
            std::getenv("USERNAME"),
            std::getenv("NUMBER_OF_PROCESSORS"),
            std::getenv("PROCESSOR_LEVEL"),
            std::getenv("OS")
        };
        // Display the contents of the array one by one
        for (auto& info : info_array) {
            std::cout << info << "\n";
        }
        std::system("pause");
    } else {
        // Display the error if it couldn't correctly get one of the env variables
        DWORD error = GetLastError();
        std::cout << "ERROR: " << error;
        std::system("pause");
        std::exit(0);
    }
}
frank dawn
#

std::string_view is preferable to std::string here

frank dawn
proper berry
#

It is decent, but it can always be improved. 🙂

With some redesign, I would suggest the following. simpler structure:

1. variables = ...
2. if for any variable in variables std::getenv(variable) is null:
3.   log the error
4.   exit

5. for every variable in variables:
6.   print std::getenv(variable)

For the line 2, there's a library function std::any_of.