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

Bad Default: Assembly.getArea() is 1.0 #1682

Open
john-science opened this issue Apr 5, 2024 · 3 comments
Open

Bad Default: Assembly.getArea() is 1.0 #1682

john-science opened this issue Apr 5, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@john-science
Copy link
Member

john-science commented Apr 5, 2024

Over on this PR, I noted a strange point in the ARMI API.

When an Assembly has no children/Blocks, this method returns a "default area" of 1.0. Surely that should be an error? Or it should return None. Or it should return 0.0. Returning a valid numerical value just seems crazy.

except IndexError:
runLog.warning(
"{} has no blocks and therefore no area. Assuming 1.0".format(self)
)
return 1.0

@john-science john-science added the enhancement New feature or request label Apr 5, 2024
@albeanth
Copy link
Member

albeanth commented Apr 9, 2024

I say make it None. Rip the band aid off!

@john-science john-science self-assigned this Apr 9, 2024
@jakehader
Copy link
Member

This might be one of those "not all things in the composite pattern" should be implemented kind of thing. I'd be careful with areas though. Typically you make a composite and then add things to it. Not sure if it should be None either. Can you point to the code here?

@albeanth
Copy link
Member

This might be one of those "not all things in the composite pattern" should be implemented kind of thing. I'd be careful with areas though. Typically you make a composite and then add things to it. Not sure if it should be None either. Can you point to the code here?

except IndexError:
runLog.warning(
"{} has no blocks and therefore no area. Assuming 1.0".format(self)
)
return 1.0

I'd say None with a runLog.warning would be appropriate. Or you could raise an error. That'd work too. If someone is creating an Assembly and then querying its area without a Block I think it would be a mistake and getting 1.0 would be unexpected. But maybe there's an application I'm not aware of where an Assembly can do "stuff" without a Block?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants