• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Liutauras Vilda
  • Jeanne Boyarsky
  • Devaka Cooray
  • Paul Clapham
Sheriffs:
  • Tim Cooke
  • Knute Snortum
  • Bear Bibeault
Saloon Keepers:
  • Ron McLeod
  • Tim Moores
  • Stephan van Hulst
  • Piet Souris
  • Ganesh Patekar
Bartenders:
  • Frits Walraven
  • Carey Brown
  • Tim Holloway

Enumerations and Vectors

 
Greenhorn
Posts: 17
3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

I am writing a for class that uses Enumerations to create 4 types of Objects, randomly select a random number of them for a "wallet" Vector, display the contents, shuffle them, display again, and then use exactly 4 methods to print each enum type separately using instanceOf. The "Paper Currency" output should calculate the total value of the Currency as well. I've completed the assignment and it worked, but without calling the getValue() method. I've corrected that, but my inquiry surrounds understanding why it worked the other way.

I cannot use methods outside of the ones used here, however, I can (probably) be more efficient in my approach.

That said, the question I have is in relation to the calculation of the Paper Currency value. I initially tried to call the getValue() method on Object "o" in the currencyPrint() method. This is now commented out. It worked, but I received indication from my Professor that this wasn't correct as I was not using the getValue() method.

Currency enum and getValue() method



currencyPrint() method



I would ask:

1) Why need the getValue() method if my first approach worked?

2) Was there a better way to achieve what I have other than moving the reference BACK to a Currency Object? (I tried casting Object "o" and changing the parameters of getValue() and it did not work)

3) General observations about approach, etc. I know I have no comments (yet).

 
Marshal
Posts: 64710
226
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

C Robinson wrote:. . .  class that uses Enumerations . . . for a "wallet" Vector . . .

Don't use either of those classes, which are both regarded as legacy code. In youir method headings declare the parameter as type List<E> instead.
You are not actually using Enumerations but enumerated types, also called enums. I am not sure your data really fit into an enum. You might have an additional type of card, e.g. work ID card, in which case you would have to change all your code.
There is something wrong with the design of code which uses insteanceof repeatedly. I don't like having a collection of plain simple Objects. There is also something wrong with using the keyword static repeatedly; it suggests you aren't using object‑oriented design.

1) Why need the getValue() method if my first approach worked?

Please work out why you are getting access to the value field at all. Look where you have written the Currency class.
 
Sheriff
Posts: 6042
157
Eclipse IDE Postgres Database VI Editor Chrome Java Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Line 41 is incorrect.  It should be:
or better:
or as Campbell suggested:

Your code is not formatted consistently.  Formatting is not something you do later, do it as you code.  This helps reduce bugs.  Is HowToFormatCode (that's a link).
 
C Robinson
Greenhorn
Posts: 17
3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Thanks for the link and the advice!

As far as:

-Using ArrayList instead
-Not using instanceOf multiple times
-Type of data or whether it fits into an enum
-Using List<E> in the method headings

We did not get to set these parameters for the assignment, these were requirements.

As far as there being "something wrong with the design of code that uses instanceOf multiple times", I'm obviously new at this and wouldn't know what that something is. I'm posting here to learn things, so understanding what exactly is wrong and why is key.

I posted in the Beginners forum as I thought this would be the most appropriate place to get an answer and explanation to the question I asked regarding my specific problem and some advice to help me learn.

You all delivered on the latter, and I appreciate the constructive criticism.

Still hoping for a direct explanation to the former.

Thanks again!

 
Saloon Keeper
Posts: 10311
217
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

C Robinson wrote:1) Why need the getValue() method if my first approach worked?


In this particular case, it's a matter of taste. Some people prefer to access the field because it's less verbose, and it avoids pushing and popping another frame on the call stack. Other people prefer calling the method, because the method represents the true property that the class exposes, and the field is only there to implement the property. It appears your professor prefers the latter approach. For classes/methods that are final, it really doesn't matter that much. Note also that if you want to access the property from another class, you have no choice but to call the method, because fields should be private.

For classes that are not final (enums are implicitly final), the method can be overridden by a sub-class, so then the difference does become important. It requires that you have a clear idea of how you want sub-classes to behave, and you must clearly document this behavior in the base class.

2) Was there a better way to achieve what I have other than moving the reference BACK to a Currency Object? (I tried casting Object "o" and changing the parameters of getValue() and it did not work)


