search

TravisWhitaker / libPOLY

A Musical Instrument for Computers
MIT License
13 stars 7 forks source link

Major style changes #6

Closed msoucy closed 8 years ago

msoucy commented 10 years ago

Including turning some pointer addition into array indices (there's literally no difference from the generated ASM, it's just to make things more legible)

gambogi commented 10 years ago

Travis likes 8 width tabs (K&R and all that jazz), but I don't see any reason not to make the array indices changes.

Mikejmoffitt commented 10 years ago

Tab width shouldn't matter as if anyone is being reasonable they should be tab characters so it shows up properly for whoever is editing it. Spaces should only be included for things that need to visually line up. There are very few of those.

gambogi commented 10 years ago

Wow I misread that diff. They are currently tab characters, this would be a move to 4 space indentation.

TravisWhitaker commented 10 years ago

Using anything but 8 width tabs in C is lobotomy-worthy. Torvalds does an excellent job of explaining why in the Linux kernel coding standards document, as does the appendix of K&R version 2 (version 3 supports the heretical viewpoint and didn't involve both K and R; it's also incorrect about bracket placement; long live version 2).

I'm fine with switching to array-style pointer arithmetic and neat-o struct field access. Note that the unused 'duty' variable in saw and triangle wave generator functions is a mistake in the header file, not the implementation. I'm surprised that clang didn't complain about that even in pedantic mode.

TravisWhitaker commented 10 years ago

I also misread that. The mobile GitHub interface doesn't even let you think about looking at the binary diff.

msoucy commented 10 years ago

So, the style was literally just me being dumb and running astyle. I can fix that easily.

The unused "duty" variable was to allow the "function pointer lookup" instead of the switch statements in poly_gen_kernel

msoucy commented 10 years ago

Also I'm a bad person and made this pull request off of my fork's master. I honestly don't care enough to fix that.

TravisWhitaker commented 10 years ago

I've found that function pointers are slower than letting the compiler decide to inline the generating functions; compare with my last attempt at building a synth library called libADSR. I haven't explicitly inlined these functions because there is no compiler-agnostic way of doing it and both GCC and Clang choose to inline them with -O2.