HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Project Euler GUI for Problem #1 through #11

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
problemprojectforeulerthroughgui

Problem

I've been working on the Project Euler problems, so I created a GUI and a Solver class for all the code I've been creating. I'd like to share it with you guys and to get general feedback on my coding and how I chose to organize the class.

You can also give feedback on how I solved the Project Euler problems if you'd like. Be warned, I did nothing fancy. I just brute forced everything with nested loops, pretty much.

I think I'm proudest of my solution to #11, which is taking a giant array of ints, and multiplying n consecutive numbers together (columns, rows, diagonals) to find the greatest product. My solution can handle n consecutive numbers (I didn't use a magic number), and it checks every row, column, NW to SE diagonal, and NE to SW diagonal.

Problems 5 and 10 run the slowest. Problem 5 takes 5 seconds, problem 10 takes 3 seconds. The rest run in less than 1 second.

```
using System;
using System.Windows.Forms;
using System.Diagnostics;

namespace ProjectEuler
{
public partial class GUI : Form
{
public GUI()
{
InitializeComponent();
}

private void GUI_Load(object sender, EventArgs e)
{
ProblemList.Items.Add("Problem 1");
ProblemList.Items.Add("Problem 2");
ProblemList.Items.Add("Problem 3");
ProblemList.Items.Add("Problem 4");
ProblemList.Items.Add("Problem 5");
ProblemList.Items.Add("Problem 6");
ProblemList.Items.Add("Problem 7");
ProblemList.Items.Add("Problem 8");
ProblemList.Items.Add("Problem 10");
ProblemList.Items.Add("Problem 11");
}

private void ProblemList_Click(object sender, EventArgs e)
{
string problemSelectedText = (string)ProblemList.SelectedItem;

int problemNum = Convert.ToInt32((problemSelectedText).Substring(8, problemSelectedText.Length - 8));

ProblemBox.Text = "Calculating solution. Please wait.";

Solution

As long as the helpers are neutral and atomic, I can't see a problem there.

IMO, overall your code is readable, and for the most part you have refactored the reusable helper functions out nicely.

-
Solve11 would be one exception requiring refactoring - there are a lot of different, and repeated concerns handled here which can be DRY'd out into helpers.

-
Often regarded as stylistic, but many favour the use of the conditional (ternary) operator over an if-branch when assigning the same variable, or returning a value of the same type from two branches. e.g. both branches of this (repeated) code assign to currentProduct:

if (num == 0)
 {
     currentProduct = currentCellValue;
 }
 else
 {
     currentProduct *= currentCellValue;
 }


And can be abbreviated to:

currentProduct = (num == 0) // Parens aren't actually needed
     ? currentCellValue
     : currentProduct * currentCellValue;


-
At some point you may find it unwieldy to put all of your Euler solutions in one class / one file. You can adopt a strategy like breaking them up into partial classes, or separate classes entirely (e.g. collate Prime related puzzles vs Palindromes etc).

-
Many of the methods (like SolveX and helpers like SumNPrimes) are simply mathematical functions and do not access and member fields and do not need this and can be marked static.

-
For brevity, you can simply echo the result of a boolean expression, i.e. replace:

if (NumToTest % 2 == 0)
 {
     return true;
 }
 else
 {
     return false;
 }


With simply:

return NumToTest % 2 == 0;


  • C# offers a number of shortcuts for compile-time collection initialization, including for Dictionaries. Also, although your code will probably run just the once, it is usually a good idea to extract immutable data initialization out of methods, and initialize it once-per-process, e.g.



private static readonly IReadOnlyDictionary Problems = new Dictionary
{
     {1, "foo"},
     {2, "bar"},
     ...
};


  • Possibly pedantic, but you could also encapsulate the "problems" to include the function itself in the Dictionary. You would however need all the SolveX functions to return the same type (and weak types like object or the dynamic type should be avoided). AFAIK all the Euler solutions are integral, so long seems reasonable, (Although from memory you might also need to make use of BigInteger at some point when long is insufficient)



private static readonly IDictionary>> Problems 
= new Dictionary>>
{
     {1, new Tuple>("foo", Solve1)},
     {2, new Tuple>("bar", Solve2)},
     ...
};


You can then replace the unwieldy (and unencapsulated) switch statement reasoning over the Dictionary's keys:

switch (problemNum)
 {
      case 1:
           MyAnswer = Solve1();
           break;


To a simple Dictionary lookup:

var problem = Problems[problemNum];


Which you can then invoke

var solution = problem.Item2();


And if you don't trust the user input for problemNum, you could be more defensive with Dictionary.TryGetValue

(I've been slack with the Tuple which results in the horrid Item1 / Item2, but you could improve this with a custom strong class to replace the Tuple)

Also, since C# is starting to assume many of the features of FP languages, you might also want to look at using LINQ, which will again reduce the number of lines of code (especially around adding items to lists, filtering, grouping, projecting new values etc).

Possibly more advanced, but another really useful tool for solving Euler type problems are generators. C# has the yield keyword, which allows for lazy generation (and iteration) of items. (e.g. one of your helper functions can lazily generate a IEnumerable of Prime Numbers, which your solutions can reason over with LINQ).

Good luck with your Project Euler endeavors!

One last thought - you might also want to formulate your SolveX methods as Unit Tests (e.g. with NUnit). The idea here is that once you have the correct answer, that you can then Assert the correctness of your solution, and thus encourage you to do further refactorings and enhancements to a solution. You've already made use of a Stopwatch, so you are already thinking along these lines :)

Code Snippets

if (num == 0)
 {
     currentProduct = currentCellValue;
 }
 else
 {
     currentProduct *= currentCellValue;
 }
currentProduct = (num == 0) // Parens aren't actually needed
     ? currentCellValue
     : currentProduct * currentCellValue;
if (NumToTest % 2 == 0)
 {
     return true;
 }
 else
 {
     return false;
 }
return NumToTest % 2 == 0;
private static readonly IReadOnlyDictionary<int, string> Problems = new Dictionary<int, string>
{
     {1, "foo"},
     {2, "bar"},
     ...
};

Context

StackExchange Code Review Q#124894, answer score: 6

Revisions (0)

No revisions yet.