Java-Gaming.org    
Featured games (81)
games approved by the League of Dukes
Games in Showcase (498)
Games in Android Showcase (117)
games submitted by our members
Games in WIP (564)
games currently in development
News: Read the Java Gaming Resources, or peek at the official Java tutorials
 
    Home     Help   Search   Login   Register   
Pages: [1] 2
  ignore  |  Print  
  Which is better?  (Read 6686 times)
0 Members and 1 Guest are viewing this topic.
Offline SwampChicken
« Posted 2010-11-09 05:24:30 »

This?

1  
2  
3  
4  
5  
6  
7  
private Object checkSomething() {
  if(getSomeObject() == null) {
    return something;
  } else {
    return somethingElse;
  }
}


or this?

1  
2  
3  
4  
5  
6  
private Object checkSomething() {
  if(getSomeObject() == null) {
    return something;
  }
  return somethingElse;
}

Offline CyanPrime
« Reply #1 - Posted 2010-11-09 05:59:29 »

bottom one.
Offline zammbi

JGO Coder


Medals: 4



« Reply #2 - Posted 2010-11-09 06:54:30 »

I personally use the bottom one as its cleaner. I believe FindBugs also warns you if your not doing the bottom iirc.

Current project - Rename and Sort
Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline ewjordan

Junior Member





« Reply #3 - Posted 2010-11-09 06:58:28 »

Some people prefer brevity:

1  
2  
3  
private Object checkSomething() {
  return (getSomeObject() == null) ? something : somethingElse;
}


And others get very upset with more than one return statement in a function; they also tend to hate the ternary operator:

1  
2  
3  
4  
5  
6  
7  
private Object checkSomething() {
  Object obj = something;
  if (getSomeObject() != null) {
    obj = somethingElse;
  }
  return obj;
}
Offline princec

JGO Kernel


Medals: 380
Projects: 3
Exp: 16 years


Eh? Who? What? ... Me?


« Reply #4 - Posted 2010-11-09 07:19:12 »

Hm I always liked the top one more. I prefer the symmetry. I like symmetry.

Cas Smiley

Offline oNyx

JGO Coder


Medals: 2


pixels! :x


« Reply #5 - Posted 2010-11-09 07:47:54 »

2nd one. I too prefer symmetry, however, in this case it's more like always doing one thing with some quick exit at the beginning if you get some fishy parameter(s).

Consider this:

1  
2  
3  
4  
5  
6  
7  
private void bar(Baz a) {
    if (a == null) {
        catchFire();
        return;
    }
    [... 1000 lines of code here w/o return ...]
}


I'd rather jump out right away and not have yet another indention level.

弾幕 ☆ @mahonnaiseblog
Offline Matzon

JGO Knight


Medals: 19
Projects: 1


I'm gonna wring your pants!


« Reply #6 - Posted 2010-11-09 07:55:25 »

I'd rather jump out right away and not have yet another indention level.
+1

I usually have a list of conditions for jumping out of a method and then the "normal" flow following.

Offline JL235

JGO Coder


Medals: 10



« Reply #7 - Posted 2010-11-09 09:03:25 »

I prefer the top one too because due to the braces and indentation it's clear that the second return has nothing to do with the first. The structure of the code in the bottom one shows that the second return statement will be executed after the first one (because it follows the if statement). It's laid out like this even though clearly this is untrue.

With the second you also require the knowledge that the statements there specifically are return statements in order to work out that only one of them will ever be reached. You don't need this knowledge with the first example because the control flow points this out already.

Offline cylab

JGO Ninja


Medals: 49



« Reply #8 - Posted 2010-11-09 09:19:08 »

I think thinking about this is pointless. It simply doesn't matter.

Mathias - I Know What [you] Did Last Summer!
Offline princec

JGO Kernel


Medals: 380
Projects: 3
Exp: 16 years


Eh? Who? What? ... Me?


« Reply #9 - Posted 2010-11-09 09:33:30 »

er yeah but that's a completely different bit of code. This is 5 lines. See what I mean?

Cas Smiley

Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline fruitmaze

Senior Member


Medals: 3
Projects: 2



« Reply #10 - Posted 2010-11-09 09:40:40 »

With such a simple function I prefer the bottom one - less code.
Offline JL235

