Results 1 to 9 of 9
  1. #1
    Rebugger is offline CS Student
    Join Date
    Jul 2014
    Posts
    7
    Rep Power
    0

    Default Could somebody please review my code?

    Would it be possible for somebody to take a look over my code?

    I have made a Java Binary Encoder and Decoder program and, to date, this is the program I have spent the most time developing.
    I have ran Alpha tests to check the functionality is correct and it appears to work well. However I am more interested in the quality of my code and learning about ways in which I would improve to help me in my future work. I am not expecting anybody to thoroughly look into my code, just point out things that are obviously bad and stand out.

    The code can be found over at: https://github.com/mikegreen1995/Binary-Encoder-Decoder

    Thank you in advance, I appreciate any time you can give me.


    -------

    I have also asked for suggestions on reddit at the following URL [Java] Could somebody please review my code? : learnprogramming

    I have received great feedback from one person on there and have a branch on my repository for the development of one suggestion he gave me about my error checking system

  2. #2
    jim829 is offline Senior Member
    Join Date
    Jan 2013
    Location
    Northern Virginia, United States
    Posts
    3,616
    Rep Power
    5

    Default Re: Could somebody please review my code?

    Most folks on this forum are reluctant to visit other websites for code. So what exactly does the program do?
    If you have questions about certain key areas of code, why not just post them (with an explanation of course).

    Regards,
    Jim
    The Java™ Tutorial | SSCCE | Java Naming Conventions
    Poor planning our your part does not constitute an emergency on my part.

  3. #3
    Rebugger is offline CS Student
    Join Date
    Jul 2014
    Posts
    7
    Rep Power
    0

    Default Re: Could somebody please review my code?

    Quote Originally Posted by jim829 View Post
    Most folks on this forum are reluctant to visit other websites for code. So what exactly does the program do?
    If you have questions about certain key areas of code, why not just post them (with an explanation of course).

    Regards,
    Jim

    Oh really?
    I imagined places like GitHub would be commonplace on a programming forum.

    To be honest, there is no one specific piece of code that I have questions about. It was really just to see if my coding style is correct or if there is anything blatantly wrong with it. Something that would be immediately noticeable to more experienced programmers because of all their knowledge, but something that I just don't know about because I am so new to programming in general.

  4. #4
    Rebugger is offline CS Student
    Join Date
    Jul 2014
    Posts
    7
    Rep Power
    0

    Default Re: Could somebody please review my code?

    Quote Originally Posted by jim829 View Post
    So what exactly does the program do?
    Sorry, forgot to answer this.

    From my README.md:

    Binary Encoder/Decoder is a utility which enables the user to convert between binary and text or decimal formats.

    The program currently supports three number encoding methods:

    - Standard
    - Binary Coded Decimal
    - Two's Complement

    Text uses the UTF-8 format.
    It is a swing application

  5. #5
    jim829 is offline Senior Member
    Join Date
    Jan 2013
    Location
    Northern Virginia, United States
    Posts
    3,616
    Rep Power
    5

    Default Re: Could somebody please review my code?

    Ok, I decided to look at some of your code. First, if it works, that is great (and the most important part). Second, what
    I am about to tell you is based on my experience and also personal coding style. Others may disagree which is fine.

    1. When calculating BCD, instead of switching on 0 thru 9 to get the BCD equivalent, you could simply initialize a
    List<String> of those values and retrieve them directly. For the other direction, you could use a hash map. Or,
    since the two keys don't overlap, put them all in the same hashmap. An enum may be useful here, perhaps with
    an EnumMap (but I have not really investigated it). Note: although it is now allowed to switch on Strings, I don't
    do it. I have no idea whether it is efficient or not, I just prefer not to do it.

    2. When converting binary to decimal you reverse the user's input. Why not just index from the end of the String
    to the front?
    3. Instead of using foo.charAt(i) over and over within a loop, just assign it to a simple char and use that instead.
    The compiler may do this internally but I prefer to do it myself as it results in less typing and imho cleaner code.
    4. Don't use Math.pow() to convert from binary to decimal. I try to avoid going from integer to floating point to
    integer whenever possible. There are several ways I might do it, here is one (actually, in production I would use
    Integer.parseInt in the JDK).

    Java Code:
    int val = 0;
    String userInput = "111001101";
    for (int j = 0; j < userInput.length(); j++) {
        int bit = userInput.charAt(j) - '0';
        val <<= 1; // or val *= 2;
        val |= bit; // or val += bit
    }
    Some of the most important things to do are these (which I believe you do).

    1. Use standard Java coding conventions.
    2. Use {}, even for simple blocks
    3. Indent blocks to keep code readable
    4. Document what is not obvious.

    Donald Knuth referred to programming as an art form. So do you own thing and worry more about correctness
    and readability than using optimum coding logic. In time, the latter will improve.

    Regards,
    Jim
    The Java™ Tutorial | SSCCE | Java Naming Conventions
    Poor planning our your part does not constitute an emergency on my part.

  6. #6
    Rebugger is offline CS Student
    Join Date
    Jul 2014
    Posts
    7
    Rep Power
    0

    Default Re: Could somebody please review my code?

    Quote Originally Posted by jim829 View Post
    Ok, I decided to look at some of your code...
    That was a phenomenal reply. Thank you so much for taking the time to look through it.
    I will get to work on implementing all of the suggestions that you provided. Many of the things you suggested I have not learnt about yet but I will familiarise myself with them in the next few days - using them in this program will be the perfect opportunity to practise what I have learnt too!

  7. #7
    Rebugger is offline CS Student
    Join Date
    Jul 2014
    Posts
    7
    Rep Power
    0

    Default Re: Could somebody please review my code?

    I have implemented your suggestions from points 1 and 2.
    In 1, instead of the switch statement I made an array of the BCD values for each decimal value and put these in the corresponding index. Therefore there is no comparison needed each time and it should be more efficient. I didn't use a List because I only need something fixed size.

    This is the code I used instead of the switch statement:

    Java Code:
    long total = 0L;
    	
    		for(int i = input.length(); i >= 0; i--){
    		
    			if ((input.charAt(i) != '1') && (input.charAt(i) != '0')){
    				//if, whilst scanning, non 0 or 1 encountered, return error
    				throw new java.lang.IllegalArgumentException("Non-binary input.");
    			}
    			else if (input.charAt(i) == '1'){
    				total = total + ((long)Math.pow(2,i));
    			}
    		}
    		return Long.toString(total);
    However, I do not understand what you mean in point 3, could you please explain?

    In your 4th point, you mention not switching between int-double-int. One idea I had would be to make my own pow method and strictly use ints in it so there is no casting. I can simply do a multiplication loop which loops the number of the power. Would this be an ok solution too?

    Pseudo for idea:
    Java Code:
    input int base, int power
    
    output = base
    while i is less than power{
        output = output*base;
    }
    return output

  8. #8
    jim829 is offline Senior Member
    Join Date
    Jan 2013
    Location
    Northern Virginia, United States
    Posts
    3,616
    Rep Power
    5

    Default Re: Could somebody please review my code?

    By point (3) I simply meant do the following:

    Java Code:
    for (int k = 0;k < str.length; k++) {
        char c = str.charAt(k);
        // do something with c
        // do something else with c
        // continue using c
    }
    before you were using str.charAt(k) everwhere. Now it may not matter as the compiler
    may do the substitution for you. Basically it results in fewer method calls.

    Regarding you pseudocode, don't worry too much about my statement about floats and ints.
    But I would recommend you try to integrate the power function as you compute the new base.
    My recommended way of doing it is similar to Horner's Method for calculating polynomials.
    For example, if you think about converting 101 to decimal, it is essentially a polynomial.

    1 x 2^2 + 0 x 2^1 + 1 x 2^0

    You can read about it here --> Horner's method - Wikipedia, the free encyclopedia

    But most important is not to worry too much about efficiency or optimization. It will
    come as you gain more experience.

    Regards,
    Jim
    Last edited by jim829; 08-09-2014 at 05:19 PM.
    The Java™ Tutorial | SSCCE | Java Naming Conventions
    Poor planning our your part does not constitute an emergency on my part.

  9. #9
    Rebugger is offline CS Student
    Join Date
    Jul 2014
    Posts
    7
    Rep Power
    0

    Default Re: Could somebody please review my code?

    Oh I see, I can easily implement that, thanks for pointing it out. It does seem incredibly wasteful to keep computing charAt().

    I'll take a look at that wiki link in more detail later and see what I can do with my code afterwards.

    Other than that, thanks for your input. It has been really helpful, I'll be able to learn from your suggestions to improve my future code!

Similar Threads

  1. Replies: 3
    Last Post: 02-14-2012, 10:12 PM
  2. Requesting Code Review
    By Zeramat in forum New To Java
    Replies: 9
    Last Post: 06-16-2011, 05:59 AM
  3. Receiving code review
    By sunde887 in forum New To Java
    Replies: 0
    Last Post: 03-08-2011, 05:31 AM
  4. please review the following code
    By ajbj in forum New To Java
    Replies: 3
    Last Post: 08-25-2009, 08:37 AM
  5. [SOLVED] Code review
    By saeedsubedar in forum Advanced Java
    Replies: 14
    Last Post: 06-25-2008, 05:25 AM

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •