BrainSlugs83 / NewBiospheresMod

An update of Risugami's old Biospheres Mod for Minecraft. To avoid confusion, I've named it the *New* Biospheres Mod. :-)
Other
9 stars 8 forks source link

Code style is weird #30

Open TheLoneWolfling opened 9 years ago

TheLoneWolfling commented 9 years ago

Most coding styles for Java tend to loosely follow Oracle's Code Conventions for the Java Language, which, among other things, states that functions should start with a lowercase letter:

Methods should be verbs, in mixed case with
the first letter lowercase, with the first letter of
each internal word capitalized.

Except for variables, all instance, class,   and
class constants are in mixed case with a lower-
case first letter. Internal words start with capi-
tal letters.

The names of variables declared class con-
stants  and of ANSI constants should be all
uppercase with words separated by under-
scores (“_”). (ANSI constants should be
avoided, for ease of debugging.)

NBM seems to, among other things, often use capitalized function / variable names (GetDomeBlock, LruCacheList<Chunk> ChunksCache, GenerateChunk, etc.) . Is this intentional? Or are you used to a programming language with different conventions? Would you be interested in a pull request that changed this?

Ditto - Windows or Unix newlines?

BrainSlugs83 commented 9 years ago

It is intentional -- or was, I suppose. -- I'm used to the .NET naming conventions and standards -- and have a strong affinity for them (versus the Java ones) -- but I suppose, "when in Rome...".

Especially if I'm working with other Java developers on this project it seems I'll have to get used to at least making the APIs look like they belong in a Java package. -- So, yeah, if you want to submit a patch, I can review it.

Regarding newline characters -- I prefer to have Git store them as Unix style line endings in the repository and convert them to Windows when I pull or check out -- it should be converting them back again to Unix when I commit and push -- but now that you mention it -- this is a new OS installation, and my settings might not be here yet (the .NET tools have spoiled me! >.<) -- I will have to double check.

A couple of things I would hate to give up are curly brackets on a newline (i.e. "{" goes on a new line for me -- and is always required [no "if", "while", or "for" statements (etc.) without curly brackets]).

Also, sometimes if function calls, or other statements get crazy long, I like to break them out so they look like XML or JSON (for readability) here's an example:

blah.doit
(
    new Foo
    (
        new Bar
        (
            1, 5, 7, 9
        ),
        new Bar
        (
            11, 13, 15, 17
        )
    )
);

Is there anything like FxCop for Java? -- Like maybe "JavaCop" or something?

Also, the lack of Unit Testing is kind of driving me nuts -- not sure anyone is doing TDD with Minecraft, but ... I really fucking want to. Any thoughts?

BrainSlugs83 commented 9 years ago

How do you feel about a .gitattributes with core.autocrlf set to true? (I suppose false is probably fine too -- I'm only using Eclipse, and I don't think it cares.)

TheLoneWolfling commented 9 years ago

I thought you were from the .net ecosystem. That explains much.

I don't much like the whole 'brackets on a newline' thing, but meh. I find it ends up adding enough newlines that it becomes hard to read because even trivial things take up half the screen. Easy enough to have it swap back and forth on load / save, though, and if you keep that that's what I'll probably end up doing.

With braceless conditional statements? I tend to only use them for if (foo) continue;, or if (bar) break;. Though I saw you removed all of the continues I had anyways. Meh. I find continues easier to read (less indentation levels for the bulk of the code, especially if you have multiple failure points). Though if you want me to avoid them I will.

.gitattributes for the line endings is good.

CheckStyle is what tends to be used for Java, though it has... issues. Combines reasonably well with Eclipse though.

As for TDD? Look at JUnit, though most of NBM is so heavily tied into Minecraft that I'm not sure how you'd go about it without major refactoring.

BrainSlugs83 commented 9 years ago

Yeah, I found those continues harder to read -- it's just like silently lurking in there -- the indentation calls out that that whole block is clearly conditional -- feels easier on my eyes -- and conditionals without brackets ... just scream ... apple bug to me anymore -- not to mention -- I just don't like them for some reason (preference, I guess?)

You can feel free to submit code like that, but I'll probably take the time to format it the way I like it -- the automatic formatting you mention sounds interesting to me -- honestly, I wouldn't mind doing the reverse of what you mention (I'm clearly in the minority in the Java world, when it comes to code formatting, so I wouldn't mind having a tool that just formatted the code in a way that was easy for me to use, and put it back to the Java way when I saved it). -- Care to send some more details?

As for JUnit needing refactoring -- do you mean refactoring of the add-in's code? or of minecraft's? -- The add-in has a couple major refactors still ahead of it anyway, as I see it -- but if it's not going to work well with Minecraft, well, that will suck. :-/

TheLoneWolfling commented 9 years ago

Odd, I find it easier to read the other way. Probably, again, because I'm used to it. And I find myself too often having absurd indentation depths otherwise. (Far too often you have a loop with a bunch of alternating conditional bailout checks and variable assignments - each one adds an indentation level without continue/break.) Meh, I can work with it either way.

Again with braceless conditional blocks, I only use it for break / continue. And with auto-indentation it's relatively obvious when you've made a mistake like the apple bug.

Don't mind you formatting code however you wish in the slightest, it's your project after all!

W.r.t. automatic formatting, Eclipse's autoformatter is pretty good. Though I haven't found a way to automate formatting on load. So the easiest way I've found is just to have a keybind to Java > Code Style > Formatter in Eclipse (Window -> Preferences -> General -> Keys to change keybindings), and just swap between your code format and the project one.

Though it doesn't currently handle continues / breaks. Bleh.

Trying to do unit testing will be "fun". It'll require some major refactoring of NBM, but also there are a number of things that I'm not sure will be possible to test in any sensible manner. Even things as simple as block IDs pull in and load a huge chunk of Minecraft. And, if you're not careful, pull in things in the wrong order and cause weird crashes.

The easiest way I've found to do something vaguely TDD-like with Minecraft is to have a debug flag, which, if it is enabled, you wait until MC is loaded and then call JUnit explicitly to run the tests from within Minecraft with something like new JUnitCore().run(tests). But note that that won't really help with catching things that are a problem on initialization. And it prevents integration of JUnit with much of anything else.

BrainSlugs83 commented 9 years ago

On a side note -- I just saw during the build (day 2) keynote that you can develop Java based Minecraft mods in Visual Studio now. -- It looks like they're using C# style formatting in there (though, Java based naming conventions). I might have to check this out. -- I've been very unimpressed with most of Eclipses features (including code formatting, seemed rigid and not extremely reconfigurable). So, I'm pretty excited about the new Visual Studio integration.

For TDD -- and refactoring in general -- I'd like to do some more DI in this project -- just because it will make it easier to port it between different frameworks -- should alleviate some of the concerns with tying into Minecraft -- it's true that I won't be able to do any integration testing, but I'll at least be able to test my code for correctness at that point (we can still cause crashes, etc -- but that's okay, it just means that our idea of "correctness" was wrong, and will improve over time.) -- I wonder if you can use VS Test Explorer with JUnit...

BrainSlugs83 commented 9 years ago

On anoter side note -- I'm wondering if git can take plug-ins for pull and push -- so on pull, run every file matching a pattern through an auto-formatter -- and on push, run it through a different auto-formatter. -- Then you just never have to worry about it again...