I'm not really sure what you mean by this. What do you mean by "moving the reference back"? How did you try to change the parameters of getValue()? If you meant to ask if there was a better way to access the value property of a Currency object when you have a reference to it of type Object, then the only thing you CAN do it cast it. But as Campbell has pointed out, you want to avoid having to cast, and use generic type parameters in the way they were intended.

3) General observations about approach, etc. I know I have no comments (yet).



As far as there being "something wrong with the design of code that uses instanceOf multiple times", I'm obviously new at this and wouldn't know what that something is. I'm posting here to learn things, so understanding what exactly is wrong and why is key.


Expanding on what Campbell said, normally you would not want to mix collections of different types. But going by the original code, the entire purpose of this exercise is to mix objects of different types and then cast them back at a later time. Realize though that in the real world, you will want to avoid code that like. Using instanceof and casting references is usually an indication that your design is off. In most applications, the only place where you would want to use instanceof and then cast the reference is inside an equals(Object) method implementation.

Another indication that the design is off is that there are a lot of static methods that operate on the same principal object type: Vector<Object>. In object oriented programming, you want to create a class that actually represents a wallet and to which you add instance methods. Let's assume that there is a strong requirement that the wallet must keep track of the ordering of different types of objects. A solution to this problem that doesn't require casting objects is to use the Visitor pattern. Warning, this is advanced material:

Other types of WalletItem, such as Photo, implement visitBy() in a similar way, calling the appropriate method of WalletVisitor. Here's an example of a visitor that implements a cash counting operation:

You might notice that I used int to represent the value of a denomination, and I used BigDecimal to print it. That is because you should NEVER use floating point types such as float and double to represent exact values, such as monetary amounts. When you use floating point types, all arithmetic operations may cause rounding errors. With BigDecimal, you can move rounding operations to the end of the calculation and explicitly specify how the rounding is done.
 
Saloon Keeper
Posts: 3305
146
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
hi Stephan

not easy indeed, but very interesting. I was tinking of a way to make the interface 'WalletVisitor' flexible, i.e. adaptable when new Items become available, Is that possible?
 
Stephan van Hulst
Saloon Keeper
Posts: 10311
217
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Not really, and you also don't want it to be. If a new type of item becomes available, you want the compiler to raise an error when you're trying to compile old code that doesn't take the new item type into account. The best way to do that is by adding an overload to the visitor interface, and the source code of existing operations that implement the visitor MUST then be updated.

It should be clear that the visitor interface should remain fairly static, and therefore you would only use the visitor pattern if the chance of adding new sub-types that can be visited is smaller than the chance of adding new operations (visitor implementations). If you expect new sub-types to be added regularly, then it's better to declare the operations in the polymorphic base type:
 
Marshal
Posts: 6860
470
Mac OS X VI Editor BSD Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stephan, very nice code two posts above.

I just willing to mention a slight improvement which could be made in order to avoid no-args constructor to better convey a result of such wallet's instantiation. In my opinion a static factory method(s) would fit here nicely, i.e.:


So API's use would read as:
 
Stephan van Hulst
Saloon Keeper
Posts: 10311
217
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Sure, except move the parameter checking and the defensive copying to the factory methods. Private constructors may assume that their parameters are validated and copied, to avoid unnecessary validation and object creation. Case in point, when you call Wallet.empty(), you're creating two empty lists, one of which is immediately eligible for garbage collection. On top of that, contains(null) is called unnecessarily.

If for some reason you don't want to do this, then at least use Collections.emptyList() instead of new ArrayList<>() in the empty() factory method.
 
C Robinson
Greenhorn
Posts: 17
3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Thanks again for the support!

I appreciate the explanation Stephan. (And that you managed to right size it to my minimal level of experience; I know that's tough for folks to do sometimes when they have as much experience and knowledge as you all do)

For classes/methods that are final, it really doesn't matter that much. Note also that if you want to access the property from another class, you have no choice but to call the method, because fields should be private.  



My Professor seemed surprised that calling the value would be from the getValue() method would work because, as you say, the "value field is private" He did not think the commented out section of code should increment the "total" variable. Taking a stab at why it does, either:

A) The original Currency enum Object that is created is instantiated with a "dollar-value" when it is instantiated through the constructor. As the reference to it's memory address is passed into the array, wallet Vector in main, wallet Vector in the currencyPrint() method, Object "o" in the for:each loop, and ultimately into the Currency object "value" created in the currencyPrint() method: the "dollar-value" initially assigned to the memory address is still associated with it, so:

