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.

Sign up for GitHub

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

Jump to bottom

Internal: replace struct "Revolved" #465

Closed
phkahler opened this issue Aug 22, 2019 · 4 comments
Closed

Internal: replace struct "Revolved" #465

phkahler opened this issue Aug 22, 2019 · 4 comments
Labels
internals

Comments

@phkahler
Copy link
Member

In surface .cpp there is a struct defined as:

`

typedef struct {
    hSSurface d[100];
} Revolved;

`
This was originally used in Lathe groups to temporarily collect 4 90 degree sections of a surface. I expanded it to 100 to support more sections in Helix groups, but having a fixed number in there is not so great.

Also, in one of my branches (draft) I wanted to pass a list of these to a function and so had to move the definition into surface.h which to me is also a bit undesirable.

A better container for these would be nice. Is the right answer just a list? I'm not fan of doing things like list of lists in C++ but even that is better than making this a fixed size. @rpavlik ?
@phkahler phkahler changed the title Replace struct "Revolved" Internal: replace struct "Revolved" Aug 22, 2019
@rpavlik
Copy link
Contributor

rpavlik commented Aug 22, 2019 •
edited

Well, if it needs the slightly-weird semantics of List<> (that is, calling clear on everything before removing it), then use List<> otherwise use std::vector directly.

It sounds like you probably want std::vector.

@whitequark
Copy link
Contributor

whitequark commented Aug 22, 2019 •
edited

Yeah, fixed-size containers are rather undesirable in my view, and I've tried to eliminate them before as they tend to create problems at some point.

We don't really have a policy on using List vs std::vector in existing code. I've used std::vector in parts that are self-contained and completely new (like new export functions), but I'd say if it's a part of the geometry code then List would be less annoying to work with.

But this is fairly minor and just my opinion.

@phkahler
Copy link
Member Author

std::vector worked out nicely because we know what length is needed when they're created.

@phkahler
Copy link
Member Author

This was done in PR #466 commits ced879c and cad7002

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
internals
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@whitequark @rpavlik @phkahler