bschwind / opencascade-rs

Rust bindings to the OpenCascade CAD Kernel
GNU Lesser General Public License v2.1
100 stars 18 forks source link

Proposal: Consistent naming / patterns #17

Open DSchroer opened 1 year ago

DSchroer commented 1 year ago

Currently the naming of methods and functions in lib.rs is inconsistent:

Proposal:

Naming

  1. Always use the Opencascade convention for C++. (CammelCase with no _, including standalone functions). Not the rust convention.

    type_name(...) -> TypeName(...)
  2. Since cxx has no better way yet, use _ to show associated types for functions that cant be bound using self. This covers constructors and other similar functions.

    HandleGeomCurve_Value
  3. In lib.rs, since it is in rust, we always use rust naming conventions and make use of the cxx_name attribute to point to the correct c++ method.

    #[cxx_name = "Shape"]
    pub fn shape(self: Pin<&mut BRepAlgoAPI_Fuse>) -> &TopoDS_Shape;

Access

Free floating functions are hard to reason about. Since Opencascade is a class based library, we should follow the same by re-attaching methods to the respective parent. If a function is associated with a type it should be private and attached via an impl block to the associated type.

...
    type StlAPI_Writer;
    fn StlAPI_Writer_ctor() -> UniquePtr<StlAPI_Writer>;
  }
}
...

impl ffi::StlAPI_Writer {
    pub fn new() -> cxx::UniquePtr<ffi::StlAPI_Writer> {
        ffi::StlAPI_Writer_ctor()
    }
}

What do you think? I feel this will be a good step forward and help reason about how future additions to the -sys crate should be made.

bschwind commented 1 year ago

This sounds reasonable to me!

I hadn't put much thought into organization as you can probably tell, my original goal was to just get this working on the bottle tutorial and get a feel for how to bind to C++ code.

You're right to be thinking about better organization now though so it doesn't become a pain.

Maybe the best way to start is to apply what you've suggested to one of the opencascade types and open a PR so we can get a feel for how it'll look.

Eventually I want to make a much higher level layer on top of all this, but it will be very helpful to more easily bind to the lower level stuff with your suggestions.