O
Keep the account of the characters foundThe main problem is that you don't have a record of what characters you've found.So, for example, if you take the chain "aaa" in your loop...for (int x = 0; x < line.Length; x++)
{
// ...
}
You find yourself finding line[x] That's it. 'a' in the first iteration, and then it's 'a' in the second, and again 'a' in the third.You need to take the account that characters you found.Let's see what data structure serves you. ..The order does not matter (you will not show the user the content).You need to find elements of value (the character).You will not have duplicates (you will not add a character twice)What you need is a https://es.wikipedia.org/wiki/Conjunto_(programaci%C3%B3n) ..NET offers us https://msdn.microsoft.com/es-es/library/bb359438(v=vs.110).aspx and https://msdn.microsoft.com/es-es/library/dd412070(v=vs.110).aspx . Either one of us serves.These are the differences:HashSet<T> exists from .NET 3.5, and SortedSet<T> was introduces in .NET 4.0HashSet<T> is compact in memory, SortedSet<T> It's not.SortedSet<T> keeps the data order. HashSet<T> No.Note: You don't need to keep the data order. It's additional work the computer would do unnecessarily, and it's a functionality you'd have to keep (use IComparer).Why not use an arrangement or a string?Because the search for an arrangement or a string occurs in linear time (O(n)), that is to say that the computer has to compare element by element. It is less efficient than looking for a set that has logarithmic time (O(log n)).I assume this is irrelevant to you, under the assumption that the size of the small input data.What linear time and logarithmic time means is that the time taken by the computer to perform the search is a linear function (the computer takes additional x milliseconds for each saved element) or logarithmic (the computer takes additional x milliseconds for each order of magnitude of the saved elements).There is an additional reason not to use an arrangement. And it's a fixed-size structure. I would like you to know beforehand how many different characters you're going to find... or make it big enough for the worst case (considering Unicode, we'd be talking about more than a million items).There is an additional reason not to use string. And it's just that the string They're immutable. You don't really change a stringYou create a new one every time you do an operation. That means every time you concaten a character you're creating a string new.This process creates trash unnecessarily.Again, this is not a problem if the input size is small (the modern computers have a lot of memory, at last and beyond). However, it is worth knowing.Compare ignoring capital and lowercasesThe other thing you mention is that you have to make the comparison not distinguish uppercases and lowercases.For this you can use https://msdn.microsoft.com/en-us/library/system.char.toupper(v=vs.110).aspx I don't recommend it, though. The reason is that the conversion of upper and lowercases is not the same in all languages. The example for this case is the Turk, in which the capital version of i That's it. İinstead of I.Of course, you're not thinking that your code has to work on a computer in Turk. However, it is good to take into account cultural differences when handling strings.Addenum: for Spanish Ñ It's a different letter. Nfor other languages is N with a diacritic mark.There are six solutions:Use ToUpper and assume that all comparisons are the language of the computer. Which can hide some defect that may appear when your program has to run elsewhere in the world.Use ToUpper and specify culture. This is, use the overload ToUpper received CultureInfo as a second parameter.Use ToUpperInvariantthat ignores the specific rules of the language.Do not change capitals, instead convert to string and use String.Compare. Which has on loads they receive ignoreCase to compare by ignoring upper and lowercases.Do not change capitals, instead convert to string and use String.Compare. Which also has on loads they receive CultureInfo in case you want to specify it.Do not change capitals, instead convert to string and use String.CompareOrdinal. Which has on loads they receive ignoreCase to compare by ignoring upper and lowercases.Now, come in. Ordinal e InvariantYou have to answer a question: á and A the same letter? Your requirements speak of capital letters, but not accents. And these are different characters. If your program should treat a and á separately you want a comparison Ordinal.Integrating the solutionOkay, we had already chosen data structure. It's a HashSet<T>. We need to tell him not to take a capital and a tiny account. Because what HashSet<T> receives is a IEqualityComparer<T>I'd say you use StringComparer And that way you save yourself from implementing that interface.We must also work around the drawback. HashSet<T> Wait a collection if we want to IEqualityComparer<T>. This is no problem, we can go through an empty arrangement.Then the code would be:var conjunto = new HashSet<string>(new string[]{}, StringComparer.OrdinalIgnoreCase);
Ovar conjunto = new HashSet<string>(new string[]{}, StringComparer.InvariantCultureIgnoreCase);
Ovar conjunto = new HashSet<string>(new string[]{}, StringComparer.CurrentCultureIgnoreCase);
Depending on whether you want ordinal comparison (OrdinalIgnoreCase), or if not, but you want to ignore specific rules of language (InvariantCultureIgnoreCase), or you want to use the specific rules of the current language (CurrentCultureIgnoreCase).Then you can use https://msdn.microsoft.com/es-es/library/bb356440(v=vs.110).aspx to verify if an element has been added. And of course https://msdn.microsoft.com/es-es/library/bb353005(v=vs.110).aspx to add.Note that conjunto.Add return bool. This value tells you if the element is new. Which can save you some code lines.Finally, your loops are the same:for (int x = 0; x < line.Length; x++)
{
for (int y = 0; y < line.Length; y++)
{
// ...
}
// ...
}
Both begin at the beginning of the line (0and end at the end of the line (line.Length).This means you're gonna start by comparing the first character line[x] with the first character line[y] because in the first iteration so much x Like y They are. 0 (int x = 0 e int y = 0).You should only compare with the characters following the current one (because the previous ones already compared them).Note: I'm assuming you should do the line counts per line of the file.With the changes mentioned, this would be the code to use:string line;
string caracter;
int compt = 0;
var comparador = StringComparer.InvariantCultureIgnoreCase;
while ((line = sr.ReadLine()) != null)
{
var conjunto = new HashSet<string>(new string[]{}, comparador);
for (int x = 0; x < line.Length; x++)
{
caracter = line[x].ToString();
if (conjunto.Add(caracter))
{
var compt = 1; // <--- ya lo encontré, lo coloco en 1
for (int y = x + 1; y < line.Length; y++)
{
if (comparador.Equals(caracter, line[y].ToString()))
{
compt++;
}
}
Console.WriteLine("El caracter " + caracter + " apareix " + compt + " vegades");
}
}
}
Notes:I've changed. caracter a guy. string. The reason is we're making comparisons at the level string. This because we're using StringComparer (there is no one CharComparer available in the BCL, and I preferred not to explain how to implement IEqualityComparer<char>This answer is long enough as it is.Using string also leave caracter without initializing. It's not what you need, but it doesn't make sense to give you any value.I got it. comparador aside, to use it when counting characters. It makes sense that they are counted in the same way that is verified if it has already been found.I'm starting. y in x + 1, so that y only checks from the following character of the x.It didn't make sense to value caracter in each cycle y. It just makes sense to value caracter when x change. That's why I pulled it out of the inner loop.Inside the loop, since the current character I have found it, I can start the counting 1.Notice how it is created conjunto and as used Add in a if.In fact, I think I can further improve the code...var comparador = StringComparer.InvariantCultureIgnoreCase;
while ((var line = sr.ReadLine()) != null)
{
var conjunto = new HashSet<string>(new string[]{}, comparador);
for (int x = 0; x < line.Length; x++)
{
var caracter = line[x].ToString();
if (conjunto.Add(caracter))
{
var compt = 1; // <--- ya lo encontré, lo coloco en 1
for (int y = x + 1; y < line.Length; y++)
{
if (comparador.Equals(caracter, line[y].ToString()))
{
compt++;
}
}
Console.WriteLine($"El caracter {caracter} apareix {compt} vegades");
}
}
}
Taking advantage of modern features of C#.Note: I have not tried this code.An alternativeAll of the above has been under the assumption that you must report the characters in the order you find them... if not... you can save in a data structure as many times you have found a character. So you run through the string once (you would have a single loop to go through the stringinstead of two nesting loops), adding to the count every character you find.Let's see what data structure serves you. ..It may matter (you will show the user the content)You need to find key elements (the character)You will not have duplicates (you will not add a character twice)You need key (the character) and value (the count).The data structure is a dictionary. We've got Dictionary<TKey, TValue> and SortedDictionary<TKey, TValue>. In this variant of the code, you will show the content of the data structure... I will assume that the order does not matter, since it makes things a little easier.while ((var line = sr.ReadLine()) != null)
{
var diccionario = new Dictionary<string, int>(StringComparer.InvariantCultureIgnoreCase);
for (int x = 0; x < line.Length; x++)
{
var caracter = line[x].ToString();
if (diccionario.ContainsKey(caracter))
{
diccionario[caracter] ++;
}
else
{
diccionario.Add(caracter, 1);
}
}
foreach (var entrada in diccionario)
{
Console.WriteLine($"El caracter {entrada.Key} apareix {entrada.Value} vegades");
}
}
Notes:I don't need to compare to count characters anymore, so I don't need to declare the variable comparador.When I find a character, I check if it's already in the dictionary. If so, it increases the value. If he's not there, I add it with courage. 1.Separation of concernsYour code handles three operations:Access and read the fileProcess the character stringsShow the result to the userIf in the future you will need to process multiple files, or maybe process the user input etc... you will want to separate these operations.If in the future you will need to make this code work on a website or on a form, etc... you should separate these operations.Even, simply if you want to change the language of the software, you should not have to modify the code that is dedicated to counting characters.Of course, this is an academic job, and these things are not going to happen (or maybe if, depending on the teacher). However, it is something you will find in many ways (commonly with the database) if you get to work as a programmer.So I suggest you separate those operations. What is the criterion? Simple: Isolate the code that is responsible for interacting with external systems (e.g. file system, console) in such a way that it communicates with the rest of the software by exchanging values (e.g. text strings).This also makes the code easier to test. For example I can try the code that counts characters without using a file.Another advantage is that it facilitates dividing the work between several programmers. Thus one can be responsible for displaying the messages to the user, another for working with files and another for processing the text. Maybe it doesn't sound like an equitable division, but it's a way of splitting that prevents them from "those blinds" and it's - assuming everyone knows what they do - it's faster than leaving all the work one person does.I'm only interested in being able to test the code to count characters (to be able to put an answer confident that it works), so I' going to extract it. And I'm gonna limit myself to that. This is the code:while ((var line = sr.ReadLine()) != null)
{
var diccionario = ContarCaracteres(line);
foreach (var entrada in diccionario)
{
Console.WriteLine($"El caracter {entrada.Key} apareix {entrada.Value} vegades");
}
}
// ...
static Dictionary<string, int> ContarCaracteres(string line)
{
var diccionario = new Dictionary<string, int>(StringComparer.InvariantCultureIgnoreCase);
for (int x = 0; x < line.Length; x++)
{
var caracter = line[x].ToString();
if (diccionario.ContainsKey(caracter))
{
diccionario[caracter] ++;
}
else
{
diccionario.Add(caracter, 1);
}
}
return diccionario;
}
I've tried the Contar codeFeatures. I can say it works.AddendumFlxtr has an important point (apart from Linq): We can take the chain and convert it to capital letters (with ToUpper or ToUpperInvariant) and then convert it to a character arrangement (with ToCharArray) so that we can compare the characters directly without need StringComparer.This solution is more efficient (because we don't have to use ToString character by:static Dictionary<char, int> ContarCaracteres(string line)
{
var diccionario = new Dictionary<char, int>();
foreach (var caracter in line.ToUpperInvariant().ToCharArray())
{
if (diccionario.ContainsKey(caracter))
{
diccionario[caracter] ++;
}
else
{
diccionario.Add(caracter, 1);
}
}
return diccionario;
}