JGO Coder


Medals: 10



« Reply #11 - Posted 2010-11-09 09:46:02 »

I think thinking about this is pointless. It simply doesn't matter.
For something so small, I agree. But I've seen the same style of code stretched across pages. Then this starts to make a difference.

Plus keeping to a common set of coding standards ensures that all sections of code which are structured similarly to the above will all be laid out in the same way. This makes the code patterns more recognisable on an instinctive level.

Offline Jono
« Reply #12 - Posted 2010-11-09 13:08:03 »

I normally work in the same way as Matzon, and here the two bits of code have two different meanings to me.

The first one says, "there are two equally valid cases to this function, and I will return different values depending on which it is".

The second one says "there is an exceptional case when the object is null, and the function behaves differently in this case."
Offline oNyx

JGO Coder


Medals: 2


pixels! :x


« Reply #13 - Posted 2010-11-09 23:25:28 »

er yeah but that's a completely different bit of code. This is 5 lines. See what I mean?

Well, yes. But for symmetry reasons (heh) one should handle both things the same way. Wink

弾幕 ☆ @mahonnaiseblog
Offline Kova

Senior Member





« Reply #14 - Posted 2010-11-09 23:45:41 »

for such small examples, I prefer the 2nd one.. as it's logic is easier read in my mind... if () doSomething, else doOther
Offline princec

JGO Kernel


Medals: 380
Projects: 3
Exp: 16 years


Eh? Who? What? ... Me?


« Reply #15 - Posted 2010-11-10 10:48:45 »

I find that if-else lends itself to describing two equally reasonable outcomes to a function; the latter way I use when the earlier returns are "early exit" conditions.

Cas Smiley

Offline JL235

JGO Coder


Medals: 10



« Reply #16 - Posted 2010-11-14 02:25:48 »

I think what gets more complex tho is when you have code like:
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
public void foo()
{
    if ( someCondition() ) {
        return 1;
    } else {
        if ( !otherCondition() ) {
            return 1;
        } else {
            return 2;
        }
    }
}

Where you have one return happen multiple times and the other return only happen once. Moving the first return out will make the code smaller (and so potentially easier to understand). But that goes against my preference to show the code flow.

Offline CyanPrime
« Reply #17 - Posted 2010-11-14 04:18:04 »

I think what gets more complex tho is when you have code like:
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
public void foo()
{
    if ( someCondition() ) {
        return 1;
    } else {
        if ( !otherCondition() ) {
            return 1;
        } else {
            return 2;
        }
    }
}

Where you have one return happen multiple times and the other return only happen once. Moving the first return out will make the code smaller (and so potentially easier to understand). But that goes against my preference to show the code flow.

why not just say:
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
public void foo()
{
    if ( someCondition() ||  !otherCondition()) {
        return 1;
    }

    else {
         return 2;
    }
}
Offline Eli Delventhal

JGO Kernel


Medals: 42
Projects: 11
Exp: 10 years


Game Engineer


« Reply #18 - Posted 2010-11-14 04:50:36 »

My first computer science professor in college marked off points because I did the second one. Not because of style, but because he said it was incorrect - i.e. it would return TWO values. My jaw dropped and I was close to leaving my university. In the end I had to show the professor through output that there was no way for it to be wrong...

Yeah... Fortunately he got sacked and my other professors were much better.

I have since tried to imagine a programming language where it would split itself into another thread for each return value. It would be the worst possible idea ever, but might be amusing. But yeah, implicitly creating threads is probably one of the dumbest things a language could ever do. It'd be even worse than the nil pointer in ObjC silently ignoring all calls to it...

See my work:
OTC Software
Offline JL235

JGO Coder


Medals: 10



« Reply #19 - Posted 2010-11-14 17:07:05 »

why not just say:
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
public void foo()
{
    if ( someCondition() ||  !otherCondition()) {
        return 1;
    }

    else {
         return 2;
    }
}

The example I gave was based on some PHP code I was writing at the time, and between the two if statements was lots of work (like database access) preventing the conditions from being concatenated together. I skipped that to simplify my example, but I've obviously simplified too far. So how about...
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
public void foo()
{
    if ( someCondition() ) {
        return 1;
    } else {
        // do lots of work
       if ( !otherCondition() ) {
            return 1;
        } else {
            return 2;
        }
    }
}

