Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vertex Phasors Class #209

Merged
merged 17 commits into from
Jan 13, 2023
Merged

Vertex Phasors Class #209

merged 17 commits into from
Jan 13, 2023

Conversation

willGraham01
Copy link
Collaborator

@willGraham01 willGraham01 commented Jan 4, 2023

Continues #63 |

VertexPhasors class

Along similar lines to #205, breaks out extraction of the phasors at the user-specified vertices into its own class. This reduces the number of auxiliary functions defined in iterator.cpp, moving the relevant methods into this class and adapting the syntax accordingly.

The ComplexAmplitudeSample class previously that previously handled the data storage has been absorbed into the VertexPhasors class, and hence no longer appears in the codebase. Previously, this class was one of those in arrays.h for data storage only.

Fieldsample class-breakout

Breaks out FieldSample class into it's own source and header file, since it can now handle the "fieldsample extraction" portion of iterator.cpp internally, within it's own class.

Reason for the file-breakout:

  • arrays.{h,cpp} should contain only basic data containers and related methods. Objects with their own header/source are those that are interacting with other fields, classes etc that we have defined in fields.h or arrays.h.
  • arrays.{h,cpp} is a huge file of declarations and definitions, and adding complex functionality in admist all that is going to come back and bite us if we keep going like this. Might create some more subfolders in src if we end up breaking out a lot of files like this.

Other minor things in this PR

  • test_FieldSample.cpp has been moved out of the array_tests testing subfolder since it's no longer part of arrays.h.
  • test_ComplexAmplitudeSample.cpp has been repurposed into test_VertexPhasors.cpp.
  • test_interpolation_determination.cpp was missing a spdlog:: string.

@willGraham01 willGraham01 marked this pull request as ready for review January 5, 2023 08:57
@samcunliffe samcunliffe self-requested a review January 10, 2023 15:58
Copy link
Member

@samcunliffe samcunliffe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

My only really comment: I wonder if we should have an abstract Phasors class. Can you factor out any common functionality? Does it buy us anything?

tdms/include/vertex_phasors.h Show resolved Hide resolved
tdms/src/iterator.cpp Outdated Show resolved Hide resolved
tdms/src/iterator.cpp Show resolved Hide resolved
tdms/include/fieldsample.h Outdated Show resolved Hide resolved
tdms/include/fieldsample.h Outdated Show resolved Hide resolved
tdms/src/fieldsample.cpp Show resolved Hide resolved
@willGraham01 willGraham01 merged commit 85b7562 into main Jan 13, 2023
@willGraham01 willGraham01 deleted the wgraham-vertex_phasors_class branch January 13, 2023 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants