C
C#16mo ago
katze

❔ Making collatz conjecture code more efficient

hello, i recently tried to make a C# program that runs the collatz conjecture on random numbers and saves the conjecture to a .txt file; however: its really inefficient and i was wondering if anyone had any suggestions on how to improve it?
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;
using System.Text;
using System.Linq;

namespace COLLATZCONJECTURE
{
class Program
{
static void Main(string[] args)
{
Random randgen = new Random();
UnicodeEncoding encoder = new UnicodeEncoding();

Console.ReadLine();

start:

var values = new List<string>();

long selectedNum = Convert.ToInt64(randgen.Next());
long x = selectedNum;

string localPath = (@"D:\COLLATZ\" + Convert.ToString(x) + ".txt");

if (File.Exists(localPath)) {
goto start;
}

Console.WriteLine("Input: {0}", selectedNum);

while (x > 0 && x != 1)
{
if (x == 1)
{
Console.WriteLine("END --- {0}", Convert.ToString(selectedNum));
break;
}
if (x % 2 == 0)
{
x = x / 2;
Console.WriteLine(Convert.ToString(x));

}
else if (x % 2 != 0)
{
x = (x * 3) + 1;
Console.WriteLine(Convert.ToString(x));

}
values.Add(Convert.ToString(x));
}

values.Add("END -- INPUT: " + Convert.ToString(selectedNum));

File.WriteAllLines(localPath,values);

// Console.WriteLine(values);
// Console.ReadKey();

goto start;
}
}
}
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;
using System.Text;
using System.Linq;

namespace COLLATZCONJECTURE
{
class Program
{
static void Main(string[] args)
{
Random randgen = new Random();
UnicodeEncoding encoder = new UnicodeEncoding();

Console.ReadLine();

start:

var values = new List<string>();

long selectedNum = Convert.ToInt64(randgen.Next());
long x = selectedNum;

string localPath = (@"D:\COLLATZ\" + Convert.ToString(x) + ".txt");

if (File.Exists(localPath)) {
goto start;
}

Console.WriteLine("Input: {0}", selectedNum);

while (x > 0 && x != 1)
{
if (x == 1)
{
Console.WriteLine("END --- {0}", Convert.ToString(selectedNum));
break;
}
if (x % 2 == 0)
{
x = x / 2;
Console.WriteLine(Convert.ToString(x));

}
else if (x % 2 != 0)
{
x = (x * 3) + 1;
Console.WriteLine(Convert.ToString(x));

}
values.Add(Convert.ToString(x));
}

values.Add("END -- INPUT: " + Convert.ToString(selectedNum));

File.WriteAllLines(localPath,values);

// Console.WriteLine(values);
// Console.ReadKey();

goto start;
}
}
}
5 Replies
Doombox
Doombox16mo ago
first, please do not use goto jump statements, like basically ever beyond that there's not a whole lot more efficiency to be gained, it's a fairly fixed algorithm with not much wiggle room you should only really need while(x > 1) though, but as an optimisation that's frankly irrelevant
katze
katze16mo ago
yea i tried that and when i did it, it just decided "nope, im gonna become an infinite loop" so i just didnt fix it lol what should i use instead?
Doombox
Doombox16mo ago
define a method, and use return statements to break out where necessary if you need to create an infinite loop, wrap that method in while(true) (or with a break-out condition if you want to exit it at any point)
public static string collatz(string y)
{
if (y == null)
{
return null;
}

int x = int.Parse(y); //x is my "n"
var results = new StringBuilder();
results.Append(x.ToString());
int largest = x; //keep track of biggest number

// the algorithm
// the redundancies (like x==1.. x!= 1) are part of troubleshooting :/
while (x > 1)
{
if (x % 2 == 0)
{
x = x / 2;
if (x > largest)
{
largest = x;
}
if (x != 1)
{
results.Append(" " + x.ToString());
}
if (x == 1)
{
results.Append(" " + x.ToString());
results.Append(" largest number was " + largest.ToString());
return results.ToString();
}
}

if (x % 2 != 0)
{
if (x == 1)
{
results.Append(" " + x.ToString());
results.Append(" largest number was " + largest.ToString());
return results.ToString();
}
x = (3 * x) + 1;
if (x > largest)
{
largest = x;
}
results.Append(" " + x.ToString());
}
}
return results.ToString();
}
public static string collatz(string y)
{
if (y == null)
{
return null;
}

int x = int.Parse(y); //x is my "n"
var results = new StringBuilder();
results.Append(x.ToString());
int largest = x; //keep track of biggest number

// the algorithm
// the redundancies (like x==1.. x!= 1) are part of troubleshooting :/
while (x > 1)
{
if (x % 2 == 0)
{
x = x / 2;
if (x > largest)
{
largest = x;
}
if (x != 1)
{
results.Append(" " + x.ToString());
}
if (x == 1)
{
results.Append(" " + x.ToString());
results.Append(" largest number was " + largest.ToString());
return results.ToString();
}
}

if (x % 2 != 0)
{
if (x == 1)
{
results.Append(" " + x.ToString());
results.Append(" largest number was " + largest.ToString());
return results.ToString();
}
x = (3 * x) + 1;
if (x > largest)
{
largest = x;
}
results.Append(" " + x.ToString());
}
}
return results.ToString();
}
I can't be bothered to implement it myself so you could take some lessons from this implementation I found on StackOverflow seems to be broadly similar to yours in general though
katze
katze16mo ago
thanks
Accord
Accord16mo ago
Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.