Project Euler 42 - Finding triangle numbers

Here is my solution for Project Euler Problem 42, which is described in the following:


The \$n^{th}\$ term of the sequence of triangle numbers is given by, \$t_n = ½n(n+1)\$; so the first ten triangle numbers are:


\$1, 3, 6, 10, 15, 21, 28, 36, 45, 55, \ldots\$


By converting each letter in a word to a number corresponding to its alphabetical position and adding these values we form a word value. For example, the word value for SKY is \$19 + 11 + 25 = 55 = t_{10}\$. If the word value is a triangle number then we shall call the word a triangle word.


Using words.txt, a 16K text file containing nearly two-thousand common English words, how many are triangle words?

I know it's messy and could be much improved, so please feel free to pull it apart.

I also noticed other solutions I've seen didn't seem to need to use the dictionary I typed out manually.

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;
using System.Collections;
using System.Diagnostics;

namespace ProjectEuler
{
class Program
{
// global variable to count triangle number
public static int numberTriangleWords;
// p42. tn = ½n(n+1)
// A=1, B=2, etc. case insensitive.

static void Main(string[] args)
{
Stopwatch sw = new Stopwatch();
sw.Start();
int maxScore = 0;
Dictionary myDictionary = GetDictionary();

StreamReader sr = new StreamReader("p042_words.txt");
string[] str = sr.ReadLine().Split(',');
foreach (string s in str)
{
int totalScore = 0;

for (int i = 1; i maxScore)
maxScore = totalScore;
}
//Console.WriteLine("this is the word score: {0} for the word {1} ", totalScore, s);

if (IsWordTriangleNumber(totalScore))

Nice effort and nice to see someone willing to be critiqued.

Prefer var

This is a matter of taste but where type is clearly discernible from the right hand side, use var. So instead of:

Stopwatch sw = new Stopwatch();

You can use:

var sw = new Stopwatch();

Note that sw is still strongly typed as Stopwatch instance.

Meaningful Names

First I'd suggest using meaningful names. So a line such as:

foreach (string s in str)

could become more readable by:

foreach (var word in words)

Note use of var above.

Maxscore isn't used

Therefore it is dead code and should be removed.

Scoping

The comments says numberTriangleWords is a global variable. First of all, it isn't. Rather it has class-level scoping. More importantly, it does not need to be at the class-level. You only reference numberTriangleWords inside the Main method so it should be local to that Main.

Explicit access modifier

I encourage use of public or private, especially when I deal with entry level developers as I tend to wonder if they forgot to put it in. i.e. did they mean for this method to be private be omitting the modifier, or did they forget? When I see it explicitly, there is no doubt.

Dictionary Not Needed

The dictionary is not needed. Granted its not very big either. But it doesn't help with performance either.

IsWordTriangleNumber

As is your IsWordTriangleNumber does 2 things: (1) it builds an array, and (2) checks to see if the current wordVal is in the array.

What could help with performance is building the list of triangle numbers ONCE rather than for each word. This again would be scoped just to the Main method. This also helps to separate concerns. Building the array and checking for something in the array are now 2 different things.

Also this block of code:

if (triangleNumbers.Contains(wordVal))
{
    return true;
}
else
    return false;

Can be simplified to:

return triangleNumbers.Contains(wordVal);

Update: You can explore with whether the collection of triangle numbers should be an array, a list, SortedSet, Dictionary, etc. I don't think the collection should have a value for 0, or rather my GetScore code below would need to be changed. Still, a 0 should not be included otherwise you risk accidentally counting the @ and backtick characters as triangle numbers, when they should not be. The aforementioned characters immediately precede A and a.

Instead of Dictionary

One option instead of the dictionary would be:

private static int GetScore(string word)
{
    var total = 0;
    foreach (var letter in word)
    {
        total += GetScore(letter);
    }
    return total;
}

private static int GetScore(char letter)
{
    // Restricted to 26 letters of English alphabet.
    // Ignores surrogate pairs.
    var start = Char.IsLower(letter) ? 'a' : 'A';
    var score = (int)letter - (int)start + 1;
    if (score  26) return 0;
    return score;
}

Which could then clean up the heart of the code:

var triangleNumbers = BuildTriangleNumbers();

foreach (string word in words)
{
    var score = GetScore(word);

    if (triangleNumbers.Contains(score))
    {
        numberTriangleWords++;
    }
}

Which is far less messy. I leave it to you to create BuildTriangleNumbers method to return the appropriate type. The nice thing with the var above is that the code should work whether you return an array or a list.

StackExchange Code Review Q#98037, answer score: 4