Code Review Stack Exchange

Coding Challenge Coding Live

This was a small coding challenge proposed at my school. The problem is:

By the way, its good that your code doesnt need any comments. This means it is still simple enough to understand it by reading alone.

Mixing humour with horror in fiction

Note: I am looking for any criticism. I would really like to improve my skills in general.

Yeah, I auto-created the Javadoc in Eclipse on an earlier draft, and I forgot to edit that part when posting this. It should just say

Cant Install Any Mac OS on Core 2 Duo Macbook

@JollyJoker It is not the direct problem statement. I meant just the space character. Sorry, I can see how that would be unclear.

You took extra care to avoid unnecessary processing (by checking if the string contains space), and unnecessary allocations (counting the exact storage size needed). But then the trimming kinda defeats the purpose of the rest of the code: in case anything is trimmed, that will involve the allocation of a new string, and a full iteration over the source. To be consistent with the rest of the code, you could have done the trimming yourself.

myReplace( string, target, replacement);

Can a substance be more lethal in smaller doses?

What if you were not allowed to use any of thestr.functions()? Nostr.toCharArray(),str.trim(),str.contains(), and so on? Here is an implementation using the basicStringandStringBuilderfunctionality. If you wanted to you could also replace theStringBuildermethod by concatenating into a newString. (Leaving that part to the reader).

for (char currentCharacter : text.toCharArray() )

block. This can be reversed, and you can do an early return without almost doing any work.

Is the problem statement a direct copy? In particular, do you know if whitespace was intended to mean just the space character? @tommy

I have completed this challenge successfully using the code below. My question is, is there any way to improve the efficiency? I believe the complexity is currently \$O(2n) = O(n)\$ given the 2 for loops. I do not want to use any libraries or functions likestr.replace(). But Im assuming there is a better way than trimming and then counting the whitespace.

Let me give some comments to your code as well, with a refactoring coming out at finally:

This solution still only loops through the originaltextjust once, but divided into three different parts.

I do not want to use any libraries or functions like

Start here for a quick overview of the site

Javadoc commenting looks good (but is not actually true?), otherwise there are zero comments

Can I vs May I in restaurant setting when ordering

How can wait times be higher than clock time?

As Janos mentions, in your code you have most of your code within the

Instead of a counting loop, a for-each loop is more idiomatic when possible. You could createoldArrbefore the counting loop, and use it in the loop that counts spaces

Public Data Release of Stack Overflows 2018 Developer Survey

What is the dimension of the mathematical universe?

public class URL public static String URLify(String text) if (!text.contains( )) return text; // Use urlifiedText for building the result text StringBuilder urlifiedText = new StringBuilder(); // Replace spaces with %20, after trimming leading and trailing spaces for (char currentChar : text.trim().toCharArray()) if (currentChar == ) urlifiedText.append(%20); else urlifiedText.append(currentChar); return urlifiedText.toString(); public static void main(String[] args) System.out.println(URLify( my example text ));

Building a char array, seems to be wasteful

str.trim().replaceAll( , %20)

This site uses cookies to deliver our services and to show you relevant ads and job listings. By using our site, you acknowledge that you have read and understand ourCookie PolicyPrivacy Policy, and ourTerms of Service. Your use of Stack Overflows Products and Services, including the Stack Overflow Network, is subject to these policies and terms.

A possible implementation could then look like:

@holroy One possible nitpick is that whitespace characters other than a normal space wouldnt be replaced.

, but I guess that is kind of cheating?

Instead of the long piece of code inside theif ntains( )) , you could negate the condition for an early return, and make the code flatter, which is a bit easier to read

Following all of this advice, gives code like this:

SincenewArrhas the exact size for the result string, you can use the single-parameter constructor ofString

Hard to tell. If the problem statement is a word-by-word copy Id say its likely the OP didnt realize whitespace could mean something else.

is slightly faster, I think that in this case I would opt for the

public class URL /** * @description URLify ~ A small method that makes a string with spaces URL Friendly! * @param str * @param length * @return String */ public static String URLify(String str) str = str.trim(); int length = str.length(); int trueL = length; ntains( )) for(int i = 0; i length; i++) if(str.charAt(i) == ) trueL = trueL + 2; char[] oldArr = str.toCharArray(); char[] newArr = new char[trueL]; int x = 0; for(int i = 0; i length; i++) if(oldArr[i] == ) newArr[x] = %; newArr[x+1] = 2; newArr[x+2] = 0; x += 3; else newArr[x] = oldArr[i]; x++; str = new String(newArr, 0, trueL); return str; public static void main(String[] args) String str = testing .pdf ; str = URLify(str); System.out.print(str);

loop variant. Simply because it clearer conveys what you are working with. In addition it removes the need for some extra temporary variables like

Some minor technical improvements are possible:

