Click here to Skip to main content
15,886,963 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
I have designed a simple elevator program on a windows forms where I have an array of vertical picture boxes on the same horizontal axis. The program accepts input from the user which is the floor number the elevator is currently on. The floors are the array of picture boxes, the index of the bottom most floor is equivalent to index 7(elevators.length-1) and the index of the top most floor is 0. Now to a user of the program, the bottom most floor is the ground floor and the top most floor is the last floor upwards. The logic I have has a timer that updates the floor the user is currently on by coloring the rest of the floors white and the current floor Chartreuse. The floor the user is in is updated by a timer object with an interval of 2 seconds. I have a problem making the code work for the reverse movement where the elevator is at the top and coming back down towards the ground floor. Below is the full code for reproducing the issue on your machine, Thanks for your time and effort

What I have tried:

C#
<pre>

namespace Elevator
{
	public partial class Form1 : Form
	{
		//declare a array of picture boxes of length 8
		private PictureBox[] elevators = new PictureBox[8];
		//define the vertical distance
		private int vertical_space = 20;
		//define an initial point for the first picture box
		private Point point = new Point();
		//store the index of the current elevator position where the user is
		private int currentIndex;
		//index of the target elevator
		private int targetIndex;
		//timer to move the elevator
		private System.Timers.Timer elevetorMover = new System.Timers.Timer(2000);
		//which direction is the user moving in
		private bool upwards = true;
		public Form1()
		{
			InitializeComponent();
			//enable user defined title bar
			SetStyle(ControlStyles.UserPaint, true);
			Paint += Form1_Paint;
			//register an on elapsed event hander for the timer
			elevetorMover.Elapsed += ElevetorMover_Elapsed;
			Console.WriteLine("launching a form");
		}

		private void ElevetorMover_Elapsed(object? sender, System.Timers.ElapsedEventArgs e)
		{
			
			//check the direction of the elevator
			if (upwards)
			{
				//increment the current index by 1
				currentIndex -= 1;
				Decolorize(currentIndex);
				if (currentIndex ==0)
				{
					//stop the timer
					elevetorMover.Stop();
				}
			}
			else
			{
				//this means the elevator is moving downwards
				currentIndex += 1;
				Decolorize(currentIndex);
				if(currentIndex == 7)
				{
					elevetorMover.Stop();
				}
			}

		}
		void Decolorize(int spareIndex)
		{
			for(int i = 0; i < elevators.Length; i++)
			{
				if (i == spareIndex)
				{
					elevators[i].BackColor = Color.Chartreuse;
				}
				else
				{
					elevators[i].BackColor = Color.White;
				}

			}
		}
		private void Form1_Paint(object? sender, PaintEventArgs e)
		{

		}

		protected override void OnPaint(PaintEventArgs e)
		{
			base.OnPaint(e);

		}
		private void Form1_Load(object sender, EventArgs e)
		{
			//disable button 2
			button2.Enabled = false;
			//set a scroll state for the form
			AutoScroll = true;
			//make the background of the form black
			BackColor = Color.DarkSlateGray;
			//create an array of vertical 
			var x = Location.X + Width / 2;
			var y = 10;
			point = new Point(x, y);
			//declar the dimensions of the pciture box array
			var width = 200;
			var height = 200;
			//programatically create vertical picture boxes using a loop
			for (int i = 0; i < elevators.Length; i++)
			{
				var picBox = new PictureBox();
				//set the size
				picBox.Width = width;
				picBox.Height = height;
				picBox.BackColor = Color.White;
				//set the mode
				picBox.SizeMode = PictureBoxSizeMode.StretchImage;
				if (i == 0)
				{
					//this is the first picture box, set its location to point
					picBox.Location = point;
					//add the control to the form
					Controls.Add(picBox);
				}
				else
				{
					//update the y co ordinate and add a new picture box to the control
					point.Y += picBox.Height + vertical_space;
					picBox.Location = point;
					Controls.Add(picBox);
				}
				elevators[i] = picBox;
			}

		}

		private void button2_Click(object sender, EventArgs e)
		{
		}

