gbowne1 / gradebook-management-system

This is a C++ Gradebook Management System
MIT License
2 stars 2 forks source link

Improving the Gradebook program initial code #15

Closed gbowne1 closed 8 months ago

gbowne1 commented 8 months ago

I've added some fairly simple quality of life improvements to the original code with some things that also do some error handling on the way to more modularity so that we can add more features.

gbowne1 commented 8 months ago

@andrewlod

When I synced back after the last merge, I got include errors in VSCode so to fix that I changed the include path.

Would it be better to modify the search / browse path so that VSCode can find the flies?

I do not have 3.14 installed. I only have 3.12. Would that cause an issue?

I would suggest that the minimum required version should be set to my version if not 3.10 unless any dependent requires a certain version.

I had some other updates I thought about but held back on those as I mainly wanted to work on error handling

It could use some documentation and comments. I stuck in a little bit of that. I think we should doxygen.

Thoughts?

andrewlod commented 8 months ago

@gbowne1

Would it be better to modify the search / browse path so that VSCode can find the flies?

I have checked the .vscode/c_cpp_properties.json file, where information about the include paths is located:

"includePath": [
        "${workspaceFolder}/gradebook/include/**",
        "${workspaceFolder}/build/_deps/**",
        "/usr/include/**",
        "/usr/local/include/**"
]

${workspaceFolder}/gradebook/include/** should include all header files. Strange that your VSCode showed include errors, maybe it didn't update? I use the default Microsoft extension for C/C++ and it didn't complain about the include path.

I do not have 3.14 installed. I only have 3.12. Would that cause an issue?

After reading the CMake documentation on FetchContent, it says that the instructions FetchContent_Declare and FetchContent_MakeAvailable are only available on CMake 3.14+. There is a workaround for lower versions:

In some cases, the main project may need to have more precise control over the population or may be required to explicitly define the population steps (e.g. if CMake versions earlier than 3.14 need to be supported). The typical pattern of such custom steps looks like this:

FetchContent_GetProperties(googletest)
if(NOT googletest_POPULATED)
  FetchContent_Populate(googletest)
  add_subdirectory(${googletest_SOURCE_DIR} ${googletest_BINARY_DIR})
endif()

Another workaround would be, as I wrote earlier, cloning the googletest repository as a submodule of the project, then building it with add_subdirectory. This would not require FetchContent at all.

It could use some documentation and comments. I stuck in a little bit of that. I think we should doxygen.

I have wrote Doxygen-style comments on DataFrame, but other files indeed need to be commented. It is a best practice to put these comments on header files, as I did on DataFrame. I don't have experience with Doxygen documentation generation, so I'll have to take a look at it, but it seems like it is a solid plan.

gbowne1 commented 8 months ago

@andrewlod

I think we should comment wherever possible while keeping the project clean. Doxygen shouldn't be an issue here as its fairly easy to use.

I am looking to update and upgrade my development machine and two development VMs I use as well very soon. I have two development VMs I use one intentionally has old packages the other has almost nightly builds. It appears I might need to do sone tweaking to my environment. I use the older VMs to make sure that things that get released will still work in a slightly older environment as well as tested there too (part of my actual job as well).

My local dev machine has a configuration that is somewhat between those two and I also use C/C++ and CMake tools extension and the settings are synced across all of them.

I did discover recently in a C/C++project that I had the run/debug launch.json done incorrectly

I really need to publish my global VSCode configuration.

I had just noticed that it couldn't find the files when you changed the structure a bit. It had worked fine before that.

If the search / browse path needs updating to keep that from happening I would say to try and include path that will give it the best chance of finding the headers even if things get changed or moved around.

I will work on environment updates this weekend.

andrewlod commented 8 months ago

@gbowne1

I think we should comment wherever possible while keeping the project clean. Doxygen shouldn't be an issue here as its fairly easy to use.

Agreed. I had documented DataFrame before, I was focused on other issues and couldn't document the other files, but I will add that to my TODO list, as well as implementing a Doxygen build.

About the VSCode configuration, I think that makes sense. When I load the project on VSCode, it just gets whatever configuration is inside the .vscode folder.