KSP-KOS / KSLib

The standard library and examples for the Kerboscript language
MIT License
119 stars 40 forks source link

[discussion] Code style #31

Open abenkovskii opened 9 years ago

abenkovskii commented 9 years ago

We need a of code style for this repo. Here is an example of what happens if we don't have it: https://github.com/akrasuski1/KSLib/commit/75da26724bc423e8701df1eee4543e5c4cb82037 In my editor there were spaces and than they turned into tabs making it hard to see where the real difference is.

Dunbaratu commented 9 years ago

I think it may discourage contributions from newbie programmers to complain about things like not using the same type of declare parameter.

Tabs and spaces on the other hand are a big deal because they make fake diffs. Tabs really only work if you rigidly adopt the rule that all indents must consist of only tabs and never use any mix of tabs and spaces. If you do that, then individual viewers can set their tabstops where they like and effectively change the visual presentation of the whole file. But the instant there is a mix, it doesn't work because people viewing in different settings see different things and the lines don't match if a mix of people have edited it. Also, only in some contexts can you set the tab size. So sometimes hard tabs are going to appear way too wide in their standard size of 8 and you can't do anything about it (like in github diffs).

I favor a spaces-only rule to a tabs-only rule. But both are preferable to a mix of tabs and spaces.

But I see the appeal of tabs in kerboscript where people are trying to reduce char count. But I'm against that ridiculously tight limitation, and always have been. It encourages people to write horrible looking code with abbreviated gibberish variable names. People claiming "but they only had a few Kb on Apollo" keep forgetting that they weren't sending the source code, and they weren't using an object-oriented hardware abstraction. The comparison doesn't work.

abenkovskii commented 9 years ago

@Dunbaratu It's good that they didn't realize that making the whole program one long line considerably reduces the size of complied file.

abenkovskii commented 9 years ago

@Dunbaratu I vote for spaces for 2 reasons:

  1. my editor uses spaces
  2. there are no tabs in the built-in kOS editor
TDW89 commented 9 years ago

I use spaces so that would be my preference to. Also I really like the format:

parameter 
 arg1, //description of arg1
 arg2, //description of arg2
 arg3. //description of arg3