this simply references a field that is already associated with that object? But as you say, the field is private, so how would the piece of code above be able to use it to increment the total variable?

If you meant to ask if there was a better way to access the value property of a Currency object when you have a reference to it of type Object, then the only thing you CAN do it cast it. But as Campbell has pointed out, you want to avoid having to cast, and use generic type parameters in the way they were intended.



This is exactly what I meant, and I followed what Campbell was saying, however, with these assignments, whether it is generally accepted convention or not, I believe they are teaching the material in a certain way so we can grasp the concept with the easiest process, correct or not, and then they expand on it later. I find this a lot when new learners ask questions online; a lot of times we get lost in answers of "don't do it that way, this is a better way" and that way is not something we have been taught. It's 100% the correct answer by all accounts, but to a different question.

But going by the original code, the entire purpose of this exercise is to mix objects of different types and then cast them back at a later time. Realize though that in the real world, you will want to avoid code that like.



This expresses an understanding of that. Keep in mind, you all can say "Why would they teach them to do that that way? This is a much better way to do it" while from this end, I'm trying to figure out why the things that they are teaching us work at all.

In object oriented programming, you want to create a class that actually represents a wallet and to which you add instance methods.



This would have been my first approach, but this likely wasn't a requirement to keep learning the concepts less complicated. This is an online class, so if we get lost in the weeds, there is a lot of exchanging emails to try to sort it out?

You might notice that I used int to represent the value of a denomination, and I used BigDecimal to print it. That is because you should NEVER use floating point types such as float and double to represent exact values, such as monetary amounts. When you use floating point types, all arithmetic operations may cause rounding errors. With BigDecimal, you can move rounding operations to the end of the calculation and explicitly specify how the rounding is done



This seems pretty straightforward, I'm surprised we had not learned this yet as I am in the second Java course.

Thanks again everyone for all the support and advice!
 
Liutauras Vilda
Marshal
Posts: 6860
470
Mac OS X VI Editor BSD Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Stephan van Hulst wrote:...except move the parameter checking and the defensive copying to the factory methods. Private constructors may assume that their parameters are validated and copied,


Well, yes, I missed few important details, the idea was to raise the point about factory methods. I think that constructors supposed to take care of validations and not the factory methods.
 
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Great discussion. I especially applaud OP for his search for understanding.

My opinion, which of course may or may not be correct, is that you are being taught this way because the instructors themselves know of no better way to teach or do not realize the kind of problems they are creating for students on their learning journey. I think students should be made aware of the real-world problems of the example code's design.

One thing that probably hasn't been addressed in depth yet (but maybe I missed it) in this thread is why using instanceof in code like this is a bad practice. Using instanceof can be a sign that you are not using polymorphism correctly. Using conditional logic instead of polymorphism leads to code that is more brittle because it will probably need to be changed when new types are introduced. In cases like that, try to refactor the code to replace conditional with polymorphism. The conditional is the "if (someObject instanceof sometype)" check.

You might be astute enough to discern that Stephan's alternative, which uses the Visitor and Double Dispatch pattern to avoid using instanceof, would also need to be changed if new enum types are introduced. That's true, but the change would need to be made in the WalletVisitor, not the code that loops through the wallet. The looping through the wallet code, because it uses double dispatch visitor, will not change no matter how many times you change WalletVisitor to accommodate new enum types. With the approach Stephan suggested, change is now centralized and localized in the WalletVisitor class. "Client code" like the loop that goes through a wallet thus becomes more stable and more resilient to change, which allows you to write unit tests that are also more stable and resilient to change.

It's all about maintainability and to a great extent, testability. In the real world, maintainability and testability are huge factors that determine the cost of software so you want to write code that is as resilient, testable, and maintainable as possible.
 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I also would like to point out a nuance in Stephan's code: the change of name from Currency to Denomination. It may not seem like much but to the trained eye, it makes a huge difference because it has to do with semantics and cognitive load. The code represents denominations (the different values), not the currency (e.g. Dollar, Euro, Peso, Yen, etc.).  Don't take names in code lightly: choosing good names is a major design decision. Poor name choices can lead to code that is confusing and invites bugs. On the other hand, good name choices makes code easier to understand, less likely to be misunderstood and therefore more resilient to bugs.
 
Stephan van Hulst
Saloon Keeper
Posts: 10311
217
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

C Robinson wrote: the field is private, so how would the piece of code above be able to use it to increment the total variable?


