Calculate the frequency of characters in a string
$begingroup$
I wrote this program to check the number of times that each letter appears in a string input by the user. It works fine, but is there a more efficient or alternative solution of going about this task than reiterating through a twenty-six-element-long array for every single character?
import java.util.Scanner;
public class Letters {
public static void main(String args) {
@SuppressWarnings("resource")
Scanner sc = new Scanner(System.in);
char c = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'};
int f = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
System.out.println("Enter a string.");
String k = sc.nextLine();
String s = k.toUpperCase();
s = s.trim();
int l = s.length();
System.out.println("Checking string = " + s);
char ch;
for (int i = 0; i < l; i++) {
ch = s.charAt(i);
for (int j = 0; j < c.length; j++) {
if (ch == c[j]) {
f[j]++;
}
}
}
System.out.println("ChartFreq");
for (int i = 0; i < c.length; i++) {
if (f[i] != 0) {
System.out.println(c[i] + "t" + f[i]);
}
}
}
}
java strings
New contributor
$endgroup$
add a comment |
$begingroup$
I wrote this program to check the number of times that each letter appears in a string input by the user. It works fine, but is there a more efficient or alternative solution of going about this task than reiterating through a twenty-six-element-long array for every single character?
import java.util.Scanner;
public class Letters {
public static void main(String args) {
@SuppressWarnings("resource")
Scanner sc = new Scanner(System.in);
char c = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'};
int f = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
System.out.println("Enter a string.");
String k = sc.nextLine();
String s = k.toUpperCase();
s = s.trim();
int l = s.length();
System.out.println("Checking string = " + s);
char ch;
for (int i = 0; i < l; i++) {
ch = s.charAt(i);
for (int j = 0; j < c.length; j++) {
if (ch == c[j]) {
f[j]++;
}
}
}
System.out.println("ChartFreq");
for (int i = 0; i < c.length; i++) {
if (f[i] != 0) {
System.out.println(c[i] + "t" + f[i]);
}
}
}
}
java strings
New contributor
$endgroup$
1
$begingroup$
Hi Artemis! Welcome! This is a great question! I'm sure someone would help you with that!
$endgroup$
– Emma
yesterday
add a comment |
$begingroup$
I wrote this program to check the number of times that each letter appears in a string input by the user. It works fine, but is there a more efficient or alternative solution of going about this task than reiterating through a twenty-six-element-long array for every single character?
import java.util.Scanner;
public class Letters {
public static void main(String args) {
@SuppressWarnings("resource")
Scanner sc = new Scanner(System.in);
char c = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'};
int f = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
System.out.println("Enter a string.");
String k = sc.nextLine();
String s = k.toUpperCase();
s = s.trim();
int l = s.length();
System.out.println("Checking string = " + s);
char ch;
for (int i = 0; i < l; i++) {
ch = s.charAt(i);
for (int j = 0; j < c.length; j++) {
if (ch == c[j]) {
f[j]++;
}
}
}
System.out.println("ChartFreq");
for (int i = 0; i < c.length; i++) {
if (f[i] != 0) {
System.out.println(c[i] + "t" + f[i]);
}
}
}
}
java strings
New contributor
$endgroup$
I wrote this program to check the number of times that each letter appears in a string input by the user. It works fine, but is there a more efficient or alternative solution of going about this task than reiterating through a twenty-six-element-long array for every single character?
import java.util.Scanner;
public class Letters {
public static void main(String args) {
@SuppressWarnings("resource")
Scanner sc = new Scanner(System.in);
char c = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'};
int f = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
System.out.println("Enter a string.");
String k = sc.nextLine();
String s = k.toUpperCase();
s = s.trim();
int l = s.length();
System.out.println("Checking string = " + s);
char ch;
for (int i = 0; i < l; i++) {
ch = s.charAt(i);
for (int j = 0; j < c.length; j++) {
if (ch == c[j]) {
f[j]++;
}
}
}
System.out.println("ChartFreq");
for (int i = 0; i < c.length; i++) {
if (f[i] != 0) {
System.out.println(c[i] + "t" + f[i]);
}
}
}
}
java strings
java strings
New contributor
New contributor
edited yesterday
Artemis Hunter
New contributor
asked yesterday
Artemis HunterArtemis Hunter
586
586
New contributor
New contributor
1
$begingroup$
Hi Artemis! Welcome! This is a great question! I'm sure someone would help you with that!
$endgroup$
– Emma
yesterday
add a comment |
1
$begingroup$
Hi Artemis! Welcome! This is a great question! I'm sure someone would help you with that!
$endgroup$
– Emma
yesterday
1
1
$begingroup$
Hi Artemis! Welcome! This is a great question! I'm sure someone would help you with that!
$endgroup$
– Emma
yesterday
$begingroup$
Hi Artemis! Welcome! This is a great question! I'm sure someone would help you with that!
$endgroup$
– Emma
yesterday
add a comment |
3 Answers
3
active
oldest
votes
$begingroup$
Separate logical elements
It's good to separate different logical parts of a program, for example:
- Parse input: a function that takes an
InputStream
and returns aString
- Compute frequencies: a function that takes a
String
and returns frequencies in some form. In your current program you used anint
, it could have been aMap<Character, Integer>
. - Print the frequencies: a function that takes the frequencies in some form, returns nothing, and prints to screen the frequencies nicely formatted.
Computing indexes of letters
If the input string contains only uppercase letters, then you can translate those letters to array indexes in the range of 0 to 25 (inclusive) like this:
int index = ch - 'A';
This eliminates the nested loop you had.
It also eliminates the need for the c
array.
Initializing arrays
Instead of this:
int f = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
You could write simply int f = new int[26];
Instead of this:
char c = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'};
I would take a lazy approach and write char c = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".toCharArray();
Use better variable names
Single-letter variable names should only be used for trivial, highly transient things.
The names f
and c
in the program are inappropriate, and make the program more difficult to read.
$endgroup$
$begingroup$
I've been sticking to arrays because I haven't yet started learning about Maps and HashMaps. I'll be sure to check them out now... Thanks for pointing out the toCharArray() function. I was unaware that it existed.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
I didn't mean that you should use aMap
. I mentioned as an option. Both ways have advantages and disadvantages, the best solution depends on the use case. Here it doesn't matter much.
$endgroup$
– janos
yesterday
$begingroup$
Understood. BTW, thought I'd point out about the variable names, I actually find it pretty convenient to go with single-letter names: c for character, f for frequency, i for iterator and so on.
$endgroup$
– Artemis Hunter
yesterday
1
$begingroup$
@TomG But your consumer usually wouldn't have access to your code, would they? The consumer is meant to receive the application which the code runs. In case of another coder, you might be right, but even a simple remark next to the variable's declaration might suffice, would it not? Actually, I only use single letters in my basic programs, like the one above. Usually my programs have clear names.
$endgroup$
– Artemis Hunter
yesterday
7
$begingroup$
@ArtemisHunter There are aspects of style that are subjective. In this example, good objective arguments exist, against comments. 1. The consumers of your binaries indeed don't care about variable names. But the consumers of your source code do: the human reviewers and maintainers. Without descriptive names, it's hard to keep in mind whatf
meant, you may have to scan back in the program to understand, which is a mental burden. 2. Comments can go out of sync with the code they describe. If instead of comments you write in a way that's self-descriptive, that's a better long-term solution.
$endgroup$
– janos
yesterday
|
show 2 more comments
$begingroup$
Warnings
Don't use the suppresswarnings
annotation for things that can be fixed easily. I don't recognize the "resource" warning, I guess it's specific to the compiler you're using (Eclipse?). Probably comes from using the Scanner
without closing it properly. By using the try-with-resources -statement for anything that supports java.lang.AutoCloseable
, this will be handled for you automatically.
Variables and naming
There's no point in trying to save a few keystrokes by using short variable names. The compiler doesn't care what the names are, so you can just as well use human-readable names. The exception here being de-facto standardized loop indices like i
and j
.
Any decent code editor or IDE will autocomplete the names for you, so it's not that much more to type. A modified section of your original code, notice how I also chained the call to toUpperCase()
directly after the nextLine()
call. No need to create a new variable for the case-corrected string:
Scanner inputScanner = new Scanner(System.in);
String input = inputScanner.nextLine().toUpperCase();
Object methods
Familiarize yourself with the Java standard library and API. The String class has a method for returning its contents as a char array: toCharArray()
. You could use that, combined with the enhanced for loop to simplify your loop:
String input = // fetch string somehow
for (char inputChar : input.toCharArray()) {
// Loop processing here
}
Printing an array is similarly a one-line operation: System.out.println(Arrays.toString(your array here))
Tips and tricks
There's a neat(?) trick for calculations using chars in Java. As you are upper-casing all the chars, you can use 'A' as the base for the array index. So instead of having two arrays, one with the frequencies, one for the char-to-index mapping, use subtraction from 'A' to get the index:
for (char inputChar : input.toCharArray()) {
frequencies[inputChar - 'A']++;
}
Alternative implementation
Here's my alternative implementation using only the same data structures as in your original post. I do agree with Vishal Dhanotiya about the use of a map for this.
import java.util.Arrays;
import java.util.Scanner;
public class Letters {
public static void main(String args) {
int frequencies = new int[26];
try (Scanner scanner = new Scanner(System.in)) {
System.out.print("Enter a string: ");
String input = scanner.nextLine().toUpperCase().replaceAll("[^A-Z]", "");
for (char inputChar : input.toCharArray()) {
frequencies[inputChar - 'A']++;
}
for (int i = 0; i < frequencies.length; i++) {
System.out.printf("%s: %d, ", (char)('A' + i), frequencies[i]);
}
}
}
}
$endgroup$
$begingroup$
Yes, the "resource" is from eclipse. I usually ignore it, but this time I just felt like adding it.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
It would probably be a good idea to check that inputChar is an uppercase letter. The code fails withArrayIndexOutOfBoundsException
otherwise.
$endgroup$
– Eric Duminil
yesterday
$begingroup$
@EricDuminil. I convert the whole string to uppercase, so that should be taken care of:String input = scanner.nextLine().toUpperCase();
. Should it still be checked withCharacter.isUpperCase()
? Of course, this code fails in many ways if there are non-ASCII characters in the input. But ASCII was an implicit requirement also for the original code. I could have mentioned it, though.
$endgroup$
– TomG
yesterday
$begingroup$
@TomG: It fails with a space for example.
$endgroup$
– Eric Duminil
yesterday
1
$begingroup$
If I was reviewing this, I think I'd want a comment for theinputChar - 'A'
trick. Could also init the frequencies to'Z' - 'A'
just to make clear what the length means?
$endgroup$
– JollyJoker
15 hours ago
|
show 2 more comments
$begingroup$
You can use hash map in java to calculate the frequency of characters in a string
import java.util.Map;
import java.util.HashMap;
public class Letters {
public static void main(String args) {
String str = "check repeated alphabets from a string ";
int len = str.length();
Map<Character, Integer> numChars = new HashMap<Character, Integer>(Math.min(len, 26));
for (int i = 0; i < len; ++i) {
char charAt = str.charAt(i);
if (!numChars.containsKey(charAt)) {
numChars.put(charAt, 1);
} else {
numChars.put(charAt, numChars.get(charAt) + 1);
} }
System.out.println(numChars);
}
}
Result
{" " =5, a=5, b=1, c=2, d=1, e=6, h=3, k=1, l=1, m=1, n=1, o=1, p=2, r=2, s=1, t=2, w=1, y=1}
New contributor
$endgroup$
We are looking for answers that provide insightful observations about the code in the question. Answers that consist of independent solutions with no justification do not constitute a code review, and may be removed.
3
$begingroup$
You are providing an alternative implementation, but no review of the original code. Could you update your post to cover some aspects of the code?
$endgroup$
– TomG
yesterday
$begingroup$
He asks for both review or alternative solutions that's why I am providing this solution ok no issue I will review his code and provide my feedback on it
$endgroup$
– Vishal Dhanotiya
yesterday
$begingroup$
I actually don't mind an alternate implementation as well. I'd be eager to learn about the different ways I can approach a certain task. While I do not know just yet how Map and HashMap work, it does seem a lot simpler than the approach I took.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
Welcome to Code Review Vishal. Even if the original poster is willing to accept an alternative implementation, it doesn't fit within the guidelines for an answer on this site. If you can't squeeze out a little review there's a good chance this will get downvoted or deleted.
$endgroup$
– chicks
yesterday
3
$begingroup$
To be clear though, the rule is that each answer provide one insight about the original code. As written, this answer does not do that. The insight does not have to be blazing in brilliance nor long. A simple, "A HashMap offers a simpler solution than parallel arrays" would be sufficient (and you are welcome to use that without attribution). And then you can provide the alternative solution. You might also consider how that would affect performance (likely worse). Or even compare the two solutions.
$endgroup$
– mdfst13
yesterday
|
show 2 more comments
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Artemis Hunter is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f215592%2fcalculate-the-frequency-of-characters-in-a-string%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Separate logical elements
It's good to separate different logical parts of a program, for example:
- Parse input: a function that takes an
InputStream
and returns aString
- Compute frequencies: a function that takes a
String
and returns frequencies in some form. In your current program you used anint
, it could have been aMap<Character, Integer>
. - Print the frequencies: a function that takes the frequencies in some form, returns nothing, and prints to screen the frequencies nicely formatted.
Computing indexes of letters
If the input string contains only uppercase letters, then you can translate those letters to array indexes in the range of 0 to 25 (inclusive) like this:
int index = ch - 'A';
This eliminates the nested loop you had.
It also eliminates the need for the c
array.
Initializing arrays
Instead of this:
int f = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
You could write simply int f = new int[26];
Instead of this:
char c = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'};
I would take a lazy approach and write char c = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".toCharArray();
Use better variable names
Single-letter variable names should only be used for trivial, highly transient things.
The names f
and c
in the program are inappropriate, and make the program more difficult to read.
$endgroup$
$begingroup$
I've been sticking to arrays because I haven't yet started learning about Maps and HashMaps. I'll be sure to check them out now... Thanks for pointing out the toCharArray() function. I was unaware that it existed.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
I didn't mean that you should use aMap
. I mentioned as an option. Both ways have advantages and disadvantages, the best solution depends on the use case. Here it doesn't matter much.
$endgroup$
– janos
yesterday
$begingroup$
Understood. BTW, thought I'd point out about the variable names, I actually find it pretty convenient to go with single-letter names: c for character, f for frequency, i for iterator and so on.
$endgroup$
– Artemis Hunter
yesterday
1
$begingroup$
@TomG But your consumer usually wouldn't have access to your code, would they? The consumer is meant to receive the application which the code runs. In case of another coder, you might be right, but even a simple remark next to the variable's declaration might suffice, would it not? Actually, I only use single letters in my basic programs, like the one above. Usually my programs have clear names.
$endgroup$
– Artemis Hunter
yesterday
7
$begingroup$
@ArtemisHunter There are aspects of style that are subjective. In this example, good objective arguments exist, against comments. 1. The consumers of your binaries indeed don't care about variable names. But the consumers of your source code do: the human reviewers and maintainers. Without descriptive names, it's hard to keep in mind whatf
meant, you may have to scan back in the program to understand, which is a mental burden. 2. Comments can go out of sync with the code they describe. If instead of comments you write in a way that's self-descriptive, that's a better long-term solution.
$endgroup$
– janos
yesterday
|
show 2 more comments
$begingroup$
Separate logical elements
It's good to separate different logical parts of a program, for example:
- Parse input: a function that takes an
InputStream
and returns aString
- Compute frequencies: a function that takes a
String
and returns frequencies in some form. In your current program you used anint
, it could have been aMap<Character, Integer>
. - Print the frequencies: a function that takes the frequencies in some form, returns nothing, and prints to screen the frequencies nicely formatted.
Computing indexes of letters
If the input string contains only uppercase letters, then you can translate those letters to array indexes in the range of 0 to 25 (inclusive) like this:
int index = ch - 'A';
This eliminates the nested loop you had.
It also eliminates the need for the c
array.
Initializing arrays
Instead of this:
int f = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
You could write simply int f = new int[26];
Instead of this:
char c = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'};
I would take a lazy approach and write char c = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".toCharArray();
Use better variable names
Single-letter variable names should only be used for trivial, highly transient things.
The names f
and c
in the program are inappropriate, and make the program more difficult to read.
$endgroup$
$begingroup$
I've been sticking to arrays because I haven't yet started learning about Maps and HashMaps. I'll be sure to check them out now... Thanks for pointing out the toCharArray() function. I was unaware that it existed.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
I didn't mean that you should use aMap
. I mentioned as an option. Both ways have advantages and disadvantages, the best solution depends on the use case. Here it doesn't matter much.
$endgroup$
– janos
yesterday
$begingroup$
Understood. BTW, thought I'd point out about the variable names, I actually find it pretty convenient to go with single-letter names: c for character, f for frequency, i for iterator and so on.
$endgroup$
– Artemis Hunter
yesterday
1
$begingroup$
@TomG But your consumer usually wouldn't have access to your code, would they? The consumer is meant to receive the application which the code runs. In case of another coder, you might be right, but even a simple remark next to the variable's declaration might suffice, would it not? Actually, I only use single letters in my basic programs, like the one above. Usually my programs have clear names.
$endgroup$
– Artemis Hunter
yesterday
7
$begingroup$
@ArtemisHunter There are aspects of style that are subjective. In this example, good objective arguments exist, against comments. 1. The consumers of your binaries indeed don't care about variable names. But the consumers of your source code do: the human reviewers and maintainers. Without descriptive names, it's hard to keep in mind whatf
meant, you may have to scan back in the program to understand, which is a mental burden. 2. Comments can go out of sync with the code they describe. If instead of comments you write in a way that's self-descriptive, that's a better long-term solution.
$endgroup$
– janos
yesterday
|
show 2 more comments
$begingroup$
Separate logical elements
It's good to separate different logical parts of a program, for example:
- Parse input: a function that takes an
InputStream
and returns aString
- Compute frequencies: a function that takes a
String
and returns frequencies in some form. In your current program you used anint
, it could have been aMap<Character, Integer>
. - Print the frequencies: a function that takes the frequencies in some form, returns nothing, and prints to screen the frequencies nicely formatted.
Computing indexes of letters
If the input string contains only uppercase letters, then you can translate those letters to array indexes in the range of 0 to 25 (inclusive) like this:
int index = ch - 'A';
This eliminates the nested loop you had.
It also eliminates the need for the c
array.
Initializing arrays
Instead of this:
int f = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
You could write simply int f = new int[26];
Instead of this:
char c = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'};
I would take a lazy approach and write char c = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".toCharArray();
Use better variable names
Single-letter variable names should only be used for trivial, highly transient things.
The names f
and c
in the program are inappropriate, and make the program more difficult to read.
$endgroup$
Separate logical elements
It's good to separate different logical parts of a program, for example:
- Parse input: a function that takes an
InputStream
and returns aString
- Compute frequencies: a function that takes a
String
and returns frequencies in some form. In your current program you used anint
, it could have been aMap<Character, Integer>
. - Print the frequencies: a function that takes the frequencies in some form, returns nothing, and prints to screen the frequencies nicely formatted.
Computing indexes of letters
If the input string contains only uppercase letters, then you can translate those letters to array indexes in the range of 0 to 25 (inclusive) like this:
int index = ch - 'A';
This eliminates the nested loop you had.
It also eliminates the need for the c
array.
Initializing arrays
Instead of this:
int f = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
You could write simply int f = new int[26];
Instead of this:
char c = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'};
I would take a lazy approach and write char c = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".toCharArray();
Use better variable names
Single-letter variable names should only be used for trivial, highly transient things.
The names f
and c
in the program are inappropriate, and make the program more difficult to read.
answered yesterday
janosjanos
98.7k12125351
98.7k12125351
$begingroup$
I've been sticking to arrays because I haven't yet started learning about Maps and HashMaps. I'll be sure to check them out now... Thanks for pointing out the toCharArray() function. I was unaware that it existed.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
I didn't mean that you should use aMap
. I mentioned as an option. Both ways have advantages and disadvantages, the best solution depends on the use case. Here it doesn't matter much.
$endgroup$
– janos
yesterday
$begingroup$
Understood. BTW, thought I'd point out about the variable names, I actually find it pretty convenient to go with single-letter names: c for character, f for frequency, i for iterator and so on.
$endgroup$
– Artemis Hunter
yesterday
1
$begingroup$
@TomG But your consumer usually wouldn't have access to your code, would they? The consumer is meant to receive the application which the code runs. In case of another coder, you might be right, but even a simple remark next to the variable's declaration might suffice, would it not? Actually, I only use single letters in my basic programs, like the one above. Usually my programs have clear names.
$endgroup$
– Artemis Hunter
yesterday
7
$begingroup$
@ArtemisHunter There are aspects of style that are subjective. In this example, good objective arguments exist, against comments. 1. The consumers of your binaries indeed don't care about variable names. But the consumers of your source code do: the human reviewers and maintainers. Without descriptive names, it's hard to keep in mind whatf
meant, you may have to scan back in the program to understand, which is a mental burden. 2. Comments can go out of sync with the code they describe. If instead of comments you write in a way that's self-descriptive, that's a better long-term solution.
$endgroup$
– janos
yesterday
|
show 2 more comments
$begingroup$
I've been sticking to arrays because I haven't yet started learning about Maps and HashMaps. I'll be sure to check them out now... Thanks for pointing out the toCharArray() function. I was unaware that it existed.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
I didn't mean that you should use aMap
. I mentioned as an option. Both ways have advantages and disadvantages, the best solution depends on the use case. Here it doesn't matter much.
$endgroup$
– janos
yesterday
$begingroup$
Understood. BTW, thought I'd point out about the variable names, I actually find it pretty convenient to go with single-letter names: c for character, f for frequency, i for iterator and so on.
$endgroup$
– Artemis Hunter
yesterday
1
$begingroup$
@TomG But your consumer usually wouldn't have access to your code, would they? The consumer is meant to receive the application which the code runs. In case of another coder, you might be right, but even a simple remark next to the variable's declaration might suffice, would it not? Actually, I only use single letters in my basic programs, like the one above. Usually my programs have clear names.
$endgroup$
– Artemis Hunter
yesterday
7
$begingroup$
@ArtemisHunter There are aspects of style that are subjective. In this example, good objective arguments exist, against comments. 1. The consumers of your binaries indeed don't care about variable names. But the consumers of your source code do: the human reviewers and maintainers. Without descriptive names, it's hard to keep in mind whatf
meant, you may have to scan back in the program to understand, which is a mental burden. 2. Comments can go out of sync with the code they describe. If instead of comments you write in a way that's self-descriptive, that's a better long-term solution.
$endgroup$
– janos
yesterday
$begingroup$
I've been sticking to arrays because I haven't yet started learning about Maps and HashMaps. I'll be sure to check them out now... Thanks for pointing out the toCharArray() function. I was unaware that it existed.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
I've been sticking to arrays because I haven't yet started learning about Maps and HashMaps. I'll be sure to check them out now... Thanks for pointing out the toCharArray() function. I was unaware that it existed.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
I didn't mean that you should use a
Map
. I mentioned as an option. Both ways have advantages and disadvantages, the best solution depends on the use case. Here it doesn't matter much.$endgroup$
– janos
yesterday
$begingroup$
I didn't mean that you should use a
Map
. I mentioned as an option. Both ways have advantages and disadvantages, the best solution depends on the use case. Here it doesn't matter much.$endgroup$
– janos
yesterday
$begingroup$
Understood. BTW, thought I'd point out about the variable names, I actually find it pretty convenient to go with single-letter names: c for character, f for frequency, i for iterator and so on.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
Understood. BTW, thought I'd point out about the variable names, I actually find it pretty convenient to go with single-letter names: c for character, f for frequency, i for iterator and so on.
$endgroup$
– Artemis Hunter
yesterday
1
1
$begingroup$
@TomG But your consumer usually wouldn't have access to your code, would they? The consumer is meant to receive the application which the code runs. In case of another coder, you might be right, but even a simple remark next to the variable's declaration might suffice, would it not? Actually, I only use single letters in my basic programs, like the one above. Usually my programs have clear names.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
@TomG But your consumer usually wouldn't have access to your code, would they? The consumer is meant to receive the application which the code runs. In case of another coder, you might be right, but even a simple remark next to the variable's declaration might suffice, would it not? Actually, I only use single letters in my basic programs, like the one above. Usually my programs have clear names.
$endgroup$
– Artemis Hunter
yesterday
7
7
$begingroup$
@ArtemisHunter There are aspects of style that are subjective. In this example, good objective arguments exist, against comments. 1. The consumers of your binaries indeed don't care about variable names. But the consumers of your source code do: the human reviewers and maintainers. Without descriptive names, it's hard to keep in mind what
f
meant, you may have to scan back in the program to understand, which is a mental burden. 2. Comments can go out of sync with the code they describe. If instead of comments you write in a way that's self-descriptive, that's a better long-term solution.$endgroup$
– janos
yesterday
$begingroup$
@ArtemisHunter There are aspects of style that are subjective. In this example, good objective arguments exist, against comments. 1. The consumers of your binaries indeed don't care about variable names. But the consumers of your source code do: the human reviewers and maintainers. Without descriptive names, it's hard to keep in mind what
f
meant, you may have to scan back in the program to understand, which is a mental burden. 2. Comments can go out of sync with the code they describe. If instead of comments you write in a way that's self-descriptive, that's a better long-term solution.$endgroup$
– janos
yesterday
|
show 2 more comments
$begingroup$
Warnings
Don't use the suppresswarnings
annotation for things that can be fixed easily. I don't recognize the "resource" warning, I guess it's specific to the compiler you're using (Eclipse?). Probably comes from using the Scanner
without closing it properly. By using the try-with-resources -statement for anything that supports java.lang.AutoCloseable
, this will be handled for you automatically.
Variables and naming
There's no point in trying to save a few keystrokes by using short variable names. The compiler doesn't care what the names are, so you can just as well use human-readable names. The exception here being de-facto standardized loop indices like i
and j
.
Any decent code editor or IDE will autocomplete the names for you, so it's not that much more to type. A modified section of your original code, notice how I also chained the call to toUpperCase()
directly after the nextLine()
call. No need to create a new variable for the case-corrected string:
Scanner inputScanner = new Scanner(System.in);
String input = inputScanner.nextLine().toUpperCase();
Object methods
Familiarize yourself with the Java standard library and API. The String class has a method for returning its contents as a char array: toCharArray()
. You could use that, combined with the enhanced for loop to simplify your loop:
String input = // fetch string somehow
for (char inputChar : input.toCharArray()) {
// Loop processing here
}
Printing an array is similarly a one-line operation: System.out.println(Arrays.toString(your array here))
Tips and tricks
There's a neat(?) trick for calculations using chars in Java. As you are upper-casing all the chars, you can use 'A' as the base for the array index. So instead of having two arrays, one with the frequencies, one for the char-to-index mapping, use subtraction from 'A' to get the index:
for (char inputChar : input.toCharArray()) {
frequencies[inputChar - 'A']++;
}
Alternative implementation
Here's my alternative implementation using only the same data structures as in your original post. I do agree with Vishal Dhanotiya about the use of a map for this.
import java.util.Arrays;
import java.util.Scanner;
public class Letters {
public static void main(String args) {
int frequencies = new int[26];
try (Scanner scanner = new Scanner(System.in)) {
System.out.print("Enter a string: ");
String input = scanner.nextLine().toUpperCase().replaceAll("[^A-Z]", "");
for (char inputChar : input.toCharArray()) {
frequencies[inputChar - 'A']++;
}
for (int i = 0; i < frequencies.length; i++) {
System.out.printf("%s: %d, ", (char)('A' + i), frequencies[i]);
}
}
}
}
$endgroup$
$begingroup$
Yes, the "resource" is from eclipse. I usually ignore it, but this time I just felt like adding it.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
It would probably be a good idea to check that inputChar is an uppercase letter. The code fails withArrayIndexOutOfBoundsException
otherwise.
$endgroup$
– Eric Duminil
yesterday
$begingroup$
@EricDuminil. I convert the whole string to uppercase, so that should be taken care of:String input = scanner.nextLine().toUpperCase();
. Should it still be checked withCharacter.isUpperCase()
? Of course, this code fails in many ways if there are non-ASCII characters in the input. But ASCII was an implicit requirement also for the original code. I could have mentioned it, though.
$endgroup$
– TomG
yesterday
$begingroup$
@TomG: It fails with a space for example.
$endgroup$
– Eric Duminil
yesterday
1
$begingroup$
If I was reviewing this, I think I'd want a comment for theinputChar - 'A'
trick. Could also init the frequencies to'Z' - 'A'
just to make clear what the length means?
$endgroup$
– JollyJoker
15 hours ago
|
show 2 more comments
$begingroup$
Warnings
Don't use the suppresswarnings
annotation for things that can be fixed easily. I don't recognize the "resource" warning, I guess it's specific to the compiler you're using (Eclipse?). Probably comes from using the Scanner
without closing it properly. By using the try-with-resources -statement for anything that supports java.lang.AutoCloseable
, this will be handled for you automatically.
Variables and naming
There's no point in trying to save a few keystrokes by using short variable names. The compiler doesn't care what the names are, so you can just as well use human-readable names. The exception here being de-facto standardized loop indices like i
and j
.
Any decent code editor or IDE will autocomplete the names for you, so it's not that much more to type. A modified section of your original code, notice how I also chained the call to toUpperCase()
directly after the nextLine()
call. No need to create a new variable for the case-corrected string:
Scanner inputScanner = new Scanner(System.in);
String input = inputScanner.nextLine().toUpperCase();
Object methods
Familiarize yourself with the Java standard library and API. The String class has a method for returning its contents as a char array: toCharArray()
. You could use that, combined with the enhanced for loop to simplify your loop:
String input = // fetch string somehow
for (char inputChar : input.toCharArray()) {
// Loop processing here
}
Printing an array is similarly a one-line operation: System.out.println(Arrays.toString(your array here))
Tips and tricks
There's a neat(?) trick for calculations using chars in Java. As you are upper-casing all the chars, you can use 'A' as the base for the array index. So instead of having two arrays, one with the frequencies, one for the char-to-index mapping, use subtraction from 'A' to get the index:
for (char inputChar : input.toCharArray()) {
frequencies[inputChar - 'A']++;
}
Alternative implementation
Here's my alternative implementation using only the same data structures as in your original post. I do agree with Vishal Dhanotiya about the use of a map for this.
import java.util.Arrays;
import java.util.Scanner;
public class Letters {
public static void main(String args) {
int frequencies = new int[26];
try (Scanner scanner = new Scanner(System.in)) {
System.out.print("Enter a string: ");
String input = scanner.nextLine().toUpperCase().replaceAll("[^A-Z]", "");
for (char inputChar : input.toCharArray()) {
frequencies[inputChar - 'A']++;
}
for (int i = 0; i < frequencies.length; i++) {
System.out.printf("%s: %d, ", (char)('A' + i), frequencies[i]);
}
}
}
}
$endgroup$
$begingroup$
Yes, the "resource" is from eclipse. I usually ignore it, but this time I just felt like adding it.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
It would probably be a good idea to check that inputChar is an uppercase letter. The code fails withArrayIndexOutOfBoundsException
otherwise.
$endgroup$
– Eric Duminil
yesterday
$begingroup$
@EricDuminil. I convert the whole string to uppercase, so that should be taken care of:String input = scanner.nextLine().toUpperCase();
. Should it still be checked withCharacter.isUpperCase()
? Of course, this code fails in many ways if there are non-ASCII characters in the input. But ASCII was an implicit requirement also for the original code. I could have mentioned it, though.
$endgroup$
– TomG
yesterday
$begingroup$
@TomG: It fails with a space for example.
$endgroup$
– Eric Duminil
yesterday
1
$begingroup$
If I was reviewing this, I think I'd want a comment for theinputChar - 'A'
trick. Could also init the frequencies to'Z' - 'A'
just to make clear what the length means?
$endgroup$
– JollyJoker
15 hours ago
|
show 2 more comments
$begingroup$
Warnings
Don't use the suppresswarnings
annotation for things that can be fixed easily. I don't recognize the "resource" warning, I guess it's specific to the compiler you're using (Eclipse?). Probably comes from using the Scanner
without closing it properly. By using the try-with-resources -statement for anything that supports java.lang.AutoCloseable
, this will be handled for you automatically.
Variables and naming
There's no point in trying to save a few keystrokes by using short variable names. The compiler doesn't care what the names are, so you can just as well use human-readable names. The exception here being de-facto standardized loop indices like i
and j
.
Any decent code editor or IDE will autocomplete the names for you, so it's not that much more to type. A modified section of your original code, notice how I also chained the call to toUpperCase()
directly after the nextLine()
call. No need to create a new variable for the case-corrected string:
Scanner inputScanner = new Scanner(System.in);
String input = inputScanner.nextLine().toUpperCase();
Object methods
Familiarize yourself with the Java standard library and API. The String class has a method for returning its contents as a char array: toCharArray()
. You could use that, combined with the enhanced for loop to simplify your loop:
String input = // fetch string somehow
for (char inputChar : input.toCharArray()) {
// Loop processing here
}
Printing an array is similarly a one-line operation: System.out.println(Arrays.toString(your array here))
Tips and tricks
There's a neat(?) trick for calculations using chars in Java. As you are upper-casing all the chars, you can use 'A' as the base for the array index. So instead of having two arrays, one with the frequencies, one for the char-to-index mapping, use subtraction from 'A' to get the index:
for (char inputChar : input.toCharArray()) {
frequencies[inputChar - 'A']++;
}
Alternative implementation
Here's my alternative implementation using only the same data structures as in your original post. I do agree with Vishal Dhanotiya about the use of a map for this.
import java.util.Arrays;
import java.util.Scanner;
public class Letters {
public static void main(String args) {
int frequencies = new int[26];
try (Scanner scanner = new Scanner(System.in)) {
System.out.print("Enter a string: ");
String input = scanner.nextLine().toUpperCase().replaceAll("[^A-Z]", "");
for (char inputChar : input.toCharArray()) {
frequencies[inputChar - 'A']++;
}
for (int i = 0; i < frequencies.length; i++) {
System.out.printf("%s: %d, ", (char)('A' + i), frequencies[i]);
}
}
}
}
$endgroup$
Warnings
Don't use the suppresswarnings
annotation for things that can be fixed easily. I don't recognize the "resource" warning, I guess it's specific to the compiler you're using (Eclipse?). Probably comes from using the Scanner
without closing it properly. By using the try-with-resources -statement for anything that supports java.lang.AutoCloseable
, this will be handled for you automatically.
Variables and naming
There's no point in trying to save a few keystrokes by using short variable names. The compiler doesn't care what the names are, so you can just as well use human-readable names. The exception here being de-facto standardized loop indices like i
and j
.
Any decent code editor or IDE will autocomplete the names for you, so it's not that much more to type. A modified section of your original code, notice how I also chained the call to toUpperCase()
directly after the nextLine()
call. No need to create a new variable for the case-corrected string:
Scanner inputScanner = new Scanner(System.in);
String input = inputScanner.nextLine().toUpperCase();
Object methods
Familiarize yourself with the Java standard library and API. The String class has a method for returning its contents as a char array: toCharArray()
. You could use that, combined with the enhanced for loop to simplify your loop:
String input = // fetch string somehow
for (char inputChar : input.toCharArray()) {
// Loop processing here
}
Printing an array is similarly a one-line operation: System.out.println(Arrays.toString(your array here))
Tips and tricks
There's a neat(?) trick for calculations using chars in Java. As you are upper-casing all the chars, you can use 'A' as the base for the array index. So instead of having two arrays, one with the frequencies, one for the char-to-index mapping, use subtraction from 'A' to get the index:
for (char inputChar : input.toCharArray()) {
frequencies[inputChar - 'A']++;
}
Alternative implementation
Here's my alternative implementation using only the same data structures as in your original post. I do agree with Vishal Dhanotiya about the use of a map for this.
import java.util.Arrays;
import java.util.Scanner;
public class Letters {
public static void main(String args) {
int frequencies = new int[26];
try (Scanner scanner = new Scanner(System.in)) {
System.out.print("Enter a string: ");
String input = scanner.nextLine().toUpperCase().replaceAll("[^A-Z]", "");
for (char inputChar : input.toCharArray()) {
frequencies[inputChar - 'A']++;
}
for (int i = 0; i < frequencies.length; i++) {
System.out.printf("%s: %d, ", (char)('A' + i), frequencies[i]);
}
}
}
}
edited yesterday
answered yesterday
TomGTomG
47627
47627
$begingroup$
Yes, the "resource" is from eclipse. I usually ignore it, but this time I just felt like adding it.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
It would probably be a good idea to check that inputChar is an uppercase letter. The code fails withArrayIndexOutOfBoundsException
otherwise.
$endgroup$
– Eric Duminil
yesterday
$begingroup$
@EricDuminil. I convert the whole string to uppercase, so that should be taken care of:String input = scanner.nextLine().toUpperCase();
. Should it still be checked withCharacter.isUpperCase()
? Of course, this code fails in many ways if there are non-ASCII characters in the input. But ASCII was an implicit requirement also for the original code. I could have mentioned it, though.
$endgroup$
– TomG
yesterday
$begingroup$
@TomG: It fails with a space for example.
$endgroup$
– Eric Duminil
yesterday
1
$begingroup$
If I was reviewing this, I think I'd want a comment for theinputChar - 'A'
trick. Could also init the frequencies to'Z' - 'A'
just to make clear what the length means?
$endgroup$
– JollyJoker
15 hours ago
|
show 2 more comments
$begingroup$
Yes, the "resource" is from eclipse. I usually ignore it, but this time I just felt like adding it.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
It would probably be a good idea to check that inputChar is an uppercase letter. The code fails withArrayIndexOutOfBoundsException
otherwise.
$endgroup$
– Eric Duminil
yesterday
$begingroup$
@EricDuminil. I convert the whole string to uppercase, so that should be taken care of:String input = scanner.nextLine().toUpperCase();
. Should it still be checked withCharacter.isUpperCase()
? Of course, this code fails in many ways if there are non-ASCII characters in the input. But ASCII was an implicit requirement also for the original code. I could have mentioned it, though.
$endgroup$
– TomG
yesterday
$begingroup$
@TomG: It fails with a space for example.
$endgroup$
– Eric Duminil
yesterday
1
$begingroup$
If I was reviewing this, I think I'd want a comment for theinputChar - 'A'
trick. Could also init the frequencies to'Z' - 'A'
just to make clear what the length means?
$endgroup$
– JollyJoker
15 hours ago
$begingroup$
Yes, the "resource" is from eclipse. I usually ignore it, but this time I just felt like adding it.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
Yes, the "resource" is from eclipse. I usually ignore it, but this time I just felt like adding it.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
It would probably be a good idea to check that inputChar is an uppercase letter. The code fails with
ArrayIndexOutOfBoundsException
otherwise.$endgroup$
– Eric Duminil
yesterday
$begingroup$
It would probably be a good idea to check that inputChar is an uppercase letter. The code fails with
ArrayIndexOutOfBoundsException
otherwise.$endgroup$
– Eric Duminil
yesterday
$begingroup$
@EricDuminil. I convert the whole string to uppercase, so that should be taken care of:
String input = scanner.nextLine().toUpperCase();
. Should it still be checked withCharacter.isUpperCase()
? Of course, this code fails in many ways if there are non-ASCII characters in the input. But ASCII was an implicit requirement also for the original code. I could have mentioned it, though.$endgroup$
– TomG
yesterday
$begingroup$
@EricDuminil. I convert the whole string to uppercase, so that should be taken care of:
String input = scanner.nextLine().toUpperCase();
. Should it still be checked withCharacter.isUpperCase()
? Of course, this code fails in many ways if there are non-ASCII characters in the input. But ASCII was an implicit requirement also for the original code. I could have mentioned it, though.$endgroup$
– TomG
yesterday
$begingroup$
@TomG: It fails with a space for example.
$endgroup$
– Eric Duminil
yesterday
$begingroup$
@TomG: It fails with a space for example.
$endgroup$
– Eric Duminil
yesterday
1
1
$begingroup$
If I was reviewing this, I think I'd want a comment for the
inputChar - 'A'
trick. Could also init the frequencies to 'Z' - 'A'
just to make clear what the length means?$endgroup$
– JollyJoker
15 hours ago
$begingroup$
If I was reviewing this, I think I'd want a comment for the
inputChar - 'A'
trick. Could also init the frequencies to 'Z' - 'A'
just to make clear what the length means?$endgroup$
– JollyJoker
15 hours ago
|
show 2 more comments
$begingroup$
You can use hash map in java to calculate the frequency of characters in a string
import java.util.Map;
import java.util.HashMap;
public class Letters {
public static void main(String args) {
String str = "check repeated alphabets from a string ";
int len = str.length();
Map<Character, Integer> numChars = new HashMap<Character, Integer>(Math.min(len, 26));
for (int i = 0; i < len; ++i) {
char charAt = str.charAt(i);
if (!numChars.containsKey(charAt)) {
numChars.put(charAt, 1);
} else {
numChars.put(charAt, numChars.get(charAt) + 1);
} }
System.out.println(numChars);
}
}
Result
{" " =5, a=5, b=1, c=2, d=1, e=6, h=3, k=1, l=1, m=1, n=1, o=1, p=2, r=2, s=1, t=2, w=1, y=1}
New contributor
$endgroup$
We are looking for answers that provide insightful observations about the code in the question. Answers that consist of independent solutions with no justification do not constitute a code review, and may be removed.
3
$begingroup$
You are providing an alternative implementation, but no review of the original code. Could you update your post to cover some aspects of the code?
$endgroup$
– TomG
yesterday
$begingroup$
He asks for both review or alternative solutions that's why I am providing this solution ok no issue I will review his code and provide my feedback on it
$endgroup$
– Vishal Dhanotiya
yesterday
$begingroup$
I actually don't mind an alternate implementation as well. I'd be eager to learn about the different ways I can approach a certain task. While I do not know just yet how Map and HashMap work, it does seem a lot simpler than the approach I took.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
Welcome to Code Review Vishal. Even if the original poster is willing to accept an alternative implementation, it doesn't fit within the guidelines for an answer on this site. If you can't squeeze out a little review there's a good chance this will get downvoted or deleted.
$endgroup$
– chicks
yesterday
3
$begingroup$
To be clear though, the rule is that each answer provide one insight about the original code. As written, this answer does not do that. The insight does not have to be blazing in brilliance nor long. A simple, "A HashMap offers a simpler solution than parallel arrays" would be sufficient (and you are welcome to use that without attribution). And then you can provide the alternative solution. You might also consider how that would affect performance (likely worse). Or even compare the two solutions.
$endgroup$
– mdfst13
yesterday
|
show 2 more comments
$begingroup$
You can use hash map in java to calculate the frequency of characters in a string
import java.util.Map;
import java.util.HashMap;
public class Letters {
public static void main(String args) {
String str = "check repeated alphabets from a string ";
int len = str.length();
Map<Character, Integer> numChars = new HashMap<Character, Integer>(Math.min(len, 26));
for (int i = 0; i < len; ++i) {
char charAt = str.charAt(i);
if (!numChars.containsKey(charAt)) {
numChars.put(charAt, 1);
} else {
numChars.put(charAt, numChars.get(charAt) + 1);
} }
System.out.println(numChars);
}
}
Result
{" " =5, a=5, b=1, c=2, d=1, e=6, h=3, k=1, l=1, m=1, n=1, o=1, p=2, r=2, s=1, t=2, w=1, y=1}
New contributor
$endgroup$
We are looking for answers that provide insightful observations about the code in the question. Answers that consist of independent solutions with no justification do not constitute a code review, and may be removed.
3
$begingroup$
You are providing an alternative implementation, but no review of the original code. Could you update your post to cover some aspects of the code?
$endgroup$
– TomG
yesterday
$begingroup$
He asks for both review or alternative solutions that's why I am providing this solution ok no issue I will review his code and provide my feedback on it
$endgroup$
– Vishal Dhanotiya
yesterday
$begingroup$
I actually don't mind an alternate implementation as well. I'd be eager to learn about the different ways I can approach a certain task. While I do not know just yet how Map and HashMap work, it does seem a lot simpler than the approach I took.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
Welcome to Code Review Vishal. Even if the original poster is willing to accept an alternative implementation, it doesn't fit within the guidelines for an answer on this site. If you can't squeeze out a little review there's a good chance this will get downvoted or deleted.
$endgroup$
– chicks
yesterday
3
$begingroup$
To be clear though, the rule is that each answer provide one insight about the original code. As written, this answer does not do that. The insight does not have to be blazing in brilliance nor long. A simple, "A HashMap offers a simpler solution than parallel arrays" would be sufficient (and you are welcome to use that without attribution). And then you can provide the alternative solution. You might also consider how that would affect performance (likely worse). Or even compare the two solutions.
$endgroup$
– mdfst13
yesterday
|
show 2 more comments
$begingroup$
You can use hash map in java to calculate the frequency of characters in a string
import java.util.Map;
import java.util.HashMap;
public class Letters {
public static void main(String args) {
String str = "check repeated alphabets from a string ";
int len = str.length();
Map<Character, Integer> numChars = new HashMap<Character, Integer>(Math.min(len, 26));
for (int i = 0; i < len; ++i) {
char charAt = str.charAt(i);
if (!numChars.containsKey(charAt)) {
numChars.put(charAt, 1);
} else {
numChars.put(charAt, numChars.get(charAt) + 1);
} }
System.out.println(numChars);
}
}
Result
{" " =5, a=5, b=1, c=2, d=1, e=6, h=3, k=1, l=1, m=1, n=1, o=1, p=2, r=2, s=1, t=2, w=1, y=1}
New contributor
$endgroup$
You can use hash map in java to calculate the frequency of characters in a string
import java.util.Map;
import java.util.HashMap;
public class Letters {
public static void main(String args) {
String str = "check repeated alphabets from a string ";
int len = str.length();
Map<Character, Integer> numChars = new HashMap<Character, Integer>(Math.min(len, 26));
for (int i = 0; i < len; ++i) {
char charAt = str.charAt(i);
if (!numChars.containsKey(charAt)) {
numChars.put(charAt, 1);
} else {
numChars.put(charAt, numChars.get(charAt) + 1);
} }
System.out.println(numChars);
}
}
Result
{" " =5, a=5, b=1, c=2, d=1, e=6, h=3, k=1, l=1, m=1, n=1, o=1, p=2, r=2, s=1, t=2, w=1, y=1}
New contributor
New contributor
answered yesterday
Vishal DhanotiyaVishal Dhanotiya
171
171
New contributor
New contributor
We are looking for answers that provide insightful observations about the code in the question. Answers that consist of independent solutions with no justification do not constitute a code review, and may be removed.
We are looking for answers that provide insightful observations about the code in the question. Answers that consist of independent solutions with no justification do not constitute a code review, and may be removed.
3
$begingroup$
You are providing an alternative implementation, but no review of the original code. Could you update your post to cover some aspects of the code?
$endgroup$
– TomG
yesterday
$begingroup$
He asks for both review or alternative solutions that's why I am providing this solution ok no issue I will review his code and provide my feedback on it
$endgroup$
– Vishal Dhanotiya
yesterday
$begingroup$
I actually don't mind an alternate implementation as well. I'd be eager to learn about the different ways I can approach a certain task. While I do not know just yet how Map and HashMap work, it does seem a lot simpler than the approach I took.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
Welcome to Code Review Vishal. Even if the original poster is willing to accept an alternative implementation, it doesn't fit within the guidelines for an answer on this site. If you can't squeeze out a little review there's a good chance this will get downvoted or deleted.
$endgroup$
– chicks
yesterday
3
$begingroup$
To be clear though, the rule is that each answer provide one insight about the original code. As written, this answer does not do that. The insight does not have to be blazing in brilliance nor long. A simple, "A HashMap offers a simpler solution than parallel arrays" would be sufficient (and you are welcome to use that without attribution). And then you can provide the alternative solution. You might also consider how that would affect performance (likely worse). Or even compare the two solutions.
$endgroup$
– mdfst13
yesterday
|
show 2 more comments
3
$begingroup$
You are providing an alternative implementation, but no review of the original code. Could you update your post to cover some aspects of the code?
$endgroup$
– TomG
yesterday
$begingroup$
He asks for both review or alternative solutions that's why I am providing this solution ok no issue I will review his code and provide my feedback on it
$endgroup$
– Vishal Dhanotiya
yesterday
$begingroup$
I actually don't mind an alternate implementation as well. I'd be eager to learn about the different ways I can approach a certain task. While I do not know just yet how Map and HashMap work, it does seem a lot simpler than the approach I took.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
Welcome to Code Review Vishal. Even if the original poster is willing to accept an alternative implementation, it doesn't fit within the guidelines for an answer on this site. If you can't squeeze out a little review there's a good chance this will get downvoted or deleted.
$endgroup$
– chicks
yesterday
3
$begingroup$
To be clear though, the rule is that each answer provide one insight about the original code. As written, this answer does not do that. The insight does not have to be blazing in brilliance nor long. A simple, "A HashMap offers a simpler solution than parallel arrays" would be sufficient (and you are welcome to use that without attribution). And then you can provide the alternative solution. You might also consider how that would affect performance (likely worse). Or even compare the two solutions.
$endgroup$
– mdfst13
yesterday
3
3
$begingroup$
You are providing an alternative implementation, but no review of the original code. Could you update your post to cover some aspects of the code?
$endgroup$
– TomG
yesterday
$begingroup$
You are providing an alternative implementation, but no review of the original code. Could you update your post to cover some aspects of the code?
$endgroup$
– TomG
yesterday
$begingroup$
He asks for both review or alternative solutions that's why I am providing this solution ok no issue I will review his code and provide my feedback on it
$endgroup$
– Vishal Dhanotiya
yesterday
$begingroup$
He asks for both review or alternative solutions that's why I am providing this solution ok no issue I will review his code and provide my feedback on it
$endgroup$
– Vishal Dhanotiya
yesterday
$begingroup$
I actually don't mind an alternate implementation as well. I'd be eager to learn about the different ways I can approach a certain task. While I do not know just yet how Map and HashMap work, it does seem a lot simpler than the approach I took.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
I actually don't mind an alternate implementation as well. I'd be eager to learn about the different ways I can approach a certain task. While I do not know just yet how Map and HashMap work, it does seem a lot simpler than the approach I took.
$endgroup$
– Artemis Hunter
yesterday
$begingroup$
Welcome to Code Review Vishal. Even if the original poster is willing to accept an alternative implementation, it doesn't fit within the guidelines for an answer on this site. If you can't squeeze out a little review there's a good chance this will get downvoted or deleted.
$endgroup$
– chicks
yesterday
$begingroup$
Welcome to Code Review Vishal. Even if the original poster is willing to accept an alternative implementation, it doesn't fit within the guidelines for an answer on this site. If you can't squeeze out a little review there's a good chance this will get downvoted or deleted.
$endgroup$
– chicks
yesterday
3
3
$begingroup$
To be clear though, the rule is that each answer provide one insight about the original code. As written, this answer does not do that. The insight does not have to be blazing in brilliance nor long. A simple, "A HashMap offers a simpler solution than parallel arrays" would be sufficient (and you are welcome to use that without attribution). And then you can provide the alternative solution. You might also consider how that would affect performance (likely worse). Or even compare the two solutions.
$endgroup$
– mdfst13
yesterday
$begingroup$
To be clear though, the rule is that each answer provide one insight about the original code. As written, this answer does not do that. The insight does not have to be blazing in brilliance nor long. A simple, "A HashMap offers a simpler solution than parallel arrays" would be sufficient (and you are welcome to use that without attribution). And then you can provide the alternative solution. You might also consider how that would affect performance (likely worse). Or even compare the two solutions.
$endgroup$
– mdfst13
yesterday
|
show 2 more comments
Artemis Hunter is a new contributor. Be nice, and check out our Code of Conduct.
Artemis Hunter is a new contributor. Be nice, and check out our Code of Conduct.
Artemis Hunter is a new contributor. Be nice, and check out our Code of Conduct.
Artemis Hunter is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f215592%2fcalculate-the-frequency-of-characters-in-a-string%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
1
$begingroup$
Hi Artemis! Welcome! This is a great question! I'm sure someone would help you with that!
$endgroup$
– Emma
yesterday