can experienced java users critique this?

megaspaz · 01-18-2003, 05:50 PM

AtoBtoA.tar.gz
it's my first java program for linux (not including hello world), so i was wondering if you could look at this and tell me how i can improve on the application and my java programming knowledge. the tarball is in distibutal form. but included is the source code and classes and javadocs ( i know the javadocs are prolly not right in the information content). basically, you can ignore the install scripts and other stuff in the top level directory although you can critique that too if you'd like as i'm still a newbie to linux and the install script i wrote only works for me using red hat 7.3. actually the program will install as i'm sure every distro has a /usr/local/ directory. it's just inserting the desktop icon into the kstartmenu that maybe different on different distros. to run the java app, as you prolly already know, you need to specify the classpath which is the full path to the classes directory. to compile it, as you prolly already know, you need to specify the source path which is the full path to the src directory.

Things I'd like to still have done:

Get the progress bar to work on the convertAtoB and convertBtoA classes. right now, i've only got the code written out in convertBtoA and it's commented out cause it doesn't seem to work right. i followed the things in the java tutorial, but still no go.

How to exception handle the convertAtoB and convertBtoA classes. Basically the program when run gets text from a text box and converts it from binary to ascii or from ascii to binary. with a large enough text, there's a chance to run out of memory. i'm trying to figure out how to catch that in java and how to catch any other errors that come by ( a generic catch all type of thing). i'm thinking of catching the errors in the convertAtoB and convertBtoA classes and throwing them back to the calling method which is the button clicks in the main form.

thanks in advance for your time and response.

edit: i think i got the exception handling but not sure if i used the right class. the link now points to the newest version. i'd still like to know how to get the progress bar working right and is there a way to get "hot" keys in a java program? like a button would be labeled "Click Me" and the first "C" would be underlined so you press the key combination alt-c in the program and have it activate that button.
i'm also using sun's sdk version 1.4.1
thanks again.

oulevon · 01-21-2003, 08:29 PM

I think you'd get more comments if you give more details about your program. Nice job though, for your first program. I'll give you a few comments:

You're methods are too long. I'm not sure what your background is, but in java you should abstract things as much as possible. Your methods should really have one job, and if you find they are doing lots of jobs, you should break it up into more methods. Take your time designing the program before you actually start coding - it'll save you a lot of time in the long run. Use UML if you're familiar with it, or even pseudocode will help. For example I'd rewrite your getBinary(String ascii) method like this:

Code:

public String  CharToBits(char c){
		byte b = 1;
		StringBuffer buff = new StringBuffer();
		for(byte j= 7; j>=0; j--){
			if(((b << j) & c) !=0)
				buff.append("1");
			else
				buff.append("0");
		}
		return (buff.toString());
	
}

public String StringToBits(String word){

		StringBuffer buff = new StringBuffer();
		for(int i=0; i<word.length(); ++i){
			char c = word.charAt(i);
			 String hold = this.CharToBits(c);
			 buff.append(hold);
		}
		return(buff.toString());
}

I've broken it up into two methods, and each is really doing only one job. This makes it more readable, maintainable, and it's a lot more reusable. Compare my implementation with yours, it's a fraction of the size:

Code:

