search

ChickenProp / cloggle

OpenGL library for Clojure based on JOGL
8 stars 0 forks source link

Clojure Idiomaticity #1

Open elliottslaughter opened 15 years ago

elliottslaughter commented 15 years ago

I'd like to propose a couple of suggestions for making cloggle more idiomatic. I'm new to Clojure, but have been a Common Lisp user for a while now, so I still think my suggestions apply to Clojure.

Wrapper macro names should begin with "with-". (For examples just look at all the API functions that have a "with-" prefix, like with-in-str, with-local-vars, etc.)

Specifically, I think ctx should be either with-ctx, with-context, or something similar.

I suggest doing the same with your beg-end macro: with-primitive is the name used by cl-opengl, is probably my first choice.

Similarly, it would be nice to have a with-pushed-matrix macro which handles calling glPushMatrix and glPopMatrix.

My other general suggestion is that you name global variables with asterisks (e.g. *ns* in the Clojure API). So opengl-context should be *opengl-context*, etc.

Hope this helps. I've learned a lot from your source code so far and look forward to working with it more :-)

Let me know if you'd like me to provide a patch with these changes.

Thanks.

elliottslaughter commented 15 years ago

One more point: Tabs are not idiomatic in Lisp. Use spaces!

elliottslaughter commented 15 years ago

I made a fork with my suggested changes. Feel free to use them as you please.

http://github.com/slaguth/cloggle/tree/master

elliottslaughter commented 15 years ago

Ok, I just added another 3 patches which accomplish about 75% of your renaming todo item in the README.

I managed to rename glFuncNames to func-names and GL_FIELD_NAMES to gl-field-names (modulo glFlush which couldn't be made public because of a conflict with clojure.core/flush). I didn't manage to remove the last gl- from the field names because of conflicts with various function names.

Enjoy :-)

elliottslaughter commented 15 years ago

One more update. I added a patch which automatically generates convenience functions (like what was done manually for vertex and color). It works by stemming all of the function names, partitioning them by their stemmed names, and then generating methods for those stemmed names.

ChickenProp commented 15 years ago

Thank you very much for this! I've merged your patches and pushed them to my repo.

I think with-context would be better called with-gl, as context is rather generic (though still miles better than "ctx" was). I'll change that when I get a moment.

The convenience methods are also not yet perfect: the weak forms (including Ratios) aren't accepted, and the ::nums catch-all won't work to emulate functions like light?v like it does vertex??v. I'll have a think about how best to handle this. It may be that convenience functions need to be specified with more precision than can be completely automated.

Aside from that, this is very helpful. Thanks again!