Brian Cole

Glen Chamisa wrote:It seems even when I use BI_RLE4, I get the same issue. However, with the other string types, BI_RGB and BI_BITFIELDS the program runs fine. Can someone explain this cos I don't understand the problem.

I don't know a whole lot about the BMP format, but the javadoc for javax.imageio.plugins.bmp.BMPImageWriteParam says, "If the specified compression scheme is not compatible with the type of image being written then the IOException will be thrown by the BMP image writer." It sounds like this is happening.

You might want to try calling getCompressionTypes() on your BMPImageWriteParam and examine what it returns. I can't say for sure that this would be helpful, but it might be.

peter m hayward wrote:can you expand upon the Never do I/O in a renderer

Because I/O is slow and renderers are intended to be fast. And even if I/O weren't slow, it would still be wasteful to call createImageIcon() more than once per image. And a renderer can and will be called many many times for each item in your list.

Mr. Camick's suggestion to make type of your ComboBox be a compound object that holds both text and an icon is sound. That's a good solid way to write your program.

Here's another way, where the type of the ComboBox can remain a simple String: public class MappedIconRenderer extends DefaultListCellRenderer {     private Map<?, ? extends Icon> map;          public MappedIconRenderer(Map<?, ? extends Icon> iconMap) {         map = iconMap;     }     @Override     public Component getListCellRendererComponent(JList<?> list, Object value, int index, boolean isSelected, boolean cellHasFocus) {         Component c = super.getListCellRendererComponent(list, value, index, isSelected, cellHasFocus);         setIcon(map.get(value)); // might be passing null as the Icon, which would be fine         return c; // or: return this; (c and this are the same object)     }          private static final long serialVersionUID = 1L; }

This way you load all your images up front, like this: Map<String, Icon> iconMap = new HashMap<>(); iconMap.put("hardback", createImageIcon("file:///C:/images/hardback.gif")); iconMap.put("paperback", ...); ... bindings.setRenderer(new MappedIconRenderer(iconMap));

peter m hayward wrote:can you expand upon the Never do I/O in a renderer

Because I/O is slow and renderers are intended to be fast. And even if I/O weren't slow, it would still be wasteful to call createImageIcon() more than once per image. And a renderer can and will be called many many times for each item in your list.

Mr. Camick's suggestion to make type of your ComboBox be a compound object that holds both text and an icon is sound. That's a good solid way to write your program.

Here's another way, where the type of the ComboBox can remain a simple String:
public class MappedIconRenderer extends DefaultListCellRenderer { private Map<?, ? extends Icon> map; public MappedIconRenderer(Map<?, ? extends Icon> iconMap) { map = iconMap; } @Override public Component getListCellRendererComponent(JList<?> list, Object value, int index, boolean isSelected, boolean cellHasFocus) { Component c = super.getListCellRendererComponent(list, value, index, isSelected, cellHasFocus); setIcon(map.get(value)); // might be passing null as the Icon, which would be fine return c; // or: return this; (c and this are the same object) } private static final long serialVersionUID = 1L; }
This way you load all your images up front, like this:
Map<String, Icon> iconMap = new HashMap<>(); iconMap.put("hardback", createImageIcon("file:///C:/images/hardback.gif")); iconMap.put("paperback", ...); ... bindings.setRenderer(new MappedIconRenderer(iconMap));

Paul Clapham wrote:    setLayout(new GridLayout(4, 1));This creates a layout with 4 rows and 1 column. Since you want 2 columns then the second parameter ought to be 2, not 1. And the first parameter ought to be however many rows you want.

At the risk of being irrelevant to the original poster, allow me to elaborate on this a bit.

If you look at how GridLayout is actually implemented, then you will see that new GridLayout(4, 1) will actually lay out with 4 rows and with as many columns as necessary. The cols argument is essentially ignored. (If you doubt, the javadocs for the GridLayout.setColumns() method explains this. Or just run a few quick examples and you'll see.)

Because of this, when I use GridLayout I always specify zero either for rows or for cols, just to remind myself how GridLayout works. So I would write either new GridLayout(n, 0) or new GridLayout(0, n). In this case, in which a two-column grid seems to be desired, I would use new GridLayout(0, 2).

Three things:
1) This was cross-posted to stackoverflow. http://stackoverflow.com/questions/47528136/

2) It is a mistake to append System.getProperty("line.separator"). You should simply be appending "\n" because that's how all Swing text components represent it internally. Any line-ending conversions that might be needed happen elsewhere, such as in textArea.write().

3) If the body of the DocumentListener were { Unregister the listener; Append the newline (directly, not via invokeLater) ; Register the listener back again; } then you would not see the looping behavior. That's not a good way to actually implement it, but as an example it can help demonstrate what's going on. A good way would be via DocumentFilter, which was designed exactly for this kind of thing.

