1 reply to this topic

[adash]

Hi Guys !
I am new here as well as I am new in the Java Developpment. I know pretty good C++ and now I start to learn Java.

I imagined by myself to write a short binary number adder, Hamming Distance function and counting ones (simplest problem as I could have imagined) just to gain some experience in Java. Actually it's my first program written in Java excluding some examples from Internet, books etc. (Of course it works well) :

public class MyClass {


	public static void main(String[] args) 

	{

		String test="111110111111";

		String test1="111111111111";

		BinaryNumber Ntest = new BinaryNumber(test);

		BinaryNumber Ntest1 = new BinaryNumber(test1);

		BinaryNumber suma = Ntest.sum(Ntest1);

		suma.DisplayNumber();

		

	}


}


class StringNumber

{

	public StringNumber(String n)

	{

		this.num_length=n.length();

		this.n_string=n;

		Numbers=new int[num_length];

		StringToIntArray();

	}

	

	public StringNumber(int[] t)

	{

		this.num_length=t.length;

		Numbers=new int[num_length];

		this.Numbers=t;	

	}

	

	private void StringToIntArray()

	{

		for(int i=0;i<n_string.length();++i)

		{

		Character c = new Character(n_string.charAt(i));

		String str = c.toString();

	    int temp = Integer.parseInt(str);

		Numbers[i]=temp;

		}

	}

		

	public void DisplayNumber()

	{

	for(int i=0; i<this.Numbers.length; ++i)

		System.out.print(this.Numbers[i]);

	}

	

	protected int[] Numbers; 

	private String n_string;

	private int num_length;

}



//-----------------------------------------------------------------------------------------------------------------------//

class BinaryNumber extends StringNumber

{

	public BinaryNumber(String n) {

		super(n);

	}

	

	public BinaryNumber(int[] t)

	{

	super(t);

	}

		

	public int Hamming(BinaryNumber b)

	{		

	if(b.Numbers.length==this.Numbers.length)

	{

	for(int i=0; i<b.Numbers.length; ++i)

	if(b.Numbers[i]!=this.Numbers[i]) ++HammingsDistance;

	} else

	{HammingsDistance=-1;}

	return HammingsDistance;

	}

	

	public BinaryNumber sum(BinaryNumber a)

	{

		int CarryIn = 0, CarryOut = 0, Nbits=this.Numbers.length,sum,j;

		int[] Bin = new int[Nbits+1];

		

		for(int i=Nbits-1; i>=0; --i)

		{

		sum=this.Numbers[i]+a.Numbers[i]+CarryIn;

		switch(sum)

		{

		case 0:

			CarryOut=0;

			Bin[i+1]=0;

			break;

		case 1:

			CarryOut=0;

			Bin[i+1]=1;

			break;

		case 2:

			CarryOut=1;

			Bin[i+1]=0;

			break;

		case 3:

			CarryOut=1;

			Bin[i+1]=1;

			break;

		}

		CarryIn=CarryOut;

		}

		Bin[0]=CarryOut;		

		BinaryNumber AddResult = new BinaryNumber(Bin);

			

		return AddResult;		

	}


	public int HammingsDistance;

	public int _ones;

}

Is my code style good?
Are there any errors?
Is there any better way to solve this problem with class inheritance ?

I would appreciate all comments, critics and feedbacks.
Hope you can understand my english :c-biggrin:
Thanks for help!

Adam

[wim DC]

My humble opinion :c-biggrin:

• You use the "this" keyword too much.
  You should only use it when you have 2 variables who's name collide. I often use it in a constructor. Let's take the Person class:
  

  public class Person{
  
    private String name;
  
  
    public Person(String name){
  
      this.name = name;
  
    }
  
  }

  I used the same variable name because it's nice with the intellisence when you're creating a new Person, then you see that variable name,
  and because it's "name" it's meaningful and you know what to give as parameter to the constructor.
  To make a difference between the variable in the current scope and the class variable, i use "this" to point at the class variable.
  
• Variables never begin with a captial letter
  Bin, CarryIn, CarryOut, AddResult, Nbits,... All should start with a small letter.
  
• Methods/functions don't start with a captial letter either.
  
• class variables never are public
  It's extremely rare to have public variables. In Java it's much more comon to have them as private as possible. And if other classes need them, then you write getter / setter methods.
  (protected is fine with inheritance)
  

  public class Person{
  
    private String name;
  
  
    public Person(String name){
  
      this.name = name;
  
    }
  
  
    public String getName(){
  
      return name;
  
    }
  
  
    public void setName(String name){
  
      this.name = name;
  
    }
  
  }

  
  
• unused variable: num_length (java people love camelCase -->numLength), _ones, j(in sum function)
  
• I'm not sure if hammingDistance should be a class variable. Can be turned into a local variable in the hamming function.
  
• Shorter code: (just because it's shorter :P)
  
  • I rewrote the switch statement.
    

    
    carryOut = sum/2;
    
    bin[i+1]=sum%2;

    
    
  • 
    

    BinaryNumber AddResult = new BinaryNumber(Bin);			
    
    return AddResult;

    In 1 line:
    

    
    return new BinaryNumber(Bin);
    
    

    
    
  • in the stringToArray method:
    

    Character c = new Character(n_string.charAt(i));
    
    String str = c.toString();
    
    int temp = Integer.parseInt(str);

    is the same as. ( +"" will automatically call .toString.)
    

    
    int temp = Integer.parseInt( n_string.charAt(i) +"" );
    
    

  
  
  
• I'm not too sure, but i believe it's preferred to use a foreach over a normal for-loop. Unless you'd need the counter
  

  
  for(int i=0; i<this.numbers.length; ++i)
  
    System.out.print(this.numbers[i]);

  becomes:
  

  
  for(int number : numbers){
  
    System.out.print(number);
  
  }
  
  

  
  
• You used Character at the stringToIntArray, it's better to use "char" instead. It should be faster ^^