Sign in to follow this  
Saint Retro

Site for Code Discussion

Recommended Posts

Saint Retro    349

Hi,

 

I have often wanted to ask opinion on the basic exercises I do as part of me learning C# but I don't really feel such basic questions fit on this site so I was wondering if there were any other good places for such a thing.  

For example, I wrote a simple program to reverse a name, it works but I think it could be done tidier and was looking for feedback but as this site is mainly game dev (obv) I don't feel it has such a place.  

If I'm wrong of course please let me know and I'll happily ask away!

Share this post


Link to post
Share on other sites
Saint Retro    349

Ok great, well I made a simple console program in C# to ask your name and repeat it or reverse it.  Although it achieves what I want it to (after a few tries) I was wondering if anything is glaringly bad or could be made in a lot less code.  

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace NameReverser
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Please enter your name");
            string name = Console.ReadLine();
            Console.WriteLine("Would you like your name repeated(1) or reversed(2)");
            string choice = Console.ReadLine();

            if (choice == "1")
            {
                Console.WriteLine("How many times to repeat?");
                string numberofTimes = Console.ReadLine();
                int times = int.Parse(numberofTimes);

                for (int i = 1; i <= times; i++)
                  Console.WriteLine(i + " - " + name);                         
            }

            else if (choice == "2")
            {
                char[] array = name.ToArray();
                Array.Reverse(array);
                Console.WriteLine(array);                
            }
            else
            {
                Console.WriteLine("I do not understand, exiting...");
            }

            Console.ReadLine();            
        }
    }
}

Share this post


Link to post
Share on other sites
Saint Retro    349

Forgot to add my clause that I haven't got to switch statements yet (although I'm aware of what they are used for).  I was actually a bit frustrated at just how difficult I found making such a simple program.  The main issue I had was forgetting how to do the parse bit.  I'm happy if there is nothing awful there anyway!

Share this post


Link to post
Share on other sites
Saint Retro    349

Ah well I suppose I would add a conditional to make sure that times >=1.  As to the other thing I'm not sure where I would even start with that, I know I have the name stored in the array and I know I can find the size of it but that's a challenge for tomorrow I think :)  

Incidentally, I'm fairly sure when I was learning years ago I was told that a string was just a char array so I was surprised that I had to convert a string to an array in the first place.  I may have remembered this wrong of course (was reading C & C++)

Share this post


Link to post
Share on other sites
CC Ricers    1491

Ah well I suppose I would add a conditional to make sure that times >=1.  As to the other thing I'm not sure where I would even start with that, I know I have the name stored in the array and I know I can find the size of it but that's a challenge for tomorrow I think smile.png

 

You already have the chars put in an array. I think from there it should be obvious on how to go through it backwards wink.png

Edited by CC Ricers

Share this post


Link to post
Share on other sites
jpetrie    13137
am I thinking it's something to do with a for loop?

 

 

Potentially.

 

Try to break a problem you don't know how to solve down into smaller and smaller components until you know how to solve one of them. For example, first think about simply printing the string backwards. How would you do that? Well, first you'd print the last character. How would you get the last character? Okay, then how would you get the character before that? And before that? See a pattern? 

 

Once you're printing the string backwards successfully, you've solved 80% of your original problem and now simply need to tackle the smaller problem: how do I store each character somewhere instead of printing it?

Share this post


Link to post
Share on other sites
Deflinek    1619

About the only thing I think you could improve on is using a "switch" statement instead of a nest of if and else.  It might make sense to use routines.  Have one for repeating and one for reversing and call those from the switch.

 

I think it could go unnoticed but it is actually quite important thing that you can improve in your code.

 

so after line 18 you may have something like:

if (choice == "1")
{
    RepeatName(name)
}
else if (choice == "2")
{
    ReverseName(name)
}
else
{
    // print error message
}

Pay attention to your method names so it will be obvious what it does even when you get back to your code after few months and you are good.

It is also ok to create a method that is used only once as it provides code clarity if you name it properly.

Edited by Deflinek

Share this post


Link to post
Share on other sites
slayemin    6099

This may be completely ridiculous to point out for this exercise, but you are also not checking the "number of times" variable to make sure that its actually a number. What if someone types in "asdf" instead? Then the Int.Parse() method would crash your program. So, you'd want to put that in a Try/Catch type block. I know you know to put in numbers, but if you ever build software to be used by other people, then you can't trust them to put in the correct types of input. Maybe its silly for this exercise, but its good to start getting into the habit of thinking about how people could intentionally mess up your code / program.

Share this post


Link to post
Share on other sites
CC Ricers    1491

Thanks for all the replies, the patience of people on here is something else!  CC Ricers, am I thinking it's something to do with a for loop?

 

Yes, exactly. I recently took a web developer coding test which had me do something similar, output a string backwards without using built-in or third party functions. I eventually got it but I threw away the test code now.

Share this post


Link to post
Share on other sites
Bacterius    13165

This may be completely ridiculous to point out for this exercise, but you are also not checking the "number of times" variable to make sure that its actually a number. What if someone types in "asdf" instead? Then the Int.Parse() method would crash your program. So, you'd want to put that in a Try/Catch type block.


Actually, what you almost certainly want is to use TryParse() instead of anticipating exceptions. And I didn't respond earlier, but this piece of code is actually the perfect use case for a switch statement, which is absolutely not premature optimization, even if there are only three cases (not sure how you derived this conclusion). A switch statement actually isn't just syntactic sugar to compress an if-else-if chain into less lines of code, rather it tends to convey the idea of mapping a given input to one or more actions, as opposed to an if-else statement which simply takes either one action or the other based on the evaluation of a conditional (completely orthogonal notions). In fact you could easily argue that not replacing that if-else chain by a switch statement is premature obfuscation in that you are emulating the switch's behaviour instead of, you know, using an actual switch.

(apologies if this goes beyond the level of feedback expected in the For Beginners subforum, but since this is a code review question and marked as such I felt it was appropriate)

Share this post


Link to post
Share on other sites
Saint Retro    349

So I tried to do this on paper and I now totally understand why paper coding is encouraged.  As per Josh Petrie, I broke it down on paper and this made it a lot clearer.

 for (int i=0; i < name.Length; i++)
                Console.WriteLine(array[i]); 

The above printed name "GC" as G

                                                     C.

Obviously this isn't right.  I tried to work it out on paper again and realised that I just needed the reverse of that loop and decided to use Console.Write to keep it on one line.  This resulted in the below:

for (int i=array.Length; i >= 0; i--)
                Console.Write(array[i]);  

This resulted in an error.  I very quickly remembered after seeing the variable watch why it went wrong.  My name "Giancarlo" is 9 characters but I know an array indexes from 0 so I was trying to print a non existent element of the array[9], so I simply had to append a -1 to the initiator.  Got there in the end.

This made me go through a range of feelings, first of questioning my own intelligence and whether or not I'm cut out for programming if it's taken me this long to do something so simple.  Finally to happiness when I finally did it.  Considering there are much bigger challenges ahead I'll be interested to see how I cope as by nature I'm quite a defeatist but I know I have to change this for this discipline.

Edited by Saint Retro

Share this post


Link to post
Share on other sites
Lactose    11469


first of questioning my own intelligence and whether or not I'm cut out for programming if it's taken me this long to do something so simple.

Don't get too discouraged; we've all been there.

 

Regardless of how many years or even decades of experience you eventually have, you will still curse at your brain for being stupid and failing to do someting so simple.

Guaranteed.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this