August 21, 2013 [Issue 10862] Assignment inside if condition still sometimes accepted | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | http://d.puremagic.com/issues/show_bug.cgi?id=10862 --- Comment #10 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-08-20 21:51:59 PDT --- @Andrej Mitrovic Well, while I completely agree that that code is horrible and hard to read, even that's not as bad as running words together, since it least then it's _possible_ to know for sure what the code says. And even if you consider not putting a space after an if to be akin to running words together, it's at most like running two of them together, not a whole sentence, so the impact is very different. But regardless, I don't really want to get into an argument about this. Code formatting style is a very subjective thing, and one person's perfect style could be another person's worst nightmare. If you prefer an extra space after the if, because you think that it improves legibility or for any other reason, that's fine. Format your code however you like. But personally, I don't think that adding the space helps legibility at all or provide any other benefit, and it takes up additional space, so I never do it in my code. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- |
August 21, 2013 [Issue 10862] Assignment inside if condition still sometimes accepted | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | http://d.puremagic.com/issues/show_bug.cgi?id=10862 --- Comment #11 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-08-21 03:48:27 PDT --- (In reply to comment #10) > it takes up additional space, so I never do it in my code. I'm baffled by this, you say it wastes spaces, yet look at how much space you're wasting in std.datetime: void _assertPred(string op, L, R) (L lhs, R rhs, lazy string msg = null, string file = __FILE__, size_t line = __LINE__) if((op == "<" || op == "<=" || op == "==" || op == "!=" || op == ">=" || op == ">") && You've saved one character and wasted 5 lines for a predicate, in a module that reaches 34000 lines. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- |
August 21, 2013 [Issue 10862] Assignment inside if condition still sometimes accepted | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | http://d.puremagic.com/issues/show_bug.cgi?id=10862 monarchdodra@gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |monarchdodra@gmail.com --- Comment #12 from monarchdodra@gmail.com 2013-08-21 05:23:08 PDT --- (In reply to comment #11) > (In reply to comment #10) > > it takes up additional space, so I never do it in my code. > > I'm baffled by this, you say it wastes spaces, yet look at how much space you're wasting in std.datetime: > > void _assertPred(string op, L, R) > (L lhs, R rhs, lazy string msg = null, string file = __FILE__, > size_t line = __LINE__) > if((op == "<" || > op == "<=" || > op == "==" || > op == "!=" || > op == ">=" || > op == ">") && > > You've saved one character and wasted 5 lines for a predicate, in a module that reaches 34000 lines. Vertical alignment FTW! Besides... he saved on *6* characters! if ((op == "<" || op == "<=" || op == "==" || op == "!=" || op == ">=" || op == ">") && I don't care much about spaces before or after operators (except for people who put a space between a function name and its empty paren, eg: "doit ()" :puke:). That said, I'm a real bitch for vertical alignment, as well as clear repeatable instructions per line. There's something about the result that just makes the code flow when looking at it. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- |
August 21, 2013 [Issue 10862] Assignment inside if condition still sometimes accepted | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | http://d.puremagic.com/issues/show_bug.cgi?id=10862 --- Comment #13 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-08-21 06:33:11 PDT --- (In reply to comment #12) > That said, I'm a real bitch for vertical alignment Sure I like that too. (In reply to comment #12) > as well as clear repeatable instructions per line. That sounds like code duplication to me. The constraint could have been: if(op.canFind("<", "<=", "==", "!=", ">=", ">")) { } But then there are other code duplications which are just lazy, such as: if(msg.empty) { throw new AssertError(format(`_assertPred!"%s" failed: Return value of [%s] %s [%s] was [%s] instead of [%s].`, op, origLHSStr, op, rhs, result, expected), file, line); } else { throw new AssertError(format(`_assertPred!"%s" failed: Return value of [%s] %s [%s] was [%s] instead of [%s]: %s`, op, origLHSStr, op, rhs, result, expected, msg), file, line); } Why not reduce this to: string tail = msg.empty ? "." : format(": %s", msg); throw new AssertError(format(`_assertPred!"%s" failed: Return value of [%s] %s [%s] was [%s] instead of [%s]%s`, op, origLHSStr, op, rhs, result, expected, tail), file, line); Boom I saved you 8 lines. Don't tell me this is somehow slowing down performance, the cost of throwing an exception is much higher than allocating a small string. Plus AssertError is typically unrecoverable. All of this duplication adds up, and you end up with the massive std.datetime. Meanwhile someone believes they have to save that one whitespace character in the "if" statement. It's completely absurd. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- |
August 21, 2013 [Issue 10862] Assignment inside if condition still sometimes accepted | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | http://d.puremagic.com/issues/show_bug.cgi?id=10862 --- Comment #14 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-08-21 10:54:30 PDT --- Sure, it's just one space, but I also see no reason whatsoever to have it. It adds zero value IMHO. There are other places where I may think that extra spaces may be worth it, and you may not. I happen to value lining up arguments far more than saving space. I also typically don't mind using extra vertical space, whereas others really don't like it. And I'm not about to claim that the formatting in std.datetime is perfect or couldn't be improved, but whatever I did with it seemed reasonable to me at the time. Different people have different opinions on what improves or harms legibility and what is good and bad formatting. For the most part, it's very subjective. Personally, I happen to see no value in putting a space after the if, whereas you and Andrei like it. I don't think that you can objectively say that one is better than the other. Other people would be arguing that no space should go there but that spaces should be put inside the parens. It's all personal preference, and arguing about it doesn't get us anywhere. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- |
August 21, 2013 [Issue 10862] Assignment inside if condition still sometimes accepted | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | http://d.puremagic.com/issues/show_bug.cgi?id=10862 --- Comment #15 from Andrei Alexandrescu <andrei@erdani.com> 2013-08-21 13:56:41 PDT --- no space after '(' or before ')' - evar -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- |
August 21, 2013 [Issue 10862] Assignment inside if condition still sometimes accepted | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | http://d.puremagic.com/issues/show_bug.cgi?id=10862 --- Comment #16 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-08-21 16:27:09 PDT --- > no space after '(' or before ')' - evar On that, we agree, but it's a subjective thing. I've worked with folks who always do that and much prefer it. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- |
August 21, 2013 [Issue 10862] Assignment inside if condition still sometimes accepted | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | http://d.puremagic.com/issues/show_bug.cgi?id=10862 --- Comment #17 from Andrei Alexandrescu <andrei@erdani.com> 2013-08-21 16:31:26 PDT --- (In reply to comment #16) > > no space after '(' or before ')' - evar > > On that, we agree, but it's a subjective thing. I've worked with folks who always do that and much prefer it. Clearly. There is one argument though - in literary and mathematical use a whitespace can never follow a ')' or precede a ')'. The counter-argument is that program formatting rule does not need to necessarily follow literary or math rules, to which the counter-counter argument is it's better to not invent gratuitous departures, either. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- |
August 22, 2013 [Issue 10862] Assignment inside if condition still sometimes accepted | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | http://d.puremagic.com/issues/show_bug.cgi?id=10862 Henning Pohl <henning@still-hidden.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |diagnostic, pull CC| |henning@still-hidden.de --- Comment #18 from Henning Pohl <henning@still-hidden.de> 2013-08-21 19:10:07 PDT --- Back to topic :] Fix was a bit easy but should work corretly. https://github.com/D-Programming-Language/dmd/pull/2491 -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- |
August 22, 2013 [Issue 10862] Assignment inside if condition still sometimes accepted | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | http://d.puremagic.com/issues/show_bug.cgi?id=10862 --- Comment #19 from github-bugzilla@puremagic.com 2013-08-22 00:07:17 PDT --- Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/34e4acf514eff195b76c614102f8050255f79b36 fix issue 10862 - Assignment inside if condition still sometimes accepted https://github.com/D-Programming-Language/dmd/commit/297a3e0980a6938be52d2f1209fd29aecf96df95 Merge pull request #2491 from hpohl/10862 fix issue 10862 - Assignment inside if condition still sometimes accepted -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- |
Copyright © 1999-2021 by the D Language Foundation