/** * @description URLifyII ~ Remove leading and trailing spaces, * and replace remaining spaces with %20. * NB! Without using str.trim(), str.contains(), and so on… */ public static String URLifyII(String text) int startIndex = 0; int endIndex = text.length() – 1; StringBuilder urlifiedText = new StringBuilder(); // Find first non-space character while (text.charAt(startIndex) == && startIndex endIndex) startIndex++; // Find last non-space character while (text.charAt(endIndex) == && endIndex = startIndex) endIndex–; // Repeat text, and replace spaces with %20 for (int i=startIndex; i = endIndex; i++) if (text.charAt(i) != ) urlifiedText.append(text.charAt(i)); else urlifiedText.append(%20); return urlifiedText.toString();

Discuss the workings and policies of this site

Include a little more vertical space

You could declarexin the initializer of the secondforloop, to limit its scope to the loops body

What are the eigenvalues of the given matrix A?

How do hosting providers prevent the compromise of one website from causing the compromise of another one?

blocks can greatly enhance the reading and understanding of code.

Can you raise your original body as an undead after moving to a clone?

Would a demon be afforded civil rights?

, then you should create a method for it, so that you can call

? I know when reinventing the wheel, there could be some debate as to what you are allowed to use or not. But youre already using

oldArrandnewArr? In general I dont think variable names includingarray,arr,table, and so on are good names. In most cases you know its an array due to the presence of the array indexing braces. MaybeoriginalCharsorstrChars, could be better names.

The variable names should follow a pattern. That is, all variables for the input string should haveinin the name, and all output variables should haveout. Or at leastiando.

got a String, and only if nothing is provided would I use the hard coded string, that would make it easier to show and test your design

site design / logo 2018 Stack Exchange Inc; user contributions licensed undercc by-sa 3.0withattribution required.rev2018.5.31.30593

Since you only uselengthandtrueLwhen you know the string contains a space, it would be better to declare those variables in the scope of theifblock

Try to avoid multiple repeated loops In your code you loop it once to get the new length into the strangly namedtrueL, this should (if kept) be namednewLengthor similar. Repeating loops is considered expensive, and should be avoided, if possible.

A personal nitpick of mine is to add a little more vertical space in the code. Adding an extra newline in front of

Sign uporlog into customize your list.

Learn more about hiring developers or posting ads with us

You should have told uswhyyou dont want to use existing libraries because when using libraries, the problem becomes trivial:return in.trim().replace( , %20).

Why do dictators ban their people from traveling?

Most of the Javadoc is redundant and can be removed. What remains is Makes a string with spaces URL-friendly.

@JollyJoker, Albeit a good point in the general case, in this particular case I think the requirements said the space character. Not space, tab and/or newline characters.

Does VS Code SFDC IDE require Salesforce DX?

How to politely write I work best when working alone in a CV

For example, inputMr John Smithwould returnMr%20John%20Smith.

Criticism and disadvantages of dependency injection

How to investigate a bug of mandatory fields not being entered?

Given a string (or URL), replace the white-spaces inside of the string with%20, but any spaces on the outside should not be included.

Instead of computing the strangely namedtrueL, you could count the number of spaces, and store it in the naturally namedspaces

is ok? 50% of the requirement is handled by a library function. Tip: be consistent.

Is it reasonable to ask a potential employer for samples of their code and/or employee contact information, to determine if I want to work there?

Detailed answers to any questions you might have

How to avoid explaining science to strangers

? Correct? Also, what do you mean by the second bullet point? How could I test that? Sorry if thats a dumb question.

I am pretty you can merge those 2 loops into one if you used astringbuilder. On the whole the replace algorithm could use work

Caught in the rain without rainwear or raingear

How can you minimize taxes on cryptocurrency trading?

Stack Exchange network consists of 174 Q&A communities includingStack Overflow, the largest, most trusted online community for developers to learn, share their knowledge, and build their careers.

By clicking Post Your Answer, you acknowledge that you have read our updatedterms of serviceprivacy policyandcookie policy, and that your continued use of the website is subject to these policies.

Even with some added comments, it still is slightly shorter than your original code, and hopefully understandable.

Learn more about Stack Overflow the company

public static String urlencode(String str) str = str.trim(); if (!str.contains( )) return str; char[] chars = str.toCharArray(); int spaces = 0; for (char c : chars) if (c == ) spaces++; char[] newArr = new char[chars.length + 2 * spaces]; for (int i = 0, x = 0; i chars.length; i++) char c = chars[i]; if (c == ) newArr[x] = %; newArr[x + 1] = 2; newArr[x + 2] = 0; x += 3; else newArr[x] = c; x++; return new String(newArr);

The next simplest solution not countingstr.replace(…)would have been to use aStringBuilder: for each space append%20, otherwise append the character itself. But I guess you didnt want to do that.

Id useCharacter.isWhitespaceinstead of

If you run your program on the command line, then you can pass a string as a parameter to the program. That parameter will be passed to you in the

I like that you calculate then length just once, and use that in the

Leave a Reply