views:

87

answers:

6

In C#, I have an Array of MenuItem. I'm trying to swap the two Objects in index 2 and index 3 of the array without success using the code below:

MenuItem Temp = Items[2];  
Items[2] = Items[3];  
Items[3] = Temp;  

There must be a reason why the second and third lines aren't working in C# which I may not understand yet. Is anyone able to clarify it a bit more? Do I have to go deeper and swap each property in the objects individually?

Edited - Sorry. Looks like I made a mess of the code when trying to clean it up for posting. Corrected now.

The actual code is :

MenuItem TempButton = MenuItems.Items[SelectedButton.CountId];  
MenuItems.Items[SelectedButton.CountId] = MenuItems.Items[SelectedButton.CountId + 1];  
MenuItems.Items[SelectedButton.CountId + 1] = TempButton;  

MenuItems.Items is an array of MenuItem

Looking at the Watch I have placed on MenuItems.Items, nothing happens on Line 2 or 3.

The MenuItems.Items property has get and set functions, which may be causing the issue... Will investigate further...

+1  A: 

You are settings Items[2] to Temp, which was Items[2] to begin with, so you are effectively not doing anything. I don't know what SelectedButton.CountId is supposed to be.

But if you just want to swap indices 2 and 3, you can do this:

Item Temp = Items[2];
Items[2] = Items[3];
Items[3] = Temp;
NullUserException
+1  A: 

Is SelectedButton.CountId = 2? if so I would try this:

Item Temp = MenuItems.Items[2];  
MenuItems.Items[SelectedButton.CountId] = MenuItems.Items[3];  
MenuItems.Items[3] = Temp;  

Note the last line has a 3 in it.

This would be clearer:

Item Temp = MenuItems.Items[SelectedButton.CountId];  
MenuItems.Items[SelectedButton.CountId] = MenuItems.Items[3];  
MenuItems.Items[3] = Temp;  
Hogan
+1  A: 

I have no idea what SelectedButton.CountId is supposed to be but you're putting Temp right back in the same slot it was to begin with. And MenuItems.Items seems to be a totally different collection from Items.

string[] items = { "one", "two", "three" };
string temp = items[1]; // temp = "two"
items[1] = items[2]; // items[1] = "three"
items[2] = temp; // items[2] = "two"

// items is now
// { "one", "three", "two" }
Josh Einstein
+1  A: 

try:

Item Temp = Items[SelectedButton.CountId];   
Items[SelectedButton.CountId] = MenuItems.Items[SelectedButton.CountId+1];   
Items[SelectedButton.CountId+1] = Temp;  

This should swap in a bubble fashion

TerrorAustralis
A: 

Hi guys,

Solved the issue. I think I complicated the problem a bit too much.

MenuItems.Items was a property with get/set functions that returns/sets a private ArrayList.

I created a function in the class for MenuItems which swapped the indexes in the private ArrayList. (which used standard swapping code similar to what I tried and what everyone has mentioned in their replies.)

Thanks for everyone's help.

tbv
@tbv: Are *you* the developer responsible for the `Items` property? If so I **highly** recommend you change it, as this behavior is only going to confuse other developers if they ever look at this code. (I'm assuming you're not, but the fact that you say "I created a function in the class..." leads me to question that assumption.)
Dan Tao
@DanTao Had a chat with co-workers who are responsible for the class. The private ArrayList contains data that is not meant to be accessible from outside. In fact, it could be disastrous if the ArrayList is modified without certain precautions which member functions like AddItems(), RemoveItems() etc. do. The property was intended to return a copy of the internal data at that instance, and as you suspected, it goes out of scope immediately.
tbv
Modification of the data should only be via member functions, so I'm actually of the opinion that the property should really be turned into a function which returns the relevant data. I think this would prevent other developers from assuming they could (try) assigning data to it...
tbv
@tbv: Yes, the list should not be exposed this way at all, particularly not with a `set`! Expose the behavior you want to allow: `Add`, `Remove`, etc. If you want to provide a way to get the items without permitting direct manipulation of the `ArrayList`, something like `GetItems` which is explicitly documented as returning a *copy* of the items would be a much better call than an `Items` property, in my opinion.
Dan Tao
+1  A: 

I remember running into a similar source of confusion some time ago, with the DataRow.ItemArray property. This property was extremely counterintuitive for the very same reason that the Items property in your example seems so odd.

What was ultimately so confusing was that the property was designed to be copied from and assigned to, just like you normally would with a field of value type (like int, double, etc.). That is, to change the element at index 2, this would not work:

row.ItemArray[2] = "New Value";

The above code would essentially copy the values from the row into a new array, take that copy and set the value at index 2 to "New Value," and then the new array would immediately be out of scope. The way this property was supposed to work was:

object[] items = row.ItemArray;
items[2] = "New Value";
row.ItemArray = items;

Very counterintuitive, in my book (note to library developers: don't do this). But it sounds like this is probably the problem behind the issue you were seeing with your code.

In other words, I think the swapping code you have (now) is correct. The problem lies with whoever had the bright idea of making that Items property behave as if it's a value field.

Dan Tao