Question Could anybody assist me with this, I've attempted it but surely there's a better way right.

NewProgrammer61711

New member
Joined
Nov 7, 2021
Messages
1
Programming Experience
Beginner
The Victoria line has these stations in the following order: Brixton, Stockwell, Vauxhall, Pimlico, Victoria, Green Park, Oxford Circus, Warren Street, Euston, King's Cross, Highbury & Islington, Finsbury Park, Seven Sisters, Tottenham Hale, Blackhorse Road and Walthamstow Central.
Write a program that allows the user to input two stations. It should then output the number of stops between the two stations. The station names can be input in any order, but you can make the problem easier by only inputting stations in the correct order if you need to.

Attempt:
C#:
using System;
using System.Collections.Generic;
namespace arrayslist
{
    public class Program
    {
        public static void Main(string[] args)
        {
            //Main program
            int two = 0;
            int one = 0;
              
            string[] stop = new string[16];
            stop[0] = "Brixton";
            stop[1]= "Stockwell";
            stop[2] = "Pimlico";
            stop[3] = "Vauxhall";
            stop[4] = "Victoria";
            stop[5]= "Green Park";
            stop[6] = "Oxford Circus";
            stop[7] = "Warren Street";
            stop[8] = "Euston";
            stop[9] = "King's Cross";
            stop[10] = "Highbury & IslingtoN";
            stop[11]= "Finsbury Park";
            stop[12]= "Seven Sisters";
            stop[13] = "Tottenham Hale";
            stop[14] = "Blackhorse Road";
            stop[15] = "Walthamstow Central";
          
            Console.WriteLine("What is your first stop?");
            string st1 = Console.ReadLine();
            Console.WriteLine("What is your second stop?");
            string st2 = Console.ReadLine();
          
            if (st1 == "Brixton" && st2 == "Stockwell")
            {
                Console.WriteLine(Array.IndexOf(stop, "Brixton"));
            // (Line 46)    one = Console.ReadLine();
                                
            }
 
Last edited by a moderator:
Firstly, I have formatted your code snippet for readability. Please always format your posts for maximum readability, as it helps us to help you. One of the most important factors when reading code is maintaining indenting.
 
As for your code, there's a lot that could be improved and a lot wrong, so let's go step by step. One of the most important things to learn with regards to programming is the various general principles that you should always strive to implement in the specific code that you write. One overriding principle is that you should always use clear, descriptive names for everything. You violate that immediately here:
C#:
namespace arrayslist
That suggests that you have named your project arrayslist, which is a terrible name. It does give some indication that arrays are used in the project but no one would have any idea what this project is really about from that name. What I would suggest to anyone is that they start by choosing a "company name" and start all their projects with that, then use a name that actually describes the project. In my case, my middle name is Philip with one L and I long ago decided that, if I ever had my own business, it would be named Wunnell. If I was creating then project, I would name it Wunnell.VictoriaLineStationCounter or something like that.
 
Next, what's going on here?
C#:
int two = 0;
int one = 0;
You have declared two variables with meaningless/deceptive names and you never use them. Don't write code for no reason. If there is a purpose to declare a variable then declare it with a name that describes that purpose and use it for that purpose. Those names don't describe a purpose and the variables aren't used, so there obviously is no purpose. I'm guessing that you were intending to use them for the indexes of the first and second station, in which case they should be named firstStationIndex and secondStationIndex, not one and two. You may think that it is obvious to you what they are for right now, in such a short program, but it will not be obvious to other people reading your code and it will not be obvious to you if you put this code down and come back to it later. We've all fallen into that trap so you should aim to avoid it from the start.
 
Next, while this is not wrong:
C#:
string[] stop = new string[16];
stop[0] = "Brixton";
stop[1]= "Stockwell";
stop[2] = "Pimlico";
stop[3] = "Vauxhall";
stop[4] = "Victoria";
stop[5]= "Green Park";
stop[6] = "Oxford Circus";
stop[7] = "Warren Street";
stop[8] = "Euston";
stop[9] = "King's Cross";
stop[10] = "Highbury & IslingtoN";
stop[11]= "Finsbury Park";
stop[12]= "Seven Sisters";
stop[13] = "Tottenham Hale";
stop[14] = "Blackhorse Road";
stop[15] = "Walthamstow Central";
it is unnecessarily verbose. There are various ways to create and populate arrays and this is not the best option in this case. If you know all the elements at the start then you should just provide the elements at the start and let the size be inferred from them:
C#:
string[] stop = new []
                {
                    "Brixton",
                    "Stockwell",
                    "Pimlico",
                    "Vauxhall",
                    "Victoria",
                    "Green Park",
                    "Oxford Circus",
                    "Warren Street",
                    "Euston",
                    "King's Cross",
                    "Highbury & Islington",
                    "Finsbury Park",
                    "Seven Sisters",
                    "Tottenham Hale",
                    "Blackhorse Road",
                    "Walthamstow Central"
                };
You could put all the elements on a single line if you wanted, but I tend to use multiple lines for three or more values. Again, you should strive for maximum readability.

Note that the type of the array is not specified on the right and neither is the size. Both are inferred from the literal elements provided. You don't even need to specify the type on the left. If you provide a literal string array then you can declare the variable using var and it will be inferred to be type string[].

Finally, the name stop is terrible, once again. Like all names, you should use something descriptive, and names for arrays and collections should generally be pluralised. In this case, stations or stationNames would be an appropriate variable name:
C#:
var stationNames = new [] {"Brixton", ..., "Walthamstow Central"};
 
Next is the logic for the user input:
C#:
Console.WriteLine("What is your first stop?");
string st1 = Console.ReadLine();
Console.WriteLine("What is your second stop?");
string st2 = Console.ReadLine();
Again, terrible variable names. There's more though. The assignment says that you can force the user to enter the stations in order but, ideally, shouldn't. There's really no need to do so. You should ask the user to enter the name of a station. The wording you have used is not accurate. You're asking the user for two station names and you're telling them the number of stops between them. You're not asking them for a stop. The wording ought to reflect that. I would suggest something like:
Please enter the name of a station on the Victoria Line
and:
Please enter the name of another station on the Victoria Line
It doesn't hurt you to provide a few extra words to make things as clear as possible.

You now need to think about what the user might enter and how you are going to handle the various scenarios. For instance, what if, at the first prompt, they enter:
What are you going to do? Some beginner homework assignments allow you to assume that the user will enter valid input but yours doesn't seem to indicate either way. As it stands, even if the user enters a valid station name but with different casing to your array, you will not be able to find that station. Are you allowed to assume valid input and, if not, how exactly are you going to detect it and handle it? Ideally, I would suggest that you keep prompting the user for the first station until they enter something valid. As far as casing is concerned, you can either write code that is case-insensitive or else you probably ought to start by displaying the list of station names to the user so they know what the exact names are.
 
Finally, you have the real meat of the assignment: the actual calculation. What you have makes little sense:
C#:
if (st1 == "Brixton" && st2 == "Stockwell")
{
    Console.WriteLine(Array.IndexOf(stop, "Brixton"));
    // (Line 46)    one = Console.ReadLine();

}
You are told to output the number of stops between the two stations but your code makes no attempt to do that. Firstly, looking for specific pairs of stations is crazy. That would require an if statement for every possible pair. You don't even care about what the stations are anyway; only the number of stops between them. This is simple mathematics. You need to determine the indexes in the array of the two stations entered and then simply subtract one index from the other. The result will be negative if the user entered the stations in the wrong order, so you can simply take the absolute value and report that.

One thing I didn't mention previously is that you might also consider what to do if the user enters the same station twice. Again, I would keep prompting them for a different station until they provided one, with the same station being considered invalid input.
 

Latest posts

Back
Top Bottom