The reason is that the private modifier means that only code inside the same top-level class' body is allowed to access the member. The value field is a member of the Currency enum, which is declared inside the D3 top-level class. That means that EVERY piece of code inside the D3 class' body can access it directly. The main() method is inside the D3 class, so it can access the Currency.value field directly.

Another way of thinking about this is that private means "private to the code in this source file", but only if you have no more than one top-level class in your source file.

I believe they are teaching the material in a certain way so we can grasp the concept with the easiest process, correct or not, and then they expand on it later.


I agree that it's better to learn to walk before you learn to run. The reason that you get a lot of strong reactions like "don't do it like this" is because many people never bother to learn beyond what they've learned in courses and textbooks, and it causes no end of headache for people who have to maintain their code when they start writing for businesses.

This would have been my first approach, but this likely wasn't a requirement to keep learning the concepts less complicated.


This is fine, always stick with the requirements of the assignment. But it's good to question them, and supplement your courses by asking questions online.

This seems pretty straightforward, I'm surprised we had not learned this yet as I am in the second Java course.


This is very common in courses and textbooks. It's likely because professors and authors don't want to distract from the lesson they're trying to teach by introducing new types. It's unfortunate though, because it often leads to bad programming habits that cause insidious bugs in real world applications.
 
Stephan van Hulst
Saloon Keeper
Posts: 10311
217
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Great discussion. I especially applaud OP for his search for understanding.


Agreed, and I have awarded them a cow for it.
 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
With regard to how instruction is delivered, I think academia can do better. The authors of "Java by Comparison" showed a great example of an innovative way of teaching students good coding habits by giving students an opportunity to compare their code with code written by professional programmers. This gives them the context in which to consider the consequences of their design decisions.

I liken what instructors like OP's do to a martial arts teacher (yes, again with the MA analogy) who shows his/her students how to punch with their thumb inside their fist. Any practitioner knows that's a good way to break your thumb. The instructor probably knows it's not a good idea but reasons that "They'll figure this out later, I just want to show them the motion and how the mechanics of the arm works." To me, this is totally wrong. If you follow Shu-Ha-Ri, the first stage of shu is to teach/learn form. You won't do well in the later stages of learning if you learn the wrong form from the start.

I have often pointed out that one of the greatest influences in my development as a programmer was one of my very first programming teachers in college. From very early on in the course, he made us think about "elegance" of code and how well-written code (maintainable, readable, and properly organized) should be a top concern. I think this came from his industry experience; other instructors who did not have industry experience didn't emphasize the same point.
 
C Robinson
Greenhorn
Posts: 17
3
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks again for the awesome support!


The reason is that the private modifier means that only code inside the same top-level class' body is allowed to access the member. The value field is a member of the Currency enum, which is declared inside the D3 top-level class. That means that EVERY piece of code inside the D3 class' body can access it directly. The main() method is inside the D3 class, so it can access the Currency.value field directly.

Another way of thinking about this is that private means "private to the code in this source file", but only if you have no more than one top-level class in your source file.



So, if I had created a separate class and main method, the value.value would not have worked?

That makes sense. My previous Professor had us writing classes with their own methods separately, but I moved on to the second class now.

It's likely because professors and authors don't want to distract from the lesson they're trying to teach by introducing new types.



This is also correct as I have overcomplicated some of the code I've written already and this Professor is reeling me back in and keeping me focused on using and understanding how the basics work.

The instructor probably knows it's not a good idea but reasons that "They'll figure this out later, I just want to show them the motion and how the mechanics of the arm works." To me, this is totally wrong.



In this circumstance, I think it may be more the format of the class. It being online, there's really only so much that can be lectured on and the rest should be reading. This is the pain of online classes. This course is lighter on the course load and reading, which I am thankful for, but a lack of grasp of some of the basics may be the impact of that.

I have the textbook, but the order in which the class is teaching subjects is out of order with the textbook, so you can't jump to Chapter 19 without reading Chapters 9-18 as a subject matter like this builds on itself. I'm trying to read the book in order anyway to make sure I dont leave anything behind.

Thanks again for the help. I tell my fellow students that this is a great resource for support.

 
Campbell Ritchie
Marshal
Posts: 64710
226
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

C Robinson wrote:. . . We did not get to set these parameters for the assignment, these were requirements.

I had better come back to this discussion after such a long absence (sorry).

I worry about people who over‑prescribe assignments. It deprives the students of creativity, and as people have told you already, you may find that the better the design, the worse the marks. I agree you*#xa0;should discuss the assignment with the teaching staff, but once it has been issued, it cannot be changed. Not unless you want a lot of formal disputes.