Offline CyanPrime
« Reply #20 - Posted 2010-11-14 19:11:50 »

The example I gave was based on some PHP code I was writing at the time, and between the two if statements was lots of work (like database access) preventing the conditions from being concatenated together. I skipped that to simplify my example, but I've obviously simplified too far. So how about...
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
public void foo()
{
    if ( someCondition() ) {
        return 1;
    } else {
        // do lots of work
       if ( !otherCondition() ) {
            return 1;
        } else {
            return 2;
        }
    }
}



1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
public void foo()
{
    if ( someCondition()  || !otherCondition() ) {
        if(!otherCondition() ){
            //do other stuff
       }
        return 1;
    }

    else {
        return 2;
    }
}
Offline Abuse

JGO Knight


Medals: 12


falling into the abyss of reality


« Reply #21 - Posted 2010-11-14 20:17:11 »


1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
public void foo()
{
    if ( someCondition()  || !otherCondition() ) {
        if(!otherCondition() ){
            //do other stuff
       }
        return 1;
    }

    else {
        return 2;
    }
}


Ew, checking the same condition twice? I'd never do that.
Especially as you've no idea how expensive evaluating 'otherCondition()' is. (It's returned value might even change, or have side-effects!)

(p.s. has anyone noticed the examples have been returning an int for a void method? Smiley

What do people think of this paradigm:

1  
2  
3  
4  
5  
6  
7  
8  
9  
public int foo()
{
   final int retVal;

   // all the shiz that has been outlined in previous examples....
  // ....but assigning to the final int instead of immediately returning.

   return retVal;
}

Make Elite IV:Dangerous happen! Pledge your backing at KICKSTARTER here! https://dl.dropbox.com/u/54785909/EliteIVsmaller.png
Offline CyanPrime
« Reply #22 - Posted 2010-11-14 20:42:18 »

1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
14  
15  
16  
public void foo()
{

    boolean b = otherCondition()

    if ( someCondition()  || !b) {
        if(!b){
            //do other stuff
       }
        return 1;
    }

    else {
        return 2;
    }
}


Edit: forgot to change it, lol.
Offline Riven
« League of Dukes »

JGO Overlord


Medals: 800
Projects: 4
Exp: 16 years


Hand over your head.


« Reply #23 - Posted 2010-11-14 21:20:35 »

This thread is so funny...

People are trying to 'optimize' a code sample that was created for discussion about indentation, because two branches return the same value, probably by accident.

Nothing to see here, move along people.

Hi, appreciate more people! Σ ♥ = ¾
Learn how to award medals... and work your way up the social rankings
Offline Eli Delventhal

JGO Kernel


Medals: 42
Projects: 11
Exp: 10 years


Game Engineer


« Reply #24 - Posted 2010-11-15 09:21:31 »

What do people think of this paradigm:

1  
2  
3  
4  
5  
6  
7  
8  
9  
public int foo()
{
   final int retVal;

   // all the shiz that has been outlined in previous examples....
  // ....but assigning to the final int instead of immediately returning.

   return retVal;
}


Yeah someone posted that above too.

I worked with someone who I really respected who did that so I started as well, but in the end I decided it wasn't worth it as sometimes it reduces readability (which is more important in my opinion).

The whole idea of kicking out of a method if you have something "invalid" is much clearer than a giant if statement around the actual body of the code; "if my object isn't null and I am allowed to be here right now then" is much more confusing than "if my object is nill, return, if I can't be here now, return" in my opinion.

The idea of having only one return statement is to reduce potential code paths you can do down and therefore reduce bug count, but I've honestly never found a bug related to return values if you keep things readable and small. A random return inside the middle of a massive method, on the other hand, is a bad idea because it's not immediately obvious how one gets there.

See my work:
OTC Software
Offline Abuse

JGO Knight


Medals: 12


falling into the abyss of reality


« Reply #25 - Posted 2010-11-15 12:24:22 »

I was in particular pointing out the use of a final; forcing the code to assign the return value only once, which IMO has a positive knock-on effect for code structure & complexity.

I don't think there is necessarily a 'right', or even a 'better' answer to this question; it very much depends on the circumstances of the method. (size, complexity, control structures).
I even sometimes find labeled breaks to be more readable than the equivalent for/while loop! (as they are self-documenting, often reduce the size of the loop condition, and keep the logic & it's effect together.

However there are definitely lots of wrong answers =)

Make Elite IV:Dangerous happen! Pledge your backing at KICKSTARTER here! https://dl.dropbox.com/u/54785909/EliteIVsmaller.png
Offline Riven
« League of Dukes »

JGO Overlord


Medals: 800
Projects: 4
Exp: 16 years


Hand over your head.


« Reply #26 - Posted 2010-11-15 21:47:33 »

I was in particular pointing out the use of a final;

Just in case you were wondering: I wasn't addressing you with my 'rant'.

Anyway, I feel that much of this is part of your own coding style and your own preferences. I too prefer to keep the indentation to a minimum. Early return is comparable to throwing an exception: it should be done to abort the expected flow as soon as possible. Throwing an exception in the middle of nowhere is just as bad as returning at some 'random' location.

It's all about readability, because if it's readable it means it 'maps to your mind', which means it's easier to comprehend, analyze and debug. I think it shows your mind also aborts incorrect thought patterns as soon as possible, as opposed to building a stack of thoughts. Obviously I base this on absolutely nothing.

Hi, appreciate more people! Σ ♥ = ¾
Learn how to award medals... and work your way up the social rankings
Offline princec

JGO Kernel


Medals: 380
Projects: 3
Exp: 16 years


Eh? Who? What? ... Me?


« Reply #27 - Posted 2010-11-15 23:02:58 »

Whoever wrote it is right.

And if you want it done the right way you do it yourself.

This solves so many dilemmas far beyond computer programming!

Cas Smiley

Offline Captain Awesome

Junior Member


Medals: 2


Hi


« Reply #28 - Posted 2010-11-23 19:52:00 »

I like both, depending on the situation. The second one saves a few bytes though Wink
Offline gouessej
« Reply #29 - Posted 2010-11-26 12:25:34 »

None of them, I prefer this:
1  
2  
3  
4  
5  
6  
7  
8  
9  
private Object checkSomething() {
  final Object result;
  if(getSomeObject() == null) {
    result = something;
  } else {
    result = somethingElse;
  }
  return result;
}

because there is only one return statement and I'm forced to set the variable.

Pages: [1] 2
  ignore  |  Print  
 
 
You cannot reply to this message, because it is very, very old.

 

Add your game by posting it in the WIP section,
or publish it in Showcase.

The first screenshot will be displayed as a thumbnail.

Grunnt (21 views)
2014-09-23 14:38:19

radar3301 (14 views)
2014-09-21 23:33:17

BurntPizza (31 views)
2014-09-21 02:42:18

BurntPizza (22 views)
2014-09-21 01:30:30

moogie (20 views)
2014-09-21 00:26:15

UprightPath (29 views)
2014-09-20 20:14:06

BurntPizza (34 views)
2014-09-19 03:14:18

Dwinin (48 views)
2014-09-12 09:08:26

Norakomi (75 views)
2014-09-10 13:57:51

TehJavaDev (108 views)
2014-09-10 06:39:09
List of Learning Resources
by Longor1996
2014-08-16 10:40:00

List of Learning Resources
by SilverTiger
2014-08-05 19:33:27

Resources for WIP games
by CogWheelz
2014-08-01 16:20:17

Resources for WIP games
by CogWheelz
2014-08-01 16:19:50

List of Learning Resources
by SilverTiger
2014-07-31 16:29:50

List of Learning Resources
by SilverTiger
2014-07-31 16:26:06

List of Learning Resources
by SilverTiger
2014-07-31 11:54:12

HotSpot Options
by dleskov
2014-07-08 01:59:08
java-gaming.org is not responsible for the content posted by its members, including references to external websites, and other references that may or may not have a relation with our primarily gaming and game production oriented community. inquiries and complaints can be sent via email to the info‑account of the company managing the website of java‑gaming.org
Powered by MySQL Powered by PHP Powered by SMF 1.1.18 | SMF © 2013, Simple Machines | Managed by Enhanced Four Valid XHTML 1.0! Valid CSS!