0

I'm trying to create an instance of a class that is used in the constructor of its outer class. Referring to the code below, I need a UserData object but I also need a TimeOnlineInfo object to create it, and I don't see a way of getting TimeOnlineInfo object without first having an instance of UserData because TimeOnlineInfo is not static. I can't make it static though because it needs to access a method from its outer class. Is there anyway I could get this to work or get the most similar effect? I did realize that I could just make the classes static and not save the data directly in the addTime method but i was already halfway through this question and I'm curious to see if there is a way of doing this.

Here is a very simplified version of my code:

class UserData { TimeOnlineInfo timeOnline; public UserData(Object data1, Object data2, Object data3, Object data4, Object data5, TimeOnlineInfo timeOnlineInfo){ this.timeOnlineInfo = timeOnlineInfo; } public class TimeOnlineInfo { private int time; public TimeOnlineInfo(int time){ this.time = time; } public void addTime(int time){ this.time += time; UserData.this.saveData(); } } } UserData userData = new UserData(new UserData.TimeOnlineInfo());//Doesn't work because PlayInfo is not a static class UserData userData = new UserData(userData.new TimeOnlineInfo());//This is just a stupid because i'm using the uncreated object in its own constructor
Improve this question
1
  • I don't think this is a particularly good arrangement, if TimeOnlineInfo and UserData are in a strict 1-to-1 relationship (as they appear to be), why have a separate, publicly visible TimeOnlineInfo class in the first place? There could be reasons for that of course, but from what you posted, the easiest solution would be to get rid of TimeOnlineInfo altogether. Commented Jul 6, 2015 at 15:56

3 Answers 3

Reset to default
2

You've got some general confusion about a few things. Let's start at the beginning.

First, this isn't a constructor.

public void UserData(TimeOnlineInfo timeOnlineInfo){ this.timeOnlineInfo = timeOnlineInfo; }

You meant to drop the void declaration.

public UserData(TimeOnlineInfo timeOnlineInfo){ this.timeOnlineInfo = timeOnlineInfo; }

Second, it doesn't make structural sense to instantiate the outer class with an instance of the inner class, since the only way to get an instance of the inner class is to use the outer class to begin with.

For example, you would have to use new UserData().new TimeOnlineInfo(int) for an instance of TimeOnlineInfo, but that's only:

  • if you elected to create a no-arg constructor for UserData
  • if you really didn't care about the instance of UserData you were getting back

If you really want to keep this design, then consider passing in an int to your construtor instead, so it can be supplied to the instance of TimeOnlineInfo.

class UserData { TimeOnlineInfo timeOnlineInfo; public void saveData() { // stub } public UserData(int value) { this.timeOnlineInfo = new TimeOnlineInfo(value); } public class TimeOnlineInfo { private int time; public TimeOnlineInfo(int time){ this.time = time; } public void addTime(int time){ this.time += time; UserData.this.saveData(); } } }
Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

I would still make the TimeOnlineInfo private :)
Was just a typo, I wrote the whole thing in browser. I guess this is probably the best solution even though it kinda hurts readability. I suppose thats my fault though for coming up with such an odd question
@kmecpp It's not your fault, these sort of problems are difficult to simplify for a post without oversimplifying.
1

First of all, your UserData constructor isn't one.

You've declared it as a void method, so no UserData constructor would compile, parametrized with a TimeOnlineInfo instance.

But this might only be a typo.

Then, if you can implement a parameterless constructor for UserData on top of the existing one, you could use the following idiom:

UserData ud = new UserData(new UserData().new TimeOnlineInfo(42));

However, this seems to indicate a more general problem within your design, as you're basically having to initialize UserData twice to get a usable instance.

The idea here is, you either inject TimeOnlineInfo in a constructor, in which case TimeOnlineInfo is likely to be used by broader scopes than just UserData, or you only use TimeOnlineInfo within UserData, in which case use an empty constructor for UserData and initialize your internal TimeOnlineInfo within it.

Improve this answer

3 Comments

Yes sorry about that, it was a typo. A parameterless constructor doesn't really seem like a good idea because of your reason but i also don't really want to have the need to create an empty, useless, object aside from its sole purpose of creating a nested class.
@kmecpp yes it looks like an ugly workaround. That's why I insist on the last paragraph about the design part.
@kmecpp - it's not just useless; it'll link to the wrong outer object.
0

You could change your constructor to this:

public void UserData(int time){ this.timeOnlineInfo = new TimeOnlineInfo(time); }

You can also make TimeOnlineInfos constructor private to stop others instantiating rogue TimeOnlineInfo instances.

But based on the snippet you posted, I'd probably get rid of TimeOnlineInfo completely.

I've been warned that what I thought was a constructor isn't one. So first things first, never create regular methods with the same name as the class.

The rest still stands though, the problem with this arrangement is that anyone can create a TimeOnlineInfo object, which will trigger UserData.this.saveData() even if it isn't referenced by the UserData it was created with.

So either:

  1. Make the TimeOnlineInfo instantiation a job for UserData.
  2. Get rid of TimeOnlineInfo. (Not feasible if your class is already big, but then again if your class is that big, you probably need to do other things too.)

  3. Make your inner class static and manage the connection explicitly: 

    public void setTimeOnlineInfo(TimeOnlineInfo timeOnlineInfo){ if (timeOnlineInfo.userData != null) { throw new IllegalArgumentException( "TOI already belongs to other UserData" ); } if (this.timeOnlineInfo != null) { this.timeOnlineInfo.userData = null; } timeOnlineInfo.userData = this; this.timeOnlineInfo = timeOnlineInfo;

    }

    public static class TimeOnlineInfo { private int time; private UserData userData;

     public TimeOnlineInfo(int time){ this.time = time; } public void addTime(int time){ this.time += time; userData.saveData(); }

    }

Far from ideal, but it would solve a couple of other problems too.

Improve this answer

6 Comments

That is not a constructor.
@Makoto You're right...pff, That's another thing to fix.
This also isn't really the best idea because UserData already has a crap load of all the parameters it takes to hold all the different data. Also TimeOnlineInfo is more complex of a type than i made it appear to be, its not just going to take one parameter.
static nested class is better.
@biziclop - I was simply agreeing with your point Make your inner class static and manage the connection explicitly
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.