As far as there being "something wrong with the design of code that uses instanceOf multiple times", . . .

Some people have already answered. There are two problems. One is that you are not using an inheritance hierarchy properly. You would have something like[[That would work a lot better. You can now declare your List as List<WalletItem> wallet new ArrayList<>(); or simlar

Second problem: what happens when I find I have stamps in my wallet? Am I going to write a new class and then have to find everywhere I am casting and add this sort of code? This should wake you in a cold sweat at 3am So far, so good. I shall stop now and write another post soon.

By the way: as others have already said, you have been given an interesting and difficult question.
 
Campbell Ritchie
Marshal
Posts: 64710
226
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

About half an hour ago, I wrote:. . . creativity . . .

I have seen what passes for creativity in some students' code It isn't a pretty sight.
 
Campbell Ritchie
Marshal
Posts: 64710
226
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Now, the WalletItem class would give you an inheritance hierarchy, but it won't sort out all the problems. For example cards keys and photos don't usually have a value, but banknotes coins and stamps do. If you try casting, you are liable to get into the position you are put in by the mess that is the ensureCapacity() method; ArrayList has it and Vector has it, but non‑array‑based Lists (e.g. LinkedList) don't. So, if you want to enlarge your 10‑element List to accommodate 1,000,000 more elements, you have to write this sort of horror:-Now we have exactly the same problem. There is also a trimToSize() method with exaclty the same design peculiarity.
You can reduce the problem with a ResizableList interface, in which case you would only need one cast, but that still isn't ideal.

You can reduce the problem elsewhere by making some WalletItem subtypes implement an interface with a getValue() method. You can call it Valuable. And if you think that is a good example of the Java® interface naming convention gone wrong, you haven't seen the arresting name they gave to a new interface describing something that can be used as a constant.

Another post to follow: watch this space.
 
Campbell Ritchie
Marshal
Posts: 64710
226
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
That isn't going to help you because you have been told to use enums, and you can't make enums extend a particular class, nor can you make another class extend an enum. By the way: when you write your enums, if they have a constructor, give that constructor private access, as you correctly did in your first post (). But, I think you can do this sort of thing:Now you can create your List<WalletItem> easily.

I usually like enums, but I am not convnced they are the best tool for this particular task. Remember, if you have a BankNote enum with a TENNER field, everywhere you write BankNote.TENNER, you get the same instance on the same JVM, So your wallet might contain five instances of the same TENNER. I do occasionally see multiple instances of the same note instance in real life, but they are usually counterfeit!

I always find it awkward to try adding methods to subtypes. You can try having a method in a superclass or interface. Maybe if you had the getValue() metho in the WalletItem interface, you could get away without any casting at all. But some types will have to find a workaround possibly returning ZERO. You can write that in the interface if you wish:-Now you only have to override that method in the types with a “real” value. Still not ideal, I don't think.

And how can you go through your wallet by types? Imposible . . . . at least until I write my next post.
 
Campbell Ritchie
Marshal
Posts: 64710
226
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Go through the Java™ Tutorials (that section) and remind yourself of the sort of thing available there. I am thinking of a Map. I am also going home now, but hope to come back in a few hours to expand on my current naughty idea.
 
Campbell Ritchie
Marshal
Posts: 64710
226
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
When you went through the Java™ Tutorials link I gave you yesterday, did you notice the example about counting words&#c3f; That inspired me to suggest you can create a Map matching type to contents. You can create a Map<Class<T extends WalletItem>, List<WalletItem>> using the Class object as the “K” and all the corresponding items collected into a List as the “V”. I think Class objects are immutable, so how do you get the Class object? Easy‑peasy: every object ever seen has a getClass() method.
As you iterate your List, test whether the Map already contains that Class object as a key. If it does, add that particular item to the List returned by the get(...) method. If not, create a new List, add the current item to it, and use put(...) to enter it into the Map.
I don't think you can sensibly sort anything by Class object, but you can use different sorts of Map depending on whether you want to get the different types back by insertion order or not.
You can iterate the Map's key set to find all the different types it contains.

I am not sure I have got the generics right. Nor am I sure whether my naughty idea will go down well. I would prefer a Stream‑based solution, but you might not have seen Streams before. I don't know whether the following code will be correct, but doesn't it look concise:-
 
Piet Souris
Saloon Keeper
Posts: 3305
146
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
hi C,

your topic is one of the top topics of the month June. So, enjoy a cow!
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!