Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Development

Reply
 
Thread Tools Display Modes
Old April 26, 2016, 03:03   #31
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
Quote:
Originally Posted by takkaria View Post
Re braces after case & mixed declarations and code, the reason I disagreed was because (and this seems uncontroversial) it's a principle that variable declarations should be as close to the code that uses them as possible. I guess we're disagreeing on the extent to which that takes precedence over other principles.
The guidelines do state that variables should be declared as locally as possible, so variables declared inside for loops, do/while blocks, case blocks, etc. are encouraged.

Quote:
For case statements I don't see a problem with them sometimes having braces and sometimes not, depending on whether variables are declared there, and if there are variables, it's easier to refactor later (e.g. by splitting the contents of the case statement to another function).
The big flaw in not using braces has been pointed out, so I agree that case statements should have braces exactly for the purpose of declaring variables locally to assist in refactoring.

Quote:
For mixed declarations and code, I guess I've never had any problem finding variables in a function, but I've repeatedly had situations where I've done something like:

Code:
int x;
assert(y != NULL);
x = y->property;
when it would have been much neater to start the function with asserts and then make the declaration of x and its value assignment on the same line.
Guidelines are, after all, guidelines... there comes a point where you need to ignore them so that you code is more readable (string constants going over 80 columns for example). I have no issue with parameter asserts being at the very start of the function, particularly when you want to do the this:
Code:
int my_function(struct object *obj)
{
    assert(obj);

    int x = obj->x;
    int y = obj->y;
}
which is cleaner and shorter than this:
Code:
int my_function(struct object *obj)
{
    int x, y;

    assert(obj);

    x = obj->x;
    y = obj->y;
}
I also don't mind them being immediately after variable declarations


Quote:
And If anything, I think putting variable declarations at the latest possible point for them makes the code less error-prone and easier to refactor, since the stuff you want to refactor is all grouped together.
Typically we refactor blocks of code, like functionising the internals of a loop. The local variables will be declared at the start of that block of code and should come out nice and cleanly.

I, personally, find it very messy when there are variable declarations spread semi-randomly through the code. I find my train of though works better when I know upfront what types of data the following code will be dealing with - like

"OK, this block of code will be messing around with a foo - I haven't seen a foo for a while, I'll quickly check foo.h - yep, got it. Now let's look at the code"

Versus

"OK yep, yep, got it, um, bugger, what's a foo look like, better check foo.h. Right, got it, now, what was this code doing, bugger, better go back to the start"

Quote:
The rest, I'm easy about, and I can see your points.
That would be multi-assignments and double blanks

Last edited by calris; April 26, 2016 at 08:47.
calris is offline   Reply With Quote
Old April 26, 2016, 03:16   #32
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
Quote:
Originally Posted by the Invisible Stalker View Post
Personally I would quite happily write
Code:
    if(test_1) i++; else if(test_2) j++; else k++;
but I'm aware that would probably result in a mob outside my door with pitchforks and torches.
Why yes, I do believe it will

Quote:
While I'm saying things most people will disagree with, I'd prefer far fewer comments in the code.
I think this is a far less contentious point. Good, clean, readable code doesn't need comment littered throughout it - it it actually decreases readability.

Quote:
Angband code, like all code that has been around for ages, is full of examples where the comments no long correspond to the code. So I need to read the code anyway to determine what's really happening.
I said it earlier - If the comments don't match the code, they are probably both wrong

Quote:
But in that case what are the comments meant to be doing for me? Letting me know what some former maintainer intended the code, as it was then, to do? I honestly can't remember a case where I found that helpful.
obj-desc.c, obj_desc_name_format() - there are couple of nice comments which help by pointing out what the next section of the function does (relating the complex nature of pluralisation in English) - I think this is a good example of commenting done right
calris is offline   Reply With Quote
Old April 28, 2016, 02:50   #33
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
I've push the following changes to the coding guidlines.
  • case statements now need braces
  • assert() calls to check function parameters can be placed before variable declarations
calris is offline   Reply With Quote
Old April 28, 2016, 12:10   #34
the Invisible Stalker
Adept
 
Join Date: Jul 2009
Posts: 164
the Invisible Stalker is on a distinguished road
I think we may be looking at this the wrong way. The Linux coding standards are simply a more detailed version of the statement "If Linus doesn't like it then it's wrong". I'd suggest replacing "Linus" with "Nick" in that sentence, since he's the current maintainer, and deducing all the details from that premise.
the Invisible Stalker is offline   Reply With Quote
Old April 28, 2016, 12:17   #35
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
Quote:
Originally Posted by the Invisible Stalker View Post
I think we may be looking at this the wrong way. The Linux coding standards are simply a more detailed version of the statement "If Linus doesn't like it then it's wrong". I'd suggest replacing "Linus" with "Nick" in that sentence, since he's the current maintainer, and deducing all the details from that premise.
Nick has already distanced himself from the argument.

Nick is smart.

Be like Nick.
calris is offline   Reply With Quote
Old April 28, 2016, 12:44   #36
the Invisible Stalker
Adept
 
Join Date: Jul 2009
Posts: 164
the Invisible Stalker is on a distinguished road
There are probably issues where Nick has no preference one way or the other, but needs things to be consistent. Indentation might well be an example of this. I don't know if Nick really cares whether we use four or eight spaces, but I doubt he we would want to read code with a mixture of the two. Most of the issues being discussed here don't seem to me to belong to that category. They mostly seem like issues where not caring one way or the other seems eminently sensible, and where I'd be quite happy to see that indifference reflected in the coding standards.
the Invisible Stalker is offline   Reply With Quote
Old April 28, 2016, 16:45   #37
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
Most of the guidelines don't really matter one way or the other, but the important thing is consistency. I've done a _little_ bit of Linux coding and fair amount of U-Boot coding (which follows the Linux coding standards) and coding for those two code bases is vastly superior to Angband - The code you start with is consistent, the code you end with is consistent, and the patches that lead from A to B are clean.

Some of the arguments I'm putting forward apply as much (if not more) to the patches as to the actual code. Once you get the code to a state of near-perfect consistency, you start to look at patches and think 'no, that's not right - there's a bug right there' without even applying the patch to the code.
calris is offline   Reply With Quote
Old April 30, 2016, 08:08   #38
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
Does anyone have any more to say on the guidelines? I would really like to start getting stuck into cleaning up the code, but there's no real point in starting until we agree with the guidelines and Nick pulls them into mainline
calris is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
Sil: 'forge' key in Angband-style keyset? Therem Harth Sil 3 October 24, 2012 18:29
Skyrim, angband style BlackKite Idle chatter 8 June 27, 2012 03:28
some more questions about coding Malak Darkhunter Vanilla 16 January 3, 2012 05:52
more variant coding woes will_asher Variants 1 February 10, 2011 06:18
Angband coding question will_asher Variants 10 September 3, 2008 23:03


All times are GMT +1. The time now is 18:03.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2019, vBulletin Solutions Inc.