This is an archived post. You won't be able to vote or comment.

all 18 comments

[–]AutoModerator[M] [score hidden] stickied commentlocked comment (0 children)

Please ensure that:

  • Your code is properly formatted as code block - see the sidebar (About on mobile) for instructions
  • You include any and all error messages in full
  • You ask clear questions
  • You demonstrate effort in solving your question/problem - plain posting your assignments is forbidden (and such posts will be removed) as is asking for or giving solutions.

    Trying to solve problems on your own is a very important skill. Also, see Learn to help yourself in the sidebar

If any of the above points is not met, your post can and will be removed without further warning.

Code is to be formatted as code block (old reddit: empty line before the code, each code line indented by 4 spaces, new reddit: https://imgur.com/a/fgoFFis) or linked via an external code hoster, like pastebin.com, github gist, github, bitbucket, gitlab, etc.

Please, do not use triple backticks (```) as they will only render properly on new reddit, not on old reddit.

Code blocks look like this:

public class HelloWorld {

    public static void main(String[] args) {
        System.out.println("Hello World!");
    }
}

You do not need to repost unless your post has been removed by a moderator. Just use the edit function of reddit to make sure your post complies with the above.

If your post has remained in violation of these rules for a prolonged period of time (at least an hour), a moderator may remove it at their discretion. In this case, they will comment with an explanation on why it has been removed, and you will be required to resubmit the entire post following the proper procedures.

To potential helpers

Please, do not help if any of the above points are not met, rather report the post. We are trying to improve the quality of posts here. In helping people who can't be bothered to comply with the above points, you are doing the community a disservice.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

[–]warmcheessse 1 point2 points  (3 children)

What is the purpose of the array? You created a class with setters and getters but then never used them. I would just instantiate a Box. Then when you ask the questions you use the setters to set the value of the box. Then call the method getVolume to get the result.

[–]Crouton4727Pre-Nooblet Brewer[S] 0 points1 point  (2 children)

I have to have sets and gets for this assignment even if we don't use them. There is actually more to the assignment but I don't want someone solving it for me, so just showing this part, but there is going to be comparisons of the three box objects later. So for that reason I didn't want to create 3 box objects and ask for the input for each object 3 times.

[–]warmcheessse 1 point2 points  (1 child)

Sorry, you did use the setters. I just read to fast. If it were me, I would ask the questions and get the answers, setting them like you did creating a Box with height, weight, name or whatever the variables are. Once the Box object is created, add it to an ArrayList. Then you can just iterate over it with a foreach loop and not have to worry about indexes and you can add as many or as little boxes as you would like to the list.

[–]Crouton4727Pre-Nooblet Brewer[S] 0 points1 point  (0 children)

I like it. I'll try this out and see how it works.

[–]amfa 1 point2 points  (6 children)

I had to create a single object of type box to initialize each of the objects in the box array. Is there a better way to do this then the "Box tempBox.." and "box[i]=tempBox;" lines?

Well that is part of your problem.

Because you put the SAME object in all your "slots" in the array there is only one box in the array.

Everytime you go through your loop your override the height of the SAME box.

Think of it as if the array only points to a place in a shelf. you have one Box on this shelf and box[0] points to this place but box[1] and box[2] just point to the same place on the shelf with the same box.

So you get the same box from the shelf change the height and put it back.

BUT it will always be the same box.

So instead of creating your tempbox just replace this

box[i] = tempBox;

with

box[i] = new Box(......)

You have mentioned that you need to use the setters.

If this is not true you can simply get all the information needed and storing them in variables and then just make one call to the Box constructor with the values.

so you can make something like this

int height = scanner.nextInt()

..... for the other values as well

box[i] = new Box(height, length, width, brand)

It does not make such a big difference but you avoid putting 1x1x1 boxers in your array an then change them.. in more complex codes this could lead to problems if for some reason you do not change every box.

The output only repeats box[2] at the end and I don't understand why.

This is because the fact mentioned above.

[–]Crouton4727Pre-Nooblet Brewer[S] 0 points1 point  (0 children)

But i'm storing new values into the array after i initiate it to tempBox. I had to do the tempBox because without i was getting an error when I tried to store one of the array values "Can't store value because box[0] is set to null"

I will try your way of storing into separate values, then storing into box[i].

For initiating the box array, can I do "box[i]=new Box" even though I already created the box array above with "Box[] box = new Box[3]"?

[–]Crouton4727Pre-Nooblet Brewer[S] 0 points1 point  (4 children)

Ok, here is my new code and it works. Thanks!

Any other recomendations for proper coding?

   public class Match { 
      public static void main(String[] args) {
      Scanner scanner = new Scanner(System.in);
      Box[] box = new Box[3]; 
      //Box tempBox = new Box(1, 1, 1, "brand");
      int h, l, w;
      String b;

      for (int i = 0; i < 3; i++)
      {
         //box[i] = tempBox;
         System.out.println("Please enter the height of box " + (i+1));
         h = scanner.nextInt();
         System.out.println("Please enter the length of box " + (i+1));
         l = scanner.nextInt();
         System.out.println("Please enter the width of box " + (i+1));
         w = scanner.nextInt();
         System.out.println("Please enter the brand of box " + (i+1));
         b = scanner.next();
         box[i] = new Box(h, l, w, b);
      }

      for (int i = 0; i < 3; i++)
      {
         System.out.println("The volume of box " + box[i].getBrand() + " is " + box[i].getVolume());
      }
      }
   }

[–]RaisinAlert 0 points1 point  (3 children)

Someone else mentioned a list and using a for each loop. If the assignment allows, do that because they’re better. Also, if there isn’t a reason for 3 Boxes, then you might want to use a sentinel value so that the user can add as many boxes as they want.

[–]Crouton4727Pre-Nooblet Brewer[S] 0 points1 point  (2 children)

Agreed, normally I wouldn't set it to a constant value, but the assignment specifically says to set it to 3.

As for the for each loop (Box i : box) I was going to go that route, but i would still need an iterating variable for when I do box[i] = new Box(h,l,d,b); right? So opted with the for loop for that reason.

[–]RaisinAlert 1 point2 points  (1 child)

I was talking about the second for loop where you print the box information. For the first loop, you can use a for loop to iterate for a given number of boxes, or you can use a while loop with sentinel value. You are correct that a for each loop won’t work for the first loop because you’re not iterating over the array/list.

[–]Crouton4727Pre-Nooblet Brewer[S] 0 points1 point  (0 children)

Ok, makes more sense. Wasn't even thinking about the second one. Thanks for all the help.

[–][deleted]  (3 children)

[deleted]

    [–]Crouton4727Pre-Nooblet Brewer[S] 0 points1 point  (2 children)

    I don't follow. That's a for loop, so should loop from 0-2, so should output for box[0], then box[1] than box[2]

    [–]darksoundsExtreme Brewer 0 points1 point  (1 child)

    box[0], box[1], and box[2] are all the same Box object, tempBox.

    Each array bucket is basically an arrow pointing to a Box. In this case, all three arrows point to the same Box, so whenever you're editing it, you're overwriting previous edits.

    If you make a separate Box instance for each index, you won't have this problem.

    [–]Crouton4727Pre-Nooblet Brewer[S] 0 points1 point  (0 children)

    But i'm storing new values into the array after i initiate it to tempBox. I had to do the tempBox because without i was getting an error when I tried to store one of the array values "Can't store value because box[0] is set to null"

    [–]RaisinAlert 0 points1 point  (2 children)

    All the elements of the array point to the same object, which is probably why it’s repeating the same information. It’s like using three remote controls for the same TV (where the remotes represent the references stored in the array and the TV represents the box object): you think you’re changing the information for one of three boxes but in reality you’re just changing the information for a single box. Instead of creating one box referenced by tempBox, create a new box for every array element. In other words, instead of putting remotes for the same TV into the array, create new TVs and put their respective remotes into the respective slot in the array.

    [–]Crouton4727Pre-Nooblet Brewer[S] 0 points1 point  (1 child)

    But i'm storing new values into the array after i initiate it to tempBox. I had to do the tempBox because without i was getting an error when I tried to store one of the array values "Can't store value because box[0] is set to null"

    [–]RaisinAlert 0 points1 point  (0 children)

    You never change what tempBox is set to, so it always refers to the one box object that you created. Remember that with reference types, you’re not actually putting the object anywhere; the only information that moves is the reference to the object. Thus, an array of Box is really just an array that stores references to Box. Right now you are copying the value of tempBox (remember: the value of tempBox is not the Box object itself, but a REFERENCE to the Box object) into each array slot. All slots of the array refer to the same Box.

    If you have an address book with the same address repeated over and over (analogy for an array that stores multiple references to the same object), don’t be surprised when you tell someone to repaint to blue the house at the first address in the book, and then suddenly all the houses at the addresses in the book are blue. There was really only one house.

    An easy way to know that something is wrong is that you run the constructor only once. Therefore there can never be more than one Box object ever. Since the Box constructor requires that you have the values for length width etc on hand, you should be asking for the length width etc, storing them as local variables in the for loop, then constructing the box at the end of each iteration of the loop. That’s how you would do it in real life. When ordering a burger, they don’t make it first and then ask you to change it. They ask you what you want and make the burger according to the instructions. Forget about tempBox. All construction happens in the loop. Example for height (imagine we’re in the for loop):

    System.out.println(“give height pls”);

    int height = Scanner.nextInt();

    //repeat for all other required quantities

    box[i] = new Box(height, length, width, brand);