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

_BoundaryConditions() v BoundaryConditions() #84

Closed
christophernhill opened this issue Feb 26, 2019 · 3 comments
Closed

_BoundaryConditions() v BoundaryConditions() #84

christophernhill opened this issue Feb 26, 2019 · 3 comments
Assignees
Labels
abstractions 🎨 Whatever that means cleanup 🧹 Paying off technical debt
Milestone

Comments

@christophernhill
Copy link
Member

@ali-ramadhan should this line

https://github.com/ali-ramadhan/Oceananigans.jl/blob/master/src/model.jl#L25

be

boundary_conditions = _BoundaryConditions(:periodic, :periodic, :rigid_lid, :free_slip)

instead of

boundary_conditions = BoundaryConditions(:periodic, :periodic, :rigid_lid, :free_slip)

?

@ali-ramadhan
Copy link
Member

Yes! Thanks for catching this.

Yeah it's ugly because I was trying to have some input sanitation before creating the boundary conditions struct but BoundaryConditions(:periodic, :periodic, :rigid_lid, :free_slip) would have conflicted with the default struct constructor.

There must be some way of doing this nicely but I don't know enough Julia right now...

@vchuravy
Copy link
Collaborator

Should be a perfect example of a inner constructor (which I really only recommend for input sanitization and invariants checking).

struct BoundaryConditions
    x_bc::Symbol
    y_bc::Symbol
    top_bc::Symbol
    bottom_bc::Symbol

    function BoundaryConditions(x_bc, y_bc, top_bc, bottom_bc)
        @assert x_bc == :periodic && y_bc == :periodic "Only periodic horizontal boundary conditions are currently supported."
        @assert top_bc == :rigid_lid "Only rigid lid is currently supported at the top."
        @assert bottom_bc in [:no_slip, :free_slip] "Bottom boundary condition must be :no_slip or :free_slip."
        new(x_bc, y_bc, top_bc, bottom_bc)
    end
end

Instead of Symbols you can also use @enum, but that is a stylistic choice.

@ali-ramadhan
Copy link
Member

Thanks for the tip, did not know about inner constructors!

@ali-ramadhan ali-ramadhan reopened this Feb 27, 2019
@ali-ramadhan ali-ramadhan added this to the v0.5 milestone Mar 2, 2019
@ali-ramadhan ali-ramadhan added cleanup 🧹 Paying off technical debt abstractions 🎨 Whatever that means labels Mar 2, 2019
@ali-ramadhan ali-ramadhan self-assigned this Mar 17, 2019
ali-ramadhan added a commit that referenced this issue Oct 19, 2020
Periodic advection verification experiment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abstractions 🎨 Whatever that means cleanup 🧹 Paying off technical debt
Projects
None yet
Development

No branches or pull requests

3 participants