public String getBinary (String strAscii) throws Exception {
    String strBinary = new String (); 
    String strBinTemp;  
    String strZero; //string of 0's
    char charElement;
    int quotient;
    int remainder;
    int numOfZeros;
    int index = 0;

    try
    {
      //to do - set the algorithm to convert to binary
      //loop to get characters for processing
      //get a character to process from the input string at index
      for (int i = 0; i < strAscii.length(); i++) {
        //initialize variables
        charElement = strAscii.charAt(i);
        quotient = charElement;
        strBinTemp = "";
        strZero = "";
        while (quotient != 0) {
          //get modulus
          remainder = quotient % 2;
          //get the new quotient
          quotient = quotient / 2;
          //insert it into the buffer string
          strBinTemp += remainder;
        }
        //get # of 0's to stuff to make 8 bits
        numOfZeros = 8 - strBinTemp.length();
        //append zeros to temporary buffer
        for (int j = 0; j < numOfZeros; j++) {
          strZero += "0";
        }
        //concat strZero with temporary buffer
        strBinTemp += strZero;
      
        //laziness
        StringBuffer myTemp = new StringBuffer();
        myTemp.append(strBinTemp);
        myTemp.reverse();
        strBinary += myTemp.toString();
        //pretty format - insert \n into Binary string
        if (0 == ( (index + 1) % 3)) {
          strBinary += "\n";
        }
        index++;
      }
      return strBinary.toString();
    }
    catch (BufferOverflowException e)
    {
      //try to clean out variables
      strBinary = null;
      strBinTemp = null;
      throw e; //throw exception to caller

    }
    catch (Exception e)
    {
      //try to clean out variables
      strBinary = null;
      strBinTemp = null;
      throw e;
    }
  }

I'm not sure why you've declared your method to throw an exception either. I don't think it's necessary. With my implementation, I just fed it a String that was five lines long, and it didn't have any problems. Nice job though, and post anymore questions if you have any.

megaspaz · 01-22-2003, 09:21 PM

thanks for the reply and critique, oulevon. you're definately right, and i think you could pretty much tell i was more worried about what containers i could use to get the job done since this is a new language for me than the overall design. I'll rework it to get it more modular and I'll definately be more conscious of the overall design next time.

thanks again for your help and suggestions. you'll prolly get sick of me as i'll prolly be posting for more help soon.

edit: as far as throwing an exception, i wasn't sure what to really do. i just wanted to be safe. in c++, i think you can get an overflow using the string container and i wasn't sure how java handled strings. plus i wanted to make sure no other exceptions that i didn't think of would just crash the program. am i totally wrong with this? I think i understand and trust you when you said the class didn't need to throw an exception, but is it better to catch them and throw them as you could always get some person who would enter in too much data for java's string container to handle?

oulevon · 01-22-2003, 10:03 PM

Post any questions you have and I'll do my best to answer them. As to the limit on the size of a String. I had never thought of this, so I did a little research. The Java Language Specification doesn't specify a limit on the size of a String, but if you look in java.lang.String you'll notice the length() method returns an integer, so one would argue that a String could be as large as MAX.INTEGER. I would tend to think that it's limit would be directly related to the amount of memory available in the heap. If this worries you, you could always limit the size of the String the user can enter whether it be by creating and throwing your own type of exception, or limiting the size of a text area. I'm not sure what you are using for reference, but Bruce Eckel has a good book called Thinking In Java, and he offers it free for download at his website: http://www.mindview.net/Books

megaspaz · 01-22-2003, 11:37 PM

thanks again. i was just using sun's downloaded java api documentation.

megaspaz · 01-23-2003, 08:11 PM

AtoBtoA2.tar.gz

The link above is the reworked version. I've made it more modular but would like your suggestions. thank you.

if possible, could someone tell me if it's possible to have hot keys for the widgets? like if i had a button with it's text Convert, this normally denotes that the user can type alt+c and activate the button. How can i do that in java, if it's possible?

oulevon · 01-23-2003, 09:29 PM

I haven't looked at the file yet, but I will. It is possible to have hot keys, or Mnemonics. Do a search on Java Mnemonics or look in the API documentation under javax.swing.AbstractButton . Specifically you'd be looking at the setMnemonic(int i) method. Your values can range from a-z.

Keep in mind the direct subclasses of that class are JButton, JMenuItem, and JToggleButton. I'll get back to you on the file.

oulevon · 01-23-2003, 09:45 PM

megaspaz,

You're link to your new program isn't working. I'm getting a Not Found error.

megaspaz · 01-24-2003, 12:43 AM

you're right. crud. i forgot a subdirectory. sorry. thanks for the info on the hotkeys.

AtoBtoA2.tar.gz