Go Premium for a chance to win a PS4. Enter to Win

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 182
  • Last Modified:

seeColor challenge

Hi,

I am working on below challenge
http://codingbat.com/prob/p199216

I wrote my code as below
public String seeColor(String str) {
if(str.indexOf("red",0)==1 && str.indexOf("red",1)==1)
return "red";
if(str.indexOf("blue",0)==1 && str.indexOf("blue",1)==1)
return "blue";
else
return "";
  
}

Open in new window


I am failing as below
Expected	Run		
seeColor("redxx") → "red"	""	X	    
seeColor("xxred") → ""	""	OK	    
seeColor("blueTimes") → "blue"	""	X	    
seeColor("NoColor") → ""	""	OK	    
seeColor("red") → "red"	""	X	    
seeColor("re") → ""	""	OK	    
seeColor("blu") → ""	""	OK	    
seeColor("blue") → "blue"	""	X	    
seeColor("a") → ""	""	OK	    
seeColor("") → ""	""	OK	    
seeColor("xyzred") → ""	""	OK	    
other tests
X	   

Open in new window


How to fix and improve my code. Thanks in advance
0
gudii9
Asked:
gudii9
  • 16
  • 12
  • 11
  • +2
4 Solutions
 
CPColinCommented:
Try using startsWith() instead of indexOf().
0
 
ozoCommented:
Notice that you are always returning ""
Is there any input for which your code would return anything other than ""?
0
 
krakatoaCommented:
Not sure why you test for the two colour strings at offset 0 and offset 1 - maybe I'm drowsy - been up all night.

But as I wouldn't recommend my code, perhaps as an additional exercise, you'd like to tell us how and why this code works :
return str.indexOf("red")==0||str.indexOf("blue")==0?str.charAt(2)=='d'?"red":str.charAt(3)=='e'?"blue":"":"";

Open in new window

0
Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
awking00Commented:
A common theme in teaching/learning Java is that of "baby steps" (i.e. get one piece of code at a time to work). Testing each piece of code can often be done by printing to the console. Assume your string, str, was simply "red", System.out.println(str.indexOf("red",0)) would produce a value of 0, so your condition of str.indexOf("red",0) == 1 will always be false. So you found out you need to change your condition to == 0, which represents the beginning anyway. You also found out that the second condition of str.indexOf("red",1) == 1 is not needed at all. You can then begin testing by adding and/or subtracting characters before and after "red" and printing the results to the console. Once you have gone through all of the possibilities for "red" you can then easily deal with the "blue" requirement and the empty string requirement where strings not beginning with red or blue are found.
0
 
gudii9Author Commented:
public String seeColor(String str) {
if(str.startsWith("red"))
return "red";
if(str.startsWith("blue"))
return "blue";
else
return "";
  
}

Open in new window


when i used startsWith() as above i passed all tests. How can i improve my code. Please advise
0
 
awking00Commented:
Not much room for improvement. I guess you could nest the ternary operator to shorten the code, but I doubt if there would be any noticeable in performance
return str.startsWith("red") ? "red" : str.startsWith("blue") ? "blue" : "";
0
 
CPColinCommented:
I recommend not making the code harder to read unless there's a tangible benefit. In this case, that line would almost certainly produce the exact same performance as the separate if statements, so condensing the syntax like that is probably not a good idea.
0
 
gudii9Author Commented:
return str.indexOf("red")==0||str.indexOf("blue")==0?str.charAt(2)=='d'?"red":str.charAt(3)=='e'?"blue":"":"";

how and where to break above and below line to understand

return str.startsWith("red") ? "red" : str.startsWith("blue") ? "blue" : "";

simple syntax is as below right
   //
        // Get the maximum value
        //
        int min = a < b ? a : b;

How to relate with this syntax above two quoted  lines. Please advise
0
 