Piet Souris wrote:Well, I've used the'direct drawing method' myself a couple of times, with excellent result,

Was it due to the same book, or something else? I ask because I've always thought that JComponent.getGraphics() was (and should be) an obscure method that wasn't widely known.

Campbell Ritchie wrote:Do they also teach people to use addActionListener(this)? Another

I'm not as disapproving of this as some people. Yes, it's ugly, but to some extent so is creating an inner or anonymous class to be the listener.
If one can presume that lambda expressions are allowed (they have been available for 3½ years now) then I would suggest addActionListener( e -> something );

But I think we can agree (?) that the ActionListener part of this code isn't as bad as the getGraphics part of this code. I wouldn't even say using addActionListener(this) is incorrect, but merely clunky.

Ryan O'Mara wrote:we have been using the book Java for students

Java for Students (6th ed), Douglas Bell and Mike Parr, 2010, Pearson Education Canada
Java for Students (5th ed), Douglas Bell and Mike Parr, 2006, Prentice Hall

I found a table of contents for the 5th edition on the web. It lists chapter 3 as "Using graphics methods," which is pretty darn early in the book. It's before chapters on methods (chapter 5), conditionals (7), loops (8), classes (9), and inheritance (10).

There are also downloadable code examples and, sure enough, chapter three includes this example (and others which are similar).import java.awt.*; import java.awt.event.*; import javax.swing.*; public class DrawCircle extends JFrame     implements ActionListener {     private JButton button;     private JPanel panel;     public static void main(String[] args) {         DrawCircle frame = new DrawCircle();         frame.setSize(400, 300);         frame.createGUI();         frame.setVisible(true);     }     private void createGUI() {         setDefaultCloseOperation(EXIT_ON_CLOSE);         Container window = getContentPane();         window.setLayout(new FlowLayout() );         panel = new JPanel();         panel.setPreferredSize(new Dimension(300, 200));         panel.setBackground(Color.white);         window.add(panel);         button = new JButton("Press me");         window.add(button);         button.addActionListener(this);     }     public void actionPerformed(ActionEvent event) {         Graphics paper = panel.getGraphics();         paper.drawOval(0, 0, 100, 100);     } }There are plenty of things to quibble about here. It's not creating the GUI on the EDT*, which was normal before 2004, so maybe not a ding against this 2006 book. There's no real reason to extend JFrame here, especially since inheritance isn't until chapter 10, but no biggie. I guess I can accept implementing ActionListener (as opposed to having to declare another class, nested or otherwise) in chapter 3 of an introductory book. Why give the JPanel a preferred size when you could put it in the CENTER of a BorderLayout? Why is it calling the content pane "window" when that term usually means something else? And why use a JPanel when a JComponent would do just as well?

But calling panel.getGraphics().drawOval() in the ActionListener is pretty bad. Perhaps the authors knew it was bad and were just trying to construct a graphical program simple enough to appear in chapter 3 of an introductory book, but I don't think that's a valid excuse for this code. For one thing, this code is drawing this circle only transiently. If the program's frame is redrawn for any reason (perhaps because it was temporarily obscured by another window) then the circle will disappear until the user clicks the button again. So not only was this code not written the way Sun/Oracle says it should be, which should be enough for most of us, but this program flat out doesn't behave correctly.