		private void button1_Click(object sender, EventArgs e)
		{
			//show an input dialog for the user to enter the floor number
			var floorNumber = Microsoft.VisualBasic.Interaction.InputBox("Enter floor number (1-8)", "Elevator Info");
			//check for invalid user input
			if (Int32.TryParse(floorNumber, out var res))
			{
				//user entered a number, it needs to be  equal or less than the maximum elevator index
				if (res < 0 || res > elevators.Length)
				{
					MessageBox.Show("Invalid Floor Number");
				}
				else
				{
					//highlight the picture box selected
					currentIndex = 8-res;
					switch (res)
					{
						
						
						case 1:
							//declorize the rest of the boxes
							for (int i = 0; i < elevators.Length; i++)
							{
								if (i == 8 - 1)
								{
									continue;
								}
								elevators[i].BackColor = Color.White;
								
							}
							//highlight the bottom most picture box
							elevators[8 - 1].BackColor = Color.Chartreuse;
							button2.Enabled = true;
							break;
						case 2:
							//declorize the rest of the boxes
							for (int i = 0; i < elevators.Length; i++)
							{
								if (i == 8 - 2)
								{
									continue;
								}
								elevators[i].BackColor = Color.White;
							}
							elevators[8 - 2].BackColor = Color.Chartreuse;
							button2.Enabled = true;
							break;
						case 3:
							//declorize the rest of the boxes
							for (int i = 0; i < elevators.Length; i++)
							{
								if (i == 8 - 3)
								{
									continue;
								}
								elevators[i].BackColor = Color.White;

							}
							elevators[8 - 3].BackColor = Color.Chartreuse;
							button2.Enabled = true;
							break;
						case 4:
							//declorize the rest of the boxes
							for (int i = 0; i < elevators.Length; i++)
							{
								if (i == 8 - 4)
								{
									continue;
								}
								elevators[i].BackColor = Color.White;
							}
							elevators[8 - 4].BackColor = Color.Chartreuse;
							button2.Enabled = true;
							button2.Enabled = true;
							break;
						case 5:
							for (int i = 0; i < elevators.Length; i++)
							{
								if (i == 8 - 5)
								{
									continue;
								}
								elevators[i].BackColor = Color.White;
							}
							elevators[8 - 5].BackColor = Color.Chartreuse;
							button2.Enabled = true;
							break;
						case 6:
							for (int i = 0; i < elevators.Length; i++)
							{
								if (i == 8 - 6)
								{
									continue;
								}
								elevators[i].BackColor = Color.White;
							}
							elevators[8 - 6].BackColor = Color.Chartreuse;
							button2.Enabled = true;
							break;
							case 7:
							for (int i = 0; i < elevators.Length; i++)
							{
								if (i == 8 - 7)
								{
									continue;
								}
								elevators[i].BackColor = Color.White;
								button2.Enabled = true;
							}
							elevators[8 - 7].BackColor = Color.Chartreuse;
							break;
						case 8:
							for (int i = 0; i < elevators.Length; i++)
							{
								if (i == 8 - 8)
								{
									continue;
								}
								elevators[i].BackColor = Color.White;
							}
							elevators[8 - 8].BackColor = Color.Chartreuse;
							button2.Enabled = true;
							break;
					}
				}
			}
			else
			{
				MessageBox.Show("Invalid input, try again in a moment");
			}
		}