especially as an aid when other people are going to be using your lib files and don’t necessarily want to (or can't) go look at documentation for it because they forgot which way round the args were supposed to be.

abenkovskii commented 9 years ago

@TDW89 What do you think about

function foo {
  if condition {
  } else {
  }
}

vc

function foo
{
  if condition
  {
  }
  else
  {
  }
}

I personally prefer 2nd variant. P.S. I'm not 100% sure we should go for such a detailed code style. There are many kOS users that are not familiar with programming.

TDW89 commented 9 years ago

um not sure on that i have been using

function foo {
 if condition {
  stuff.
 }
 else {
 }.
}.

so similar to the top one but on continuing on a line after i have just closed a brace.

abenkovskii commented 9 years ago

@TDW89 Yes there are a lot of possible variations. I think I'm using the simplest rule possible: each brace on a separate line. P.S. Never knew that you can put . before else.

TDW89 commented 9 years ago

@abenkovskii you cant (updated example)

abenkovskii commented 9 years ago

@TDW89 actually you can screenshot4

TDW89 commented 9 years ago

ohhhh that’s new.

docs probably need changing then

abenkovskii commented 9 years ago

No. Let's not document it. It looks bad.

TDW89 commented 9 years ago

the docs currently have a statement saying that it will produce an error. I was thinking that that just needed removing as its not true any more. Not adding anything that explicitly states that it is now possible.

Dunbaratu commented 9 years ago

I always hate the extra blank vertical space that this standard causes:

    if blah
    {
        blah.
        blah.
    }
    else
    {
        blah.
        blah.
    }

It's a ratio of only half the lines being the code and the other half being formatting. That's a massive waste of precious vertical screen space, especially when you have people who think that a function's body must fit in one screenfull - I keep wanting to fire back, "but the actual code DOES fit. It's the idiotic wasteful formatting you forced on me that doesn't fit."

It gets worse if you decide to follow the (good) practice of using braces even when it's optional because the body is only one statement. Then the ratio of code to formatting gets even worse.

I rankle at the fact that erendrake chose that formatting for the mod.

And you don't have to go with type 1 as the alternative. If you like the braces to line up AND to not waste the lines, you have this:

    if blah
    {   blah.
        blah.
    } else
    {   blah.
        blah.
    }

You get both in one standard that way. But sadly, most people think it's impossible because their editors don't understand that it's an option and fancy IDE's keep trying to 'fix' it on them.

abenkovskii commented 9 years ago

That's one of the reasons why I like python: no braces - no problem

abenkovskii commented 9 years ago

It's actually a case of habit: for me your last variant looks awful and I'd prefer leaving braces at the end of the line if screen space is a problem.

abenkovskii commented 9 years ago

There is one more thing to discuss: UPPER_CASE, CamelCase and lower_case. Where to use which type? I personally don't use upper case and camel case with the exception of @LAZYGLOBAL OFF.

TDW89 commented 9 years ago

all file names will have to be lower case other than that i don’t really mind. If i remember correctly the docs have all the kOS words in UPPER_CASE but the variable names don't really follow a convention. One page uses PREV_TIME, MyList and numOUT

abenkovskii commented 9 years ago

for me numOUT looks like a weird mix

akrasuski1 commented 9 years ago

My opinion:

  1. Parameters. In case of just one parameter:
parameter stuff.

In case of many:

parameter
    par1,
    par2,
    par3
  1. Tabs/spaces. I prefer tabs.
  2. If/else braces.
function foo{
    if asd{
        stuff.
    }
    else{
        stuff2.
    }
}

(it wastes less space than each-brace-on-new-line, while still being common in editors. To me, Dunbaratau's proposition seems too weird.

  1. Case. I usually use lower_case_variables, but I don't really care if others use other. Note that abenkovskii used wrong terminology: ThisIsPascalCase, thisIsCamelCase.
Dunbaratu commented 9 years ago

@akrasuski1, If you're using the style where you put the open brace on the end of the line, not a new line, then that style typically has the else like this:

    } else {

not like this

    }
    else {

The whole point of this style is that you are not attempting to make the braces or the if/else keywords line up, instead communicating the nest level entirely through indentation level alone.

I started using underscore_variables because kerboscript is case-insensitive. Because ThisCase and thisCase are really just thiscase, I don't like trying to make capitalization part of the 'spelling'.

But the way I figure it, I feel the standards only really matter for the publicly callable stuff. Define what library function calls look like (is it the_func() or theFunc() or TheFunc()) because that's where everyone's code style gets mixed together as you call multiple library functions from the same file. But remember the audience here. Telling new programmers just getting their feet wet, "thanks for your contribution, but now I'm going to harp on you for things the language documentation just told you was correct" really puts them off.

akrasuski1 commented 9 years ago

The "cuddled else", as they call it, is something that is used by some, but not all. In particular, Stroustrup, C++ creator, uses the same style, as I do - see: http://www.stroustrup.com/Programming/PPP-style-rev3.pdf on second page.

For me, it is just that else should be visually similar to if, so the following two lines should be similar:

if asd {}
else {}

and not:

if asd{
} else{}

The first one has a keyword as the first line, and the second is preceeded by brace, overstating the brace's importance.

But I'm not going to battle over this small detail, as usually you have just an if statement, and only a fraction of those has else following. In my window library, I have 28 ifs, and 6 elses, giving a ratio of about 20%.

Dunbaratu commented 9 years ago

The fact that this

   if asd{
   } else{}

looks bad is irrelevant since your implication that anyone ever suggested it was dishonest. THIS is what was being said:

   if asd {
   } else {
   }

You invented this alleged difference between how the if and else look, completely out of thin air. BOH the style you use and the one I use both work like so: IF the body is on the same line all together with the if/else, then the brace lineup is irrelevant because it's all run together anyway, like so:

    if asd { stuff }
    else { stuff }

The difference only occurs when you DO split the code up onto its own separate line.

    if asd {
        stuff
    } else {
        stuff
    }
akrasuski1 commented 9 years ago

OK, that was unfair comparison. I changed the code to make the example short, and missed my initial intent. Sorry for that. What I meant was the difference between:

if asd{
    stuff.
}
else {
    stuff.
}

and:

if asd{
    stuff.
} else {
    stuff.
}

First one is 20% (and percentage would be smaller in longer "if" bodies) longer, but I'd say its a fair compromise between readability an too big verbosity.

Dunbaratu commented 9 years ago

Readability is subjective. The second is more readable if you ask me. Your way makes the else look like a brand new language statement starting from scratch on its own when it emphatically isn't. It's most definitely an appendix attached to the original if, which my way communicates more clearly. You can't just cut and paste it and move it somewhere else. It is attached to that if, thus the second style.

The reason proponents of the second style prefer it is because of a preference that this:

   if asd {
   }
   if zxc {
   }

and this:

    if asd {
    }
    else if zxc {
    }

Shouldn't end up looking identical as they do there. The style I prefer shows that an else is a part of the previous if clause, as it actually really is, and not a brand new statement from scratch that could stand on its own.

But to reiterate, this isn't even the right argument to be having in the first place. Rejecting submissions based on this sort of argument sends entirely the wrong message here.

Standards arguments always boil down to people pretending that subjective preferences are fact and are provable, and thus turn people into jerks. This is an argument we shouldn't even be having.

akrasuski1 commented 9 years ago

Yeah, I agree with that.

abenkovskii commented 9 years ago

@akrasuski1 Keep in mind that the internal kOS text editor (command edit) doesn't allow to type tabs. It displays tabs correctly but when you press tab nothing happens.

akrasuski1 commented 9 years ago

Oh, I didn't know about that. I haven't used in-game editor since last year or so.

abenkovskii commented 9 years ago

To sum up:

Dunbaratu commented 9 years ago

I could reluctantly agree that within one file indent style should be consistent so the file looks sane unto itself. That makes sense. But I don't want to see "we won't accept bob's submission unless he redoes his file, because we indent with 3 spaces and he intended with 2" or that sort of thing.

I can agree about not having tabs, because there merely loading the file into an editor and saving it can screw up and make false diffs all over the file even where you didn't type any changes manually.

But to say, "this person indented with this style, so I'll keep doing it their way while I make changes to their code" is something I see as entirely reasonable and respectful to the original author.

This repository isn't quite "the official" library for kOS. If it was then maybe that sort of rigidity would make more sense. It's more open-ended than that. It's really more "show off your interesting code to other people and see if they can build off of it."

Dunbaratu commented 9 years ago

One of my reasons for preferring the same-line style for open braces is that it looks very different from the standard @erendrake enforced in the C# code. As I split my time between the C# of the mod and the Kerboscript of the game, I like having these visual hints that make my brain shift gears and say to itself "this is NOT the C# code. Remember that.". It helps reduce the number of cases where I stick periods at the ends of C# lines, or put semicolons at the end of Kerboscript lines, or try to use '=' for assignment in kerboscript. I want it to look very different, on purpose.

I actually dislike it when people making brand new languages try to make them look just like an existing one because its supposedly "easier" well, it's easier for people who live with both feet in just one world or the other, and make the transition from one to the other without jumping back and forth. But it's a pain when you have to stand with one foot in both worlds for a long time. I spent 5 years in which 50% of my time was programming in a project in C++ and the other 50% was programming in a project in Java. I found the fact that Java was designed deliberately to look like C++ despite being totally new from the ground up and in no way whatsoever based on C++, really really frustrating. It made it hard to force my thinking into two different gears when what I see in front of my face in the editor just looks the same.

dewiniaid commented 8 years ago

If you'd like, I can take a stab at writing something like Python's PEP-0008 coding style guide. I think having such a document open as a PR might facilitate a better discussion, since then people can point to specific parts of it and talk about exceptions/disagreement/etc.

TDW89 commented 8 years ago

@dewiniaid sorry it took a while to respond. If you have the free time and drive to do so go for it. There hasn't been any discussion on this for a while but having something something there that can be discussed and improved / changed could be useful. Almost all of my programming experience has been unstructured / self driven so the ins and outs of style haven't really been something I have focused on. Never the less I will be happy to try and answer anything that comes up and will endeavour to be around / available a bit more.