Tukukkk

So,basically I am working on an algorithm for circular shifting a string upto a position.There are two parameters required here.

1. String of text to shift


2. Number of shifts required.

For example:

Given string: "Hello"

Shifts: 3

Final: "ollHe"

Ignore the limited validation here.

import java.lang.*;

import java.util.*;



public class Shift

{

public String shifting(String s,Integer sh)

{

    Integer shifts=sh;

    String string=s;

    char shifted=new char[50];

    Integer pos=0;

    for(int i=string.length();i>shifts;i--)

    {

        System.out.println(""+i);

        pos++;

        shifted[pos]=string.charAt(i-1);

    }

    System.out.println("Shifting complete");

    for(int j=0;j<shifts;j++)

    {

        System.out.println(""+j);

        pos++;

        shifted[pos]=string.charAt(j);

    }

    return new String(shifted);

}   

public static void main(String args) {

        System.out.println("Enter the string ");

        Scanner sc=new Scanner(System.in);

        String string=sc.nextLine();

        System.out.println("So you have entered:"+string);

        System.out.println("Enter the number of shifts:");

        Integer shifts=sc.nextInt();

        System.out.println("number of shifts is:"+shifts.toString());

        System.out.println("Shifted string:"+new Shift().shifting(string,shifts));

    }   

}

`
Give your views on the code here.

Do you have any constraints?

• Is your example wrong? I would expected lloHe
  


• If you are concerned of modifying your method parameters make them final. Copy it to local variables and don't change the variables is kind of useless.


• Don't use short variables names (exception: the loop counter)


• If you use Integer instead of int, you should handle null
  


• Maybe a simple static utility method would be fine here, except you have some inheritance scenario in mind.


• Maybe you should also indicate that you are shifting left for positive numbers


• You should take the length of you string for the char.

Assuming your example is wrong, what about:

import java.util.Objects;

import javax.annotation.Nonnull;



public final class Shift

{

    @Nonnull

    public static String left( @Nonnull final String string, final int shift )

    {

        final int length = string.length();

        if( length == 0 ) return "";

        final int offset = ((shift % length) + length) % length; // get a positive offset



        return string.substring( offset, length ) + string.substring( 0, offset );

    }



    public static void main( String... args )

    {

        assertEquals( "loHel", Shift.left( "Hello", -2 ) );

        assertEquals( "oHell", Shift.left( "Hello", -1 ) );

        assertEquals( "Hello", Shift.left( "Hello", 0 ) );

        assertEquals( "elloH", Shift.left( "Hello", 1 ) );

        assertEquals( "lloHe", Shift.left( "Hello", 2 ) );

        assertEquals( "loHel", Shift.left( "Hello", 3 ) );

        assertEquals( "oHell", Shift.left( "Hello", 4 ) );

        assertEquals( "Hello", Shift.left( "Hello", 5 ) );

        assertEquals( "elloH", Shift.left( "Hello", 6 ) );

        assertEquals( "", Shift.left( "", 3 ) );

    }



    private static void assertEquals( String expected, String actual )

    {

        if( !Objects.equals( expected, actual ) ) throw new AssertionError( "Expected: >" + expected + "< was: >" + actual + "<" );

    }

}

("Rotation" = "shift".)

You can perform any rotation in constant time by devising a String wrapper that maintains the string itself and the index into that string which is to be considered as the head character of a rotation. Now, in order to rotate $k$ characters, just add $k$ to that internal offset and take the modulo of string length.

If you want to get a String out of that wrapper, just call its toString(). There is, however, an optimization: if the state of the wrapper is not "dirty", it returns the previous cached rotation. Otherwise, a new string will be created and the state marked as not dirty.

I had this in mind:

import java.util.Objects;



public class StringRotation {



    private final String string;

    private int headIndex;

    private boolean dirty;

    private String lastCreatedString;



    public StringRotation(String string) {

        this.string = Objects.requireNonNull(string,

                                             "The input string is null.");

        this.dirty = true;

    }



    public StringRotation rotate(int rotationCount) {

        headIndex = mod(headIndex + rotationCount, string.length());

        dirty = true;

        return this;

    }



    public char charAt(int index) {

        checkIndexWithinBounds(index);

        return string.charAt(mod(headIndex + index, string.length()));

    }



    @Override

    public String toString() {

        if (!dirty) {

            return lastCreatedString;

        }



        int stringLength = string.length();

        StringBuilder sb = new StringBuilder(stringLength);



        for (int i = 0; i < stringLength; ++i) {

            sb.append(charAt(i));

        }



        dirty = false;

        lastCreatedString = sb.toString();

        return lastCreatedString;

    }



    private void checkIndexWithinBounds(int index) {

        int stringLength = string.length();



        if (index < 0) {

            throw new IndexOutOfBoundsException(

                    "The character index is negative: " + index);

        }



        if (index >= stringLength) {

            throw new IndexOutOfBoundsException(

                    "The character index is too large: " + index + 

                    ". The length of this string is " + stringLength);

        }

    }



    private static int mod(int a, int q) {

        int ret = a % q;

        return ret < 0 ? ret + q :  ret;

    }



    public static void main(String args) {

        for (int i = 0; i < 10; ++i) {

            System.out.println(new StringRotation("hello").rotate(i));

        }



        System.out.println("---");



        for (int i = 0; i > -10; --i) {

            System.out.println(new StringRotation("hello").rotate(i));

        }

    }

}

In my opinion this is way better by a landslide:

import java.util.Scanner;



public class Example {



    static String shifter(String stringToShift, int numberOfShifts){

        for(int i = 0; i < numberOfShifts;i++)

        {

            // Store the character you want to shift

            char temporaryChar = stringToShift.charAt(stringToShift.length() - 1);



            // Store the character with the original String. Character first

            stringToShift = temporaryChar + stringToShift;



            // Now that we did that, the string is 1 character longer so we need to take a substring of it

            stringToShift = stringToShift.substring(0, stringToShift.length()-1);

        }



        // After everything is done return the String

        return stringToShift;

    }



    public static void main(String args) {

        // Start getting input

        Scanner input = new Scanner(System.in);



        // Get String input

        System.out.println("Enter the string ");

        String answer = input.nextLine();

        System.out.println("So you have entered: " + answer);



        // Get int input

        System.out.println("Enter the number of shifts:");

        int numberOfShifts = input.nextInt();

        System.out.println("number of shifts is: " + numberOfShifts);



        // Shift and print out the answer

        System.out.println("Shifted string: " + shifter(answer, numberOfShifts));



        // Close input after you're done

        input.close();



    } 



}

Pay attention at my style of coding and naming, it's better to have longer names so later you at least understand why the variable is there in the first place.