		private void button2_Click_1(object sender, EventArgs e)
		{
			//show a message box for the target floor
			var targetFloor = Microsoft.VisualBasic.Interaction.InputBox("Which floor would you like to go to?", "Elevator prompt");
			if(Int32.TryParse(targetFloor, out var res))
			{
				if (res < 0 || res > elevators.Length)
				{
					MessageBox.Show("Invalid Floor Number");
				}
				else
				{
					//update the target index
					targetIndex = 8-res;
					//get the difference between the target floor and the current floor to know the direction
					var diff = res-currentIndex;
					//if the diff is positive then the elevator is moving upwards
					if(diff>0) {
						MessageBox.Show("Moving the elevator upwards");
						//start the elevator mover component
						elevetorMover.Start();
					}
					if (diff == 0)
					{
						MessageBox.Show("You cannot remain in the same floor");
					}
					if(diff<0) {
						upwards = false;
					}
					MessageBox.Show(diff.ToString());
				}
			}
		}
	}

}
Posted
Updated 10-Apr-23 5:29am
Comments
0x01AA 10-Apr-23 11:12am    
On a first glance:
a.) In button2_Click_1 in case it goes down you don't start the elevator in case if(diff<0)
b.) In the same method make sure to always set the direction upwards to the correct value. This wheter it goes up or down.
Tim the Gamer 10-Apr-23 11:15am    
Yes the boolean upwards tells the program which direction he is moving in by getting the difference between the target floor and the current floor indices. If it's positive then the elevator is moving upwords else it's moving downwards
0x01AA 10-Apr-23 11:21am    
Yes but you don't set it to upwards when last direction was downwards.
And again: You don't start the levator in case it goes downwards.
0x01AA 10-Apr-23 11:24am    
try this:if(diff>0) {
upwards= true;
elevetorMover.Start();
}
else if (diff < 0){
upwards= false;
elevetorMover.Start();
}
else{
MessageBox.Show("You cannot remain in the same floor");
}
Tim the Gamer 10-Apr-23 11:25am    
That should only be true if the position of the elevator is equal to the index of the ground floor becsus if the elevator is on the ground floor then it can't move downwards. That's a nice suggestion, I will add that logic to the program.

1 solution

First off, try to make your code reflect the real world: if you have an elevator, it starts from the ground floor and goes up to the top floor - so make your array match that: index 0 is the ground floor, Length - 1 is the top floor.
That way, your code is much easier to read - particularly when your comments refer to "incrementing index" and the code actually subtracts!

Then make your names reflect their actual use: currentIndex is not as meaningful as "currentFloor".

Stop using hardcoded numbers as well:
if(currentIndex == 7)
only works if there are 7 floors - so if you need to work with a "taller building" later, your code is harder to maintain and that enas it's less reliable.

You don't say what the problem actually is, just
I have a problem making the code work for the reverse movement where the elevator is at the top and coming back down towards the ground floor.
so it's difficult to work out exactly what you see as a problem - and we can't run that code in isolation without some work to guess what you have done in terms of the picture boxes on your actual form that I don't really want to bother setting up a project for.

So ... instead I'm going to suggest that you use the debugger: put a breakpoint on the first line of the ElevetorMover_Elapsed method and watch what actually happens while your code is running. That should give you an idea why it's doing what you see.

But I can't help thinking you have overcomplicated this: An array, a targetFloor and a currentFloor would remplace the upward, res, and diff variables and make it easier to see:
Pseudocode
if targetFloor == currentFloor you are done.
else if targetFloor > currentFloor you go up. 
else you go down.
Why complicate it? You also don't need to test what floor you are on to decide what to paint: decolorise the current floor before you move, then colourise it after - the large switch with a lot of duplicated code is redundant (and it means that again, you can work with different building heights without changing your code!)
 
Share this answer
 
Comments
Tim the Gamer 10-Apr-23 11:46am    
The first picture box is near the top of the form hence thats the top most floor level to a user who doesn't understand coding. Making bottom most picture box the first index will result in inconsistencies because that floor will be out of the forms bounds
OriginalGriff 10-Apr-23 12:02pm    
Why? What does the user know about your code internals?
And just put them in a Panel with scroll options if they don't fit on the screen ...
Tim the Gamer 10-Apr-23 12:05pm    
Okay but can I use a dictionary instead? I loop through the picture boxes and add the last picture box to be the first element of my dictionary object then I can implement the logic accordingly?
OriginalGriff 10-Apr-23 12:46pm    
Why a dictionary? You are using sequential indexes, so an array or List is a more logical choice. and "move up" / "move down" with an array or List is a lot more logical than with a dictionary which doesn't have am inherent "sequence" only what is implied by it's Key values!
Again, that's overcomplicating things.

Why are you looping through your picture boxes at all? If they are in a Panel which represents a building, they are already in an indexable collection: the Panel.Controls. Why not use that instead? All you have to do is organise your picture boxes in the panel so the ground floor is at the bottom, and the top floor is at the top - that doesn't affect the order in Controls collection, just where they are displayed on the form.
Tim the Gamer 10-Apr-23 12:52pm    
Okay giving it a try

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900