It's a shame that there are so many books out there that are wrong. They do a disservice to the students who try to use them. And it's no fun for those trying to give good advice on forums such as CodeRanch.

footnote: *My book doesn't create the GUI on the EDT either. However it was published in 2002, before the swing single threading rule quietly changed.

Jagadesh Kumar Subramanian wrote:
I have used the InputMap and ActionMap functions to register all the arrow keys

Presuming you're using one of the typical LnFs, there were already actions assigned to the arrow keys in the InputMap/ActionMap.

Are you saying you added some custom actions to the ActionMap and reassigned the arrow keys to them? (It seems like that should work.)

I do not know how to call the default or system action function from the actionPerformed function of the Action class.

I'm not sure exactly what you're asking. But even if you have added custom actions to the ActionMap, the stock actions do remain in the ActionMap. That's one way to get to them.

Ryan O'Mara wrote:

Rob Camick wrote:

First you need to learn how to do custom painting. You should NOT be using the getGraphics() method of the panel.

The proper way to do custom painting is to extend the JPanel and override the paintComponent(...) method.

Hi and thanks for replying.

I have been coding for around 2 months and haven't been taught another way

Oh dear. Does this mean that some kind of formal course has recommended using JComponent.getGraphics() like this? That would be unfortunate.

There's more than one way to implement drag and drop, so it's hard to say anything without more information. Did you do it by calling setTransferHandler() on your JList big panel?

IIRC, a single item drags out of a JList as plain text, but multiple items drag out of a JList as a <ul>...</ul> chunk of HTML, so you have to handle those two cases differently. Both of those may be different from how your old code was dragging out of little JPanels.

You may have to post an SSCCE to get any specific help.

Marius Semeon wrote:I tried setting preferred/max/minSize on the JTextArea and it didn't effect it, but I guess it's because it was inside a JScrollPane so it's the outer container that dictates it. When I set the sizes on the JScrollPane it all worked fine.

What a JScrollPane does is allow you to put a large object inside something (the JScrollPane's JViewport) that is (or at least can be) smaller in size. So the JTextArea itself remains unconstrained, and it's not quite correct to say that the JScrollPane is constraining it.

[note: No big deal, but Dimenension should be corrected to Dimension in your code.]

The Mac LookAndFeel thinks it's doing you a favor. A "real" app that followed Apple's guidelines would necessarily have the centered tabs, so that's all you can do.

If you're ok with switching to another LookAndFeel, then you should be able to align tabs as you wish.

Piet Souris wrote:Just tested this at home. The code works without any problem, no matter how hard I tried by resizing the panel as fast as I could. The 'paint' method is not affected in any way by the Thread executing the 'run' method. This is new to me, and if I had a chance to test this earlier, I wouldn't have asked.

Thanks for the retraction.

It's funny that you mention resizing the panel, because I think the bounce() method does have a bug related to that. If you size the panel big and wait for a ball to get at the very bottom (or very right) then quickly resize to very small, the code in bounce() doesn't handle this correctly and the ball can get 'lost' with coordinates outside the visible area. I realized this when I wrote it, but I decided to keep the code simple and not address it. (Again, it's easy to be cavalier in simple demos.) I did try to "lose" a ball this way several times but failed each time, but I think it's possible if things go just right.

Tony Docherty wrote:Also shouldn't the 'visible' variable be declared as volatile as it is being used by two different threads.

Sorry, I talked about the two threads but I didn't talk about volatile. Yes, I think it might be smart to declare x, y, and visible to be volatile. It didn't occur to me but it should have. Without volatile I don't think there is any guarantee that the EDT thread will ever see changes to those variables, though of course in practice it does eventually.