krakatoaCommented:
Did you try the code I posted? If, as a programmer, you intend looking at code (someone else's) in the hope of understanding it, then trying to explain what my code does would prove helpful to you in the long run. Can you see why it works?
0
 
gudii9Author Commented:
return str.startsWith("red") ? "red" : str.startsWith("blue") ? "blue" : "";

i think i understood above one now

if str.startsWith("red") is true then return as red
if str.startsWith("red") is false then go to inner ternary operator loop str.startsWith("blue") ? "blue" : ""


str.startsWith("blue") ? "blue" : "" works similar way
if str.startsWith("blue") is true then return as blue
if str.startsWith("blue") is false then return as ""

Did you try the code I posted?
yes. i tried and it is passing all tests.
I am not able to understand below. please advise
return str.indexOf("red")==0||str.indexOf("blue")==0?str.charAt(2)=='d'?"red":str.charAt(3)=='e'?"blue":"":"";
0
 
ozoCommented:
return str.indexOf("red")==0||str.indexOf("blue")==0?str.charAt(2)=='d'?"red":str.charAt(3)=='e'?"blue":"":""

Open in new window

is equivalent to
String retval;
Boolean t1=false;
if( str.indexOf("red")==0 ){
    t1=true;
}else if( str.indexOf("blue")==0 ){
    t1=true;
}
if( t1 ){
    if( str.charAt(2)=='d' ){
        retval="red";
    }else{
        if( str.charAt(3)=='e' ){
            retval="blue";
        }else{
            retval="";  // can never get here                                                                                                                                     
        }
    }
}else{
    retval="";
}
return retval;

Open in new window

which strikes me as unduely complicated compared to
if( str.indexOf("red")==0 ){
  retval="red";
}else if( str.indexOf("blue")==0 ){
  retval="blue";
}else{
  retval="";
}

Open in new window

0
 
gudii9Author Commented:
return str.indexOf("red")==0||str.indexOf("blue")==0?str.charAt(2)=='d'?"red":str.charAt(3)=='e'?"blue":"":""

as bolded is the true part of the ternary operator right
0
 
gudii9Author Commented:
how to break down similar to how you did when there are multiple ternary operations within one. please advise
0
 
ozoCommented:
The bolded part corresponds to
    if( str.charAt(2)=='d' ){
        retval="red";
    }else{
        if( str.charAt(3)=='e' ){
            retval="blue";
        }else{
            retval="";                                          
        }
    }
0
 
gudii9Author Commented:
else{
        if( str.charAt(3)=='e' ){
            retval="blue";
        }else{
            retval="";                                          
        }

Open in new window


why else block used why not one other if block. Please advise
0
 
ozoCommented:
: corresponds to else
0
 
gudii9Author Commented:
? is if right
0
 
ozoCommented:
more like the ){ but yes.
0
 
krakatoaCommented:
I don't agree with ozo's thinking this time that the equivalent of my code is a longwinded rendering like he has given. In code, you can't say that "one thing is like another", as it's all to do with clock cycles, and you can't compare different function calls like that in such cases.

What I am getting at gudii, is that I think your "problems" in these coding exercises are not so much to do with coding, but with understanding the question in plain language in the first place. That is why I am asking you if you can decipher what my code really means, on a "philosophical" level. If you feel you can't or don't want to do this, then simply say so. And if you are interested in how it works and why, then I will gladly explain. The point of my explanation would be to help you with understanding the problem in the first place, and not with the code as such, which will all fall into place for you I think once you really understand what's going on.
0
 
gudii9Author Commented:
What I am getting at gudii, is that I think your "problems" in these coding exercises are not so much to do with coding, but with understanding the question in plain language in the first place. That is why I am asking you if you can decipher what my code really means, on a "philosophical" level. If you feel you can't or don't want to do this, then simply say so.

this challenge i understood which happened to return red or blue if the given string starts with red or blue. I understood few approaches as well.  The approach you mentioned looks bit challenging to me to understand may be due to my lack understanding on bigger multiple ternary loops.
0
 
gudii9Author Commented:
more like the ){ but yes.
can you please elaborate on this?
0
 
krakatoaCommented:
The approach you mentioned looks bit challenging to me to understand may be due to my lack understanding on bigger multiple ternary loops.

Ternary is not complicated. It's like an if-else, but the difference with a ternary is that each time you use it you MUST give an else (which in ternary is the part that comes after the ":" operator).

But anyway, back to the pathology of how to deal with red, blue, "", and other strings (that we don't want).

My ternary is simple - by asking whether "red" OR "blue" is at position 0 of the string WE IMMEDIATELY EXCLUDE ANY String that ISN'T RED or BLUE. And that INCLUDES "". Think about that - when the ternary checks for red or blue at index 0, and finds one of them, then that can only mean that one of them is there - not "daffodil" or "elephant" or "green" or "purple" or even "" ! If the ternary test is passed, then ONLY red or blue can be there. So knowing that, you then just need another ternary (nested) to test whether it is red or blue.

(The only thing you need to remember when doing something like that, is that if they wanted you to test for the two colours "red" and "ruby", you would need to choose any other character than the ones at position 0 in each substring, otherwise you get a false positive returned. So in this case, you'd test whether e,u,b,d or y was at one of the other positions.
0
 
awking00Commented:
Actually, once you know the string must begin with either red or blue, you could test for "r" (or "b") at index 0 to confirm which.
0
 
krakatoaCommented:
Yes, of course you can do that. But I chose different indices to check, so that the questioner might ask "why did you choose those positions", and so understand more completely what is going on.
0
 
awking00Commented:
I agree with the idea of choosing indices for the very purpose you state (why choose those?). The answer to why is basically, once you know the string begins with only "red" or "blue", what test can be made to verify which one. It's kind of like the old balance scale puzzle where once you determine which coin (or marble, etc.) is the odd one, it's one more small step to determine if it's heavier or lighter. In fact all of the following options would confirm if the string was red or blue"
"r" at index 0
"re" at index 0
"red" at index 0
"e" at index 1
"ed" at index 1
"d" at index 2
"b" at index 0
"bl" at index 0
"blu" at index 0
"blue" at index 0
"l" at index 1
"lu" at index 1
"lue" at index 1
"u" at index 2
"ue" at index 2
"e" at index 3
0
 
krakatoaCommented:
Yes, it's obvious that you can do all of those. But you can only do the ones that are single characters using charAt(), otherwise you'd need substring again and I wanted to avoid that.
0
 
ozoCommented:
str.indexOf("red")==0||str.indexOf("blue")==0?str.charAt(2)=='d'?"red":str.charAt(3)=='e'?"blue":"":""
You test for red,
You test for blue
then you merge the results of those two tests together,
so you now need to re-separate them,
so you test for red again
and then test for blue again.

If you're going to go through all that, why not
  String[]redblue={"red","blue"};
  return str.indexOf("red")==0||str.indexOf("blue")==0?
  redblue[str.indexOf('e')/3]
  :"";
0
 
krakatoaCommented:
You test for red,
 You test for blue

Yes . . . but I don't test for "", that's half the point, saves a whole quiz. The other half of the point is that with the OR you don't need another if. And I don't introduce the new complication of a String[].

And how would it work if you were looking for 'red' and 'tan'?
0
 
ozoCommented:
If I test for red and it is true, I then know immediately that the return value should be "red"
I don't need to test for blue, combine that test with the test for red, and then test for red again then test for blue again.
If the test for red was false, and I don't return "red", then I now only need to test for "blue"
0
 
krakatoaCommented:
You'll have to explain to me again how your algo would work if you were asked to search for 'red' and 'tan' - two colours which share no common character.
0
 
ozoCommented:
return str.indexOf("red")==0?
               "red"
            :  str.indexOf("tan")==0?
               "tan"
               :
               "";
0
 
CPColinCommented:
ozo and krakatoa,

Why make things more complicated than just using startsWith()? The code in this comment is passing all tests. Isn't that good enough?

Why also muddy the syntax by using the ternary operator? The if-else blocks are much easier to read and accomplish the same goal.
0
 
ozoCommented:
I was not suggesting http:#a40510146 as an alternative to http:#a40507397
I was suggesting it as an alternative to http:#a40503348, which was the subject of the current discussion.
0
 
CPColinCommented:
In that case, could the two of you please take the discussion elsewhere? I don't think it's helping gudii9 get any closer to a solution.
0
 
krakatoaCommented:
@CPColin

I agree completely. My agenda was only to help the OP expand the thinking a little. As I said in a comment, seems his difficulty is in the profile of the questions themselves, and my ternary was simply another way of looking at what coding challenge asks be returned.

@ozo -

ozo2014-12-19 at 21:56:32ID: 40510146

You just changed your model. You were talking about a String[] and now you've dropped that idea. I don't see your String[] model - as it stands - working at all under the conditions I mentioned. Changing your plea to two distinct return statements is chalk compared to your earlier cheese.
0
 
ozoCommented:
gudii9 has a solution, but since then asked a question about http:#a40503348
0
 
krakatoaCommented:
gudii9 has a solution, but since then asked a question about http:#a40503348 

Right, well then why not let gudii ask about the code I posted, instead of going off on another idea, that you linked to mine. It's fair enough if you bring in a new idea unilaterally, but linking it erroneously to what I posted is unhelpful, and misleading for the OP.
0
 
awking00Commented:
>>But you can only do the ones that are single characters using charAt(), otherwise you'd need substring again and I wanted to avoid that<<
No need for substring. Instead of using charAt(), you could use indexOf() -
e.g. str.indexOf("ed") = 1 or str.indexOf("ue") = 2
0
 
krakatoaCommented:
Yes, I know. But you didn't say that in your earlier post. And seeing as the OP is the one with the understanding requirement here, it should have been clarified then.
0
 
awking00Commented:
I agree. I should have been much more explicit. :-(
0
 
gudii9Author Commented:
i just all the posts now. I think i need to read and re read few times to understand all above alternate approaches which all looks great.
0
 
gudii9Author Commented:
return str.indexOf("red")==0||str.indexOf("blue")==0?str.charAt(2)=='d'?"red":str.charAt(3)=='e'?"blue":"":""

Open in new window



above is same as below right

String retval;
Boolean t1=false;
if( str.indexOf("red")==0 ){
    t1=true;
}else if( str.indexOf("blue")==0 ){
    t1=true;
}
if( t1 ){
    if( str.charAt(2)=='d' ){
        retval="red";
    }else{
        if( str.charAt(3)=='e' ){
            retval="blue";
        }else{
            retval="";  // can never get here                                                                                                                                     
        }
    }
}else{
    retval="";
}
return retval;

Open in new window

0
 
gudii9Author Commented:
public String seeColor(String str) {
String retval;
Boolean t1=false;
if( str.indexOf("red")==0 ){
    t1=true;
}else if( str.indexOf("blue")==0 ){
    t1=true;
}
if( t1 ){
    if( str.charAt(2)=='d' ){
        retval="red";
    }else{
        if( str.charAt(3)=='e' ){
            retval="blue";
        }else{
            retval="";  // can never get here                                                                                                                                     
        }
    }
}else{
    retval="";
}
return retval;

}

Open in new window


above fails for input rxd and bxxxe(replacing middle characters with x in red and blue)
please advise
0
 
awking00Commented:
>>above fails for input rxd and bxxxe(replacing middle characters with x in red and blue)<<
What gets returned in those cases?
0
 
gudii9Author Commented:
public class test40 {

	/**
	 * @param args
	 */
	public static void main(String[] args) {
		// TODO Auto-generated method stub
		
		System.out.println("seeColor red is----"+seeColor("rxd"));
		System.out.println("seeColor blue----"+seeColor("bxxxe"));
	}
	
	public static String seeColor(String str) {
		String retval;
		Boolean t1=false;
		if( str.indexOf("red")==0 ){
		    t1=true;
		}else if( str.indexOf("blue")==0 ){
		    t1=true;
		}
		if( t1 ){
		    if( str.charAt(2)=='d' ){
		        retval="red";
		    }else{
		        if( str.charAt(3)=='e' ){
		            retval="blue";
		        }else{
		            retval="";  // can never get here                                                                                                                                     
		        }
		    }
		}else{
		    retval="";
		}
		return retval;

		}

}

Open in new window

when i run like above not getting anything as output as below

seeColor red is----
seeColor blue----

please advise
0
 
awking00Commented:
Isn't that what you would expect? Since "rxd" is not "red" and "bxxxe" is not "blue", the seeColor function is returning the empty string ("").
0
 
ozoCommented:
It looks like http:#a40533258 correctly returns "" for  input rxd and bxxxe
0
 
gudii9Author Commented:
oh i see since t1 is false it will return "" from below else block right? So the code is doing what it supposed to do then.

else{
    retval="";

please advise
0
 
gudii9Author Commented:
public class test40 {

	/**
	 * @param args
	 */
	public static void main(String[] args) {
		// TODO Auto-generated method stub
		
		System.out.println("seeColor red is----"+seeColor("rxd"));
		System.out.println("seeColor blue----"+seeColor("bxxxe"));
	}
	
	public static String seeColor(String str) {
		String retval;
		Boolean t1=false;
		if( str.indexOf("red")==0 ){
		    t1=true;
		}else if( str.indexOf("blue")==0 ){
		    t1=true;
		}
		if( t1 ){
		    if( str.charAt(2)=='d' ){
		        retval="red";
		    }else{
		        if( str.charAt(3)=='e' ){
		            retval="blue";
		        }else{
		            retval="";  // can never get here                                                                                                                                     
		        }
		    }
		}else{
		    retval="aaa";
		}
		return retval;

		}

}

Open in new window


now i got
seeColor red is----aaa
seeColor blue----aaa


i change to aaa instead of "" to confirm and it is indeed giving back aaa
0
 
ozoCommented:
           if( str.indexOf("red")==0 ){
At this point you already know that the function should return "red"; and there is no further need to check str.charAt(2) or str.charAt(3).
...unless you then proceed to mix up this condition with another condition, only to have to separate them again.
0
 
gudii9Author Commented:
no further need to check str.charAt(2) or str.charAt(3).

Above make sense
0

Featured Post

What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

  • 16
  • 12
  • 11
  • +2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now