Reputation: 63
I'm trying to create a list of pairs of integers. I think I have the basic idea right but I'm getting an error whenever I try to add a new pair to the list.
Here is my Pair
Class:
public class Pair<L,R> {
private L l;
private R r;
public Pair(L l, R r){
this.l = l;
this.r = r;
}
public L getL(){ return l; }
public R getR(){ return r; }
public void setL(L l){ this.l = l; }
public void setR(R r){ this.r = r; }
}
Here is where I create my new list of pairs:
private ArrayList<Pair<Short,Short>> DominoList = new ArrayList<Pair<Short,Short>>();
And here is an example of where I try to add a new pair to the DominoList
:
DominoList.add(0, Pair<0,0>);
Does anyone see anything blatantly wrong with the way I am doing this? I feel like I'm probably missing something simple, but I cant figure out what's wrong. I feel like I am adding new pairs incorrectly.
Upvotes: 1
Views: 5436
Reputation: 338181
Does anyone see anything blatantly wrong with the way I am doing this?
Your naming could be improved. Using single letter abbreviations is confusing. I assume you mean "left" and "right". If that is what you mean, say that.
Furthermore, if indeed you meant left-and-right, I assume you chose that because you read left-to-right. But to many people reading right-to-left languages such as Arabic, Hebrew, Persian, and Urdu Sindhi, this would not make sense. Better would be something like "leading" & "trailing", or for the common "key" & "value".
This code is wrong, as it says you have a class named L
and a class named R
. But apparently you do not. You are actually storing a pair of Short
objects, so specify Short
.
private L l;
private R r;
public Pair(L l, R r){
…should be…
private Short key ;
private Short value ;
public Pair( Short key , Short value ){
Another naming issue: Java conventions say a class is named with initial-capital letter, while instance variables are named with a lowercase letter in front. So this:
private ArrayList<Pair<Short,Short>> DominoList = new…
…should have a lowercase d
on dominoList
:
private ArrayList<Pair<Short,Short>> dominoList = new…
Another thing about that code snippet immediately above: Usually we use List
or the lefthand side, if the more general interface offers all the behavior your succeeding code needs. This is one aspect of polymorphism, allowing you to swap out ArrayList
for some other List
implementation in the future without changing or breaking any other code using that list.
Map.Entry
I'm probably missing something simple
The simpler approach would be the use an existing class, built into Java. Some folks use either implementation of the Map.Entry
interface as a pair class:
The immutable versions means that neither the key part nor the value part may be replaced with another object. The objects stored as the key or value may themselves be mutable (have state that can be changed such as with setter methods).
Example:
List < AbstractMap.SimpleImmutableEntry < Short, Short > > dominoList = new ArrayList <>( 28 );
dominoList.add( new AbstractMap.SimpleImmutableEntry( 0 , 0 ) );
dominoList.add( new AbstractMap.SimpleImmutableEntry( 0 , 1 ) );
dominoList.add( new AbstractMap.SimpleImmutableEntry( 1 , 1 ) );
dominoList.add( new AbstractMap.SimpleImmutableEntry( 0 , 2 ) );
dominoList.add( new AbstractMap.SimpleImmutableEntry( 1 , 2 ) );
Applying that principle of polymorphism mentioned above, we should define our list as holding the more general Map.Entry
interface.
List < Map.Entry < Short, Short > > dominoList = new ArrayList <>( 28 );
dominoList.add( new AbstractMap.SimpleImmutableEntry( 0 , 0 ) );
dominoList.add( new AbstractMap.SimpleImmutableEntry( 0 , 1 ) );
dominoList.add( new AbstractMap.SimpleImmutableEntry( 1 , 1 ) );
dominoList.add( new AbstractMap.SimpleImmutableEntry( 0 , 2 ) );
dominoList.add( new AbstractMap.SimpleImmutableEntry( 1 , 2 ) );
Granted, some would consider this use of Entry.Map
to be a hack. These class were intended for use within a Map
. But there is no technical reason forbidding you from using the entry class separately.
If you were using these pairs of numbers as a map, the first being the name or identifier for the second, then I would recommend you actually use a Map
implementation instead of a List
. But I suspect you are not.
Domino
classIf you are trying to model domino tiles, you really should be defining your own Domino
class. Not only do you have the pair of numbers to represent, you may have other methods such as isDouble
.
And if you are building a bigger context in which these dominos will appear, such as a game, then having a specific named class makes your code more self-documenting and thread-safe.
Upvotes: 0
Reputation: 1291
You need to create a new instance of your class by inserting the new
keyword before your Pair class as others have mentioned.
So, your code should read:
private final List<Pair<Short,Short>> dominoList = new ArrayList<>();
dominoList.add(new Pair(0, 0));
This has some small changes that I think are worth mentioning.
First, if you are using Java 7 or 8 you don't need to explicitly use generic types in both sides.
Also note that it is good practice to reference the interface in the variable instead of the concrete type. In this case, it is better to use List
than ArrayList
. By doing this, you can change the list implementation later by, say, a LinkedList
that has different performance properties than ArrayList
.
Second, since you are initializing the variable at its declaration, it is also good practice to mark it as final
. By doing that you help the compiler optimize your bytecode and also get some feedback if you are unnecessarily assigning new instances to the variable.
In your specific case, you might also want to look into other collections like Map<K,V>
. You can easily map one integer key to another integer value but be careful about choosing your map implementation because, for example, HashMap
does not allow duplicated keys.
Some of these tips can be found in:
IMHO both are NECESSARY reading for better understanding and coding Java.
Upvotes: 0
Reputation: 9397
In addition to the other answers (which are correct), I'll mention that there is a common pattern in Java of using static constructors rather than new
directly; that is:
public class Pair<L, R> {
// other code
public static <L, R> Pair<L, R> of(L left, R right) {
return new Pair(left, right);
}
}
This lets you use it a little more gracefully:
DominoList.add(Pair.of(0, 0));
There is also a library offering this if you would like to explore it: Apache Commons Lang.
Upvotes: 0
Reputation: 10373
You should call the constructor to create a new Pair instance. Use
DominoList.add(0, new Pair(0, 0));
You said you want pairs of integers. Then you shouldn't use Short
but Integer
instead.
Upvotes: 2
Reputation: 20370
try
DominoList.add(new Pair(0, 0));
You can also use the built-in class AbstractMap.SimpleEntry
as a Pair class rather than a custom class. Other languages have better support for simple tuples, btw.
Upvotes: 0