Sphere Online Judge, Problem 2: Prime Number Generator I am trying to solve this problem on Sphere Online Judge. I keep getting a timeout error. Any comments or suggestions are welcome and appreciated.

package info.danforbes.sphere;

import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStreamWriter;
import java.nio.charset.MalformedInputException;
import java.util.AbstractMap.SimpleEntry;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Map.Entry;

public class PrimeNumberGenerator {
  private static BufferedReader inStream;
  private static BufferedWriter outStream;
  private static List<Entry<Integer, Integer>> pairs = new ArrayList<Entry<Integer, Integer>>();
  private static HashSet<Integer> primes = new HashSet<Integer>();

  private static void getPairs(int numCases) throws MalformedInputException {
    String line;
    int spaceNdx;
    int num1, num2;
    try {
      for (int pairNdx = 0; pairNdx < numCases; ++pairNdx) {
        line = inStream.readLine();
        spaceNdx = line.indexOf(' ');
        num1 = Integer.parseInt(line.substring(0, spaceNdx));
        num2 = Integer.parseInt(line.substring(spaceNdx + 1));
        if (num1 < 1) throw new MalformedInputException(num1);
        if (num1 > num2) throw new MalformedInputException(num2 - num1);
        if (num2 > 1000000000) throw new MalformedInputException(num2);
        if (num2 - num1 > 100000) throw new MalformedInputException(num2 - num1);
        pairs.add(new SimpleEntry<Integer, Integer>(num1, num2));
      }
      inStream.close();
    } catch (IOException e) {
      System.out.println("IOException encounterd!");
      e.printStackTrace();
      System.exit(-1);
    }
  }

  private static boolean isPrime(int num) {
    boolean isPrime = true;
    if (num < 2) isPrime = false;
    else if (num > 3) {
      if (num % 2 == 0 || num % 3 == 0) isPrime = false;
      else if (!primes.contains(num)){
        int sqrRoot = (int)Math.sqrt(num);
        for (int factorNdx = 1; 6 * factorNdx - 1 <= sqrRoot; ++factorNdx) {
          if (num % (6 * factorNdx + 1) == 0 || num % (6 * factorNdx - 1) == 0) {
            isPrime = false;
            break;
          }
        }
      }
    }

    if (isPrime) primes.add(num);
    return isPrime;
  }

  public static void main(String[] args) {
    if (args.length == 0) {
      inStream = new BufferedReader(new InputStreamReader(System.in));
      outStream = new BufferedWriter(new OutputStreamWriter(System.out));
    }

    StringBuilder resultBuilder = new StringBuilder();

    try {
      if (inStream == null && outStream == null) {
        inStream = new BufferedReader(new FileReader(args[0]));
        outStream = new BufferedWriter(new FileWriter(args[1]));
      }

      int numCases = Integer.parseInt(inStream.readLine());
      if (numCases > 10) throw new MalformedInputException(numCases);
      getPairs(numCases);
      int beg, end;
      for (Entry<Integer, Integer> anEntry : pairs) {
        beg = anEntry.getKey();
        end = anEntry.getValue();
        for (int rangeNdx = beg; rangeNdx <= end; ++rangeNdx) {
          if (isPrime(rangeNdx)) resultBuilder.append(Integer.toString(rangeNdx) + "\n");
        }

        resultBuilder.append("\n");
      }

      resultBuilder.delete(resultBuilder.length() - 2, resultBuilder.length());
      outStream.write(resultBuilder.toString());
      outStream.close();
    } catch (IOException e) {
      System.out.println("IOException encountered!");
      e.printStackTrace();
      System.exit(-1);
    }
  }
}

I'll make general statements not addressing (or not limited to) performance.

Your overall design is not very Object-Oriented. Usage of static/global variables is discouraged for a reason, it is very hard to follow the flow the program. Consider rewriting it with proper classes/encapsulation.

You could extract reading into one function, which then returns the list of pairs which another function can work on. Some pseudo code:

main()
    List pairs
    if args provided
        pairs = pairsFromFile(args[0])
    else
        pairs = pairsFromStdIn()

    foreach pair in pairs
        processpair(pair)

Your usage of one-line ifs is partly confusing, consider to expand those to at least two lines for readability:

if(test)
    return true;

You're declaring and actively throwing exceptions which never get caught, that means that your application will exit with a thrown exception, that's a bad thing. Applications should handle exceptions at least in the main loop and exit gracefully.

Consider writing directly to the output-stream instead of buffering everything in a StringBuilder. While it might hurt performance if you're doing a lot of writing, it will remove some of the complexity.

System.out.println("IOException encounterd!");

Exceptions and errors should traditionally go to System.err.

You're using System.exit() at some places, it might be more desirable to let the main loop handle the application flow instead of "exiting" from any or some functions. I also now realize that you're using some sort of template to handle exceptions (write to out, print stacktrace, exit(-1)), please don't just slap an "error-handling-template" on it and be done with it. Handle exceptions in a context-sensitive way.

isPrime() does use a temporary variable isPrime when returning at the appropriate places would be more then enough.

private static boolean isPrime(int num) {
  if (num < 2)
    return false;
  else if (num > 3) {
    // Loop here
        if (num % (6 * factorNdx + 1) == 0 || num % (6 * factorNdx - 1) == 0) {
          return false;
        }
  }

  if (isPrime) primes.add(num);
  return true;
}


Calculate different speeds between min and max speed with a stepsize public static void main(String[] args) {
    int minKmh = 10;
    int maxKmh = 20;
    int step = 5;

    String allSpeeds = "";
    while (minKmh <= maxKmh) {
        allSpeeds += minKmh + ",";
        minKmh += step;
    }
    String[] allSpeedsAsArray = removeLastChar(allSpeeds).split(","); // result: 10,15,20

}

private static String removeLastChar(String str) {
    return str.substring(0, str.length() - 1);
}

I have a minimum car speed and a maximum car speed. In this case 10 and 20. And I want every speed which is between minimum and maximum where the speed gets incremented by a step speed.

I don't like my current version because it is large in my opinion. Is it possible to shorten the code to 1-2 liner?

First off, you should capture this functionality in a dedicated method, rather than in the class' main method.

Secondly, I'm not sure why you want an array of String. I don't know the context, but as you're already calculating with speeds, it seems likely you'll calculate more with them further on, and you'll want them to be numbers again.

Next, your method of doing this is appending everything in one giant String, and then splitting it again, after dealing with an unnecessary separator at the end. Looks like a lot of work for nothing. You can just add everything to an array directly (you'll have to precalculate its size beforehand though)

Java 8 has IntStream, which is ideal for this task. It enables the one-liner you're looking for.

private int[] allSpeeds(int lower, int upper, int step) {
    return IntStream.iterate(lower, i -> i + step)
            .limit(Math.max(0, ((upper - lower) / step) + 1)) // limit to the needed size, ensure this value cannot be negative
            .toArray();
}


Loop whose endpoint (?) change during the loop I have a loop which insert rows on specific cases. The loop goes through ~2000 rows and insert about 500.

Due to the inserted rows it doesn't loop the full range I want. I have solved that problem by counting the number of row I will insert with a first loop and then looping up to LastRow + Counter.

I feel as if there must have been a better way to do it.

Sub AddRows()
Dim i As Integer: i = 8
Dim LastRow As Integer: LastRow = Cells(8, 2).End(xlDown).Row
Dim counter As Integer: counter = 0

For i = 8 To LastRow
    If Cells(i, 1) <> "" Then
        counter = counter + 1
    End If
Next i

NewLastRowAfterAddingRows = LastRow + counter

For i = 8 To NewLastRowAfterAddingRows
    If Cells(i, 1) <> "" Then
        Range(i + 1 & ":" & i + 1).Insert CopyOrigin:=xlFormatFromRightOrBelow
        i = i + 1
    End If
Next i

End Sub

It's generally not a good idea to alter the iterator variable in a for/next loop.  While your code works, it's just not a good practice.  Also, your initializing i to 8 is redundant since your loop starts at 8 as well.  Here's how I would handle it:

Sub AddRows()

    Dim i As Integer
    Dim LastRow As Integer
    Dim counter As Integer

    LastRow = Cells(8, 2).End(xlDown).Row
    counter = 8

    For i = counter To LastRow
        If Cells(counter, 1) <> "" Then
            counter = counter + 1
            Range(counter & ":" & counter).Insert CopyOrigin:=xlFormatFromRightOrBelow
        End If
        counter = counter + 1
    Next i

End Sub


Get Levenshtein Distance Logic based of this site which had a VB example I used as a base.

function Get-LevenshteinDistance{
    [cmdletbinding()]
    param(
        [string]$CompareString,
        [string]$DifferenceString
    )

    # Collect the string lengths
    $compareStringLength = $CompareString.Length
    $differenceStringLength = $DifferenceString.Length
    Write-Verbose "Comparision String: '$CompareString' with length '$compareStringLength'"
    Write-Verbose "Difference String: '$DifferenceString' with length '$differenceStringLength'"

    # If either of the strings are empty the LD is known and we can stop
    if($compareStringLength -eq 0){return $differenceStringLength}
    if($differenceStringLength -eq 0){return $compareStringLength}

    # Build the comparison matrix and populate the first column and first row.
    $comparisonMatrix = New-Object 'object[,]'  ($differenceStringLength+1),($compareStringLength+1)

    # Populate the first row
    for($index=0; $index -le $compareStringLength; $index++){
        $comparisonMatrix[0,$index]=$index
    }

    # Populate the first column
    for($index=0; $index -le $differenceStringLength; $index++){
        $comparisonMatrix[$index,0]=$index
    }

    # Calculate the Levenshtein distance by working through each position in the matrix.
    # Working the columns
    for($columnIndex=1; $columnIndex -le $compareStringLength; $columnIndex++){
        # Cycle each character in the list
        $compareCharacter = $compareString[$columnIndex-1]

        # Working the rows
        for($rowIndex=1; $rowIndex -le $differenceStringLength; $rowIndex++){
            # Cycle each character in the list
            $differenceCharacter = $differenceString[$rowIndex-1]

            Write-Verbose "Matrix location: [$rowIndex, $columnIndex]"
            Write-Verbose "Compare character: $compareCharacter - Difference character: $differenceCharacter"
            # Calculate the cost
            $cost=if($compareCharacter -eq $differenceCharacter){0}else{1}
            Write-Verbose "Cost: $cost"

            # The cell immediately above plus 1
            $cellAbove = $comparisonMatrix[($rowIndex-1), $columnIndex] + 1
            Write-Verbose "Cell Above: [$($rowIndex-1), $columnIndex] + 1 = $cellAbove"

            # The cell immediately to the left plus 1
            $cellLeft =  $comparisonMatrix[$rowIndex,($columnIndex-1)] + 1
            Write-Verbose "Cell Left: [$rowIndex,$($columnIndex-1)] + 1 = $cellLeft"

            # The cell diagonally above and to the left plus the cost
            $cellUpperLeft = $comparisonMatrix[($rowIndex-1),($columnIndex-1)] + $cost
            Write-Verbose "Cell Upper Left: [$($rowIndex-1),$($columnIndex-1)] + cost($cost) = $cellUpperLeft"

            # Select minumum of the of the last 3 cells calculations and assign it to the current matrix position.
            $comparisonMatrix[$rowIndex,$columnIndex] = $cellAbove,$cellLeft,$cellUpperLeft | Measure-Object -Minimum | select -ExpandProperty Minimum
        }
    }
    # The cacluated LD will now be in the bottom right of the matrix.
    return $comparisonMatrix[$differenceStringLength,$compareStringLength]
}

Was working on a PPCG question where I had to roll my own LD calculator. Since I saw future value in this I made a more professional looking version. Since it is iterating over every character in a string for every other character in the other string it really starts to show its performance issues when they start to get over 50 chars in size.

The model it uses supports deletions, insertions and substitutions. If I wanted to change the weight of substitutions I suppose I would need to edit this line and support a parameter for it?

$cost=if($compareCharacter -eq $differenceCharacter){0}else{1}

Never really played with multidimensional arrays before and I am not sure how I could improve on this. I know there are a few things I could do to make the code more brief and still functional however I opted for this solution as I liked the improved readability.

Here is my development series (in parentheses: approx fast ratio compared with the previous, tested using strings cca 100 characters in size, see the comparison table below):

Get-LevenshteinDistanceOrig  (= 1×, for comparison): the original function body (Copy&Paste);
Get-LevenshteinDistanceOrig+ (> 3×, still insufficient): removed all time-expensive  Write-Verbose output;
Get-LevenshteinDistanceTry1  (> 50×, a substantial improvement): time-expensive pipeline to Measure-Object replaced by [math]::Min(n,m) static method (see .NET system.math class). The following are only minor advancement steps:
Get-LevenshteinDistanceTry2  (~ 1×..2×): slight algorithm renovation:

instead calculating the cost ($cost variable): if compared characters are equal then computing and comparing values $cellAbove, $cellLeft and $cellUpperLeft is useless as we already know the necessary value;
added: if strings are equal the LD is known (zero) and we can stop;

Get-LevenshteinDistance      (~ 2×): PoSH array implementation: instead  of two-dimensional comparison matrix used a rectangular jagged array.

Comparison table: .\CR\164518test.ps1 | Format-Table -AutoSize

cmdlet                       similarity runtime (ms) lengths  LD
------                       ---------- ------------ -------  --
Get-LevenshteinDistanceOrig  equal       6829.1053   106 106   0
Get-LevenshteinDistanceOrig+ equal       2010.4538   106 106   0
Get-LevenshteinDistanceTry1  equal         35.8835   106 106   0
Get-LevenshteinDistanceTry2  equal           .1795   106 106   0
Get-LevenshteinDistance      equal           .1539   106 106   0
Get-LevenshteinDistanceOrig  stochastic  6556.4117   106 102  79
Get-LevenshteinDistanceOrig+ stochastic  1932.6051   106 102  79
Get-LevenshteinDistanceTry1  stochastic    33.9023   106 102  79
Get-LevenshteinDistanceTry2  stochastic    28.3165   106 102  79
Get-LevenshteinDistance      stochastic    13.4852   106 102  79
Get-LevenshteinDistanceOrig  similar     6640.5884   106 102   4
Get-LevenshteinDistanceOrig+ similar     2023.8179   106 102   4
Get-LevenshteinDistanceTry1  similar       31.5843   106 102   4
Get-LevenshteinDistanceTry2  similar       14.9307   106 102   4
Get-LevenshteinDistance      similar        8.4234   106 102   4
Get-LevenshteinDistanceOrig  different   6613.5267   106 102 106
Get-LevenshteinDistanceOrig+ different   1943.7630   106 102 106
Get-LevenshteinDistanceTry1  different     35.1824   106 102 106
Get-LevenshteinDistanceTry2  different     27.1371   106 102 106
Get-LevenshteinDistance      different     13.8924   106 102 106

Column explanation:

cmdlet       - function name
similarity   - in brief "like" similarity of input strings
runtime (ms) - TotalMilliseconds
lengths      - lengths of input strings, space delimited
LD           - the Levenshtein Distance of input strings

Comparison script 164518test.ps1:

Function TestLD {
    param ([string]$Similarity = '')

    $aux = 0
    Write-Progress -Activity "LD info ($Similarity strings)" `
         -CurrentOperation 'Start' -PercentComplete $aux
    $cmdletTails = 'Orig','Orig+','Try1','Try2',''
    foreach ( $cmdletTail in $cmdletTails ) {
        $cmdlet = "Get-LevenshteinDistance$cmdletTail"
        $scriptBlock = { $LevenshteinDistance = & $cmdlet `
                -CompareString $strC -DifferenceString $strD }
        $TimeSpan = Measure-Command -Expression $scriptBlock
        $aux += [int](100 / $cmdletTails.Count)
        Write-Progress -Activity "LD info ($Similarity strings)" `
             -CurrentOperation $cmdlet -PercentComplete $aux
        [PSCustomObject]@{
            'cmdlet'     = $cmdlet
            'similarity' = $Similarity
            'runtime (ms)'   = $($TimeSpan.TotalMilliseconds.
                               ToString('#####.###0',$cultureinfo).
                                   PadLeft(10))
            'lengths'    = "$($strC.Length) $($strD.Length)"
            'LD'         = $LevenshteinDistance
        }
    }
}

. D:\PShell\CR\164518.ps1                        # reload the functions
$cultureinfo = [cultureinfo]::InvariantCulture   # my one is different
$strC='Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum placerat leo ut turpis viverra lacinia'
$strD=$strC
TestLD -Similarity 'equal'
$strD='Cras efficitur nec orci et posuere. Suspendisse potenti. Quisque blandit auctor purus id facilisis est'
TestLD -Similarity 'stochastic'
$strC='a' * $strC.Length
$strD='a' * $strD.Length
TestLD -Similarity 'similar'
$strD='x' * $strD.Length
TestLD -Similarity 'different'

The main script 164518.ps1 (functions are defined here in the reverse order):

function Get-LevenshteinDistance{
    param(
        [string]$CompareString,
        [string]$DifferenceString
    )

    # Collect the string lengths
    $compareStringLength    = $CompareString.Length
    $differenceStringLength = $DifferenceString.Length

    # If either of the strings are empty the LD is known and we can stop
    if($compareStringLength -eq 0){return $differenceStringLength}
    if($differenceStringLength -eq 0){return $compareStringLength}
    # If strings are equal the LD is known and we can stop
    if ($CompareString -ceq $DifferenceString) { return 0 }

    # Build the comparison matrix as a (rectangular) jagged array
  # $comparisonMatrix = New-Object 'object[]' ($differenceStringLength+1)
    $comparisonMatrix = [System.Array]::CreateInstance([System.Object], ($differenceStringLength+1))
    for($index=0; $index -le $differenceStringLength; $index++){
        # Create row
      # $comparisonMatrix[$index]=New-Object 'object[]'  ($compareStringLength+1)
        $comparisonMatrix[$index]=[System.Array]::CreateInstance([System.Object], ($compareStringLength+1))
        # Populate the first column
        $comparisonMatrix[$index][0]=$index
    }
    # Populate the first row
    for($index=0; $index -le $compareStringLength; $index++){
        $comparisonMatrix[0][$index]=$index
    }

    # Calculate the Levenshtein distance by working through each position in the matrix.
    # Working the columns
    for($columnIndex=1; $columnIndex -le $compareStringLength; $columnIndex++){
        # Cycle each character in the list
        $compareCharacter = $compareString[$columnIndex-1] # multiple use => to a variable

        # Working the rows
        for($rowIndex=1; $rowIndex -le $differenceStringLength; $rowIndex++){
            # Cycle each character in the list (a variable for the only use)
          # $differenceCharacter = $differenceString[$rowIndex-1] # the only use

            # Calculate the cost
          if($compareCharacter -ceq $differenceString[$rowIndex-1]){
            $comparisonMatrix[$rowIndex][$columnIndex] = $comparisonMatrix[($rowIndex -1)][($columnIndex -1)]
          } else {
          # $cost = 1           # unneeded variable for the only use
            # The cell immediately above plus 1
            $cellAbove = $comparisonMatrix[($rowIndex-1)][$columnIndex] + 1

            # The cell immediately to the left plus 1
            $cellLeft =  $comparisonMatrix[$rowIndex][($columnIndex-1)] + 1

          # # The cell diagonally above and to the left             plus the cost ↓
            $cellUpperLeft = $comparisonMatrix[($rowIndex-1)][($columnIndex-1)] + 1

            # Select minumum of the of the last 3 cells calculations and assign it to the current matrix position.
            $comparisonMatrix[$rowIndex][$columnIndex] = [math]::Min([math]::Min($cellAbove,$cellLeft),$cellUpperLeft)
          }
        }
    }
    # The cacluated LD will now be in the bottom right of the matrix.
    return $comparisonMatrix[$differenceStringLength][$compareStringLength]
}

function Get-LevenshteinDistanceTry2{
    param(
        [string]$CompareString,
        [string]$DifferenceString
    )

    # Collect the string lengths
    $compareStringLength    = $CompareString.Length
    $differenceStringLength = $DifferenceString.Length

    # If either of the strings are empty the LD is known and we can stop
    if($compareStringLength -eq 0){return $differenceStringLength}
    if($differenceStringLength -eq 0){return $compareStringLength}
    # If strings are equal the LD is known and we can stop
    if ($CompareString -ceq $DifferenceString) { return 0 }
    # Build the comparison matrix and populate the first column and first row.
    $comparisonMatrix = New-Object 'object[,]' ($differenceStringLength+1),($compareStringLength+1)
  # $comparisonMatrix = [System.Array]::CreateInstance([System.Object], ($differenceStringLength+1),($compareStringLength+1))
    # Populate the first row
    for($index=0; $index -le $compareStringLength; $index++){
        $comparisonMatrix[0,$index]=$index
    }
    # Populate the first column
    for($index=0; $index -le $differenceStringLength; $index++){
        $comparisonMatrix[$index,0]=$index
    }

    # Calculate the Levenshtein distance by working through each position in the matrix.
    # Working the columns
    for($columnIndex=1; $columnIndex -le $compareStringLength; $columnIndex++){
        # Cycle each character in the list
        $compareCharacter = $compareString[$columnIndex-1]

        # Working the rows
        for($rowIndex=1; $rowIndex -le $differenceStringLength; $rowIndex++){
            # Cycle each character in the list
            $differenceCharacter = $differenceString[$rowIndex-1]

            # Calculate the cost
          if($compareCharacter -ceq $differenceCharacter){
            $comparisonMatrix[$rowIndex,$columnIndex] = $comparisonMatrix[($rowIndex -1),($columnIndex -1)]
          } else {
            $cost = 1    # a variable for the only use? Useless!

            # The cell immediately above plus 1
            $cellAbove = $comparisonMatrix[($rowIndex-1), $columnIndex] + 1

            # The cell immediately to the left plus 1
            $cellLeft =  $comparisonMatrix[$rowIndex,($columnIndex-1)] + 1

            # The cell diagonally above and to the left plus the cost
            $cellUpperLeft = $comparisonMatrix[($rowIndex-1),($columnIndex-1)] + $cost

            # Select minumum of the of the last 3 cells calculations and assign it to the current matrix position.
          # $comparisonMatrix[$rowIndex,$columnIndex] = $cellAbove,$cellLeft,$cellUpperLeft | Measure-Object -Minimum | select -ExpandProperty Minimum
          # $comparisonMatrix[$rowIndex,$columnIndex] = ($cellAbove,$cellLeft,$cellUpperLeft | Measure-Object -Minimum).Minimum
            $comparisonMatrix[$rowIndex,$columnIndex] = [math]::Min([math]::Min($cellAbove,$cellLeft),$cellUpperLeft)
          }
        }
    }
    # The cacluated LD will now be in the bottom right of the matrix.
    return $comparisonMatrix[$differenceStringLength,$compareStringLength]
}

function Get-LevenshteinDistanceTry1{
    #[cmdletbinding()]
    param(
        [string]$CompareString,
        [string]$DifferenceString
    )

    # Collect the string lengths
    $compareStringLength = $CompareString.Length
    $differenceStringLength = $DifferenceString.Length
    ##Write-Verbose "Comparision String: '$CompareString' with length '$compareStringLength'"
    ##Write-Verbose "Difference String: '$DifferenceString' with length '$differenceStringLength'"

    # If either of the strings are empty the LD is known and we can stop
    if($compareStringLength -eq 0){return $differenceStringLength}
    if($differenceStringLength -eq 0){return $compareStringLength}

    # Build the comparison matrix and populate the first column and first row.
    $comparisonMatrix = New-Object 'object[,]'  ($differenceStringLength+1),($compareStringLength+1)

    # Populate the first row
    for($index=0; $index -le $compareStringLength; $index++){
        $comparisonMatrix[0,$index]=$index
    }

    # Populate the first column
    for($index=0; $index -le $differenceStringLength; $index++){
        $comparisonMatrix[$index,0]=$index
    }

    # Calculate the Levenshtein distance by working through each position in the matrix.
    # Working the columns
    for($columnIndex=1; $columnIndex -le $compareStringLength; $columnIndex++){
        # Cycle each character in the list
        $compareCharacter = $compareString[$columnIndex-1]

        # Working the rows
        for($rowIndex=1; $rowIndex -le $differenceStringLength; $rowIndex++){
            # Cycle each character in the list
            $differenceCharacter = $differenceString[$rowIndex-1]

            ##Write-Verbose "Matrix location: [$rowIndex, $columnIndex]"
            ##Write-Verbose "Compare character: $compareCharacter - Difference character: $differenceCharacter"
            # Calculate the cost
            $cost=if($compareCharacter -ceq $differenceCharacter){0}else{1}
            ##Write-Verbose "Cost: $cost"

            # The cell immediately above plus 1
            $cellAbove = $comparisonMatrix[($rowIndex-1), $columnIndex] + 1
            ##Write-Verbose "Cell Above: [$($rowIndex-1), $columnIndex] + 1 = $cellAbove"

            # The cell immediately to the left plus 1
            $cellLeft =  $comparisonMatrix[$rowIndex,($columnIndex-1)] + 1
            ##Write-Verbose "Cell Left: [$rowIndex,$($columnIndex-1)] + 1 = $cellLeft"

            # The cell diagonally above and to the left plus the cost
            $cellUpperLeft = $comparisonMatrix[($rowIndex-1),($columnIndex-1)] + $cost
            ##Write-Verbose "Cell Upper Left: [$($rowIndex-1),$($columnIndex-1)] + cost($cost) = $cellUpperLeft"

            # Select minumum of the of the last 3 cells calculations and assign it to the current matrix position.
          # $comparisonMatrix[$rowIndex,$columnIndex] = $cellAbove,$cellLeft,$cellUpperLeft | Measure-Object -Minimum | select -ExpandProperty Minimum
            $comparisonMatrix[$rowIndex,$columnIndex] = [math]::Min([math]::Min($cellAbove,$cellLeft),$cellUpperLeft)
        }
    }
    # The cacluated LD will now be in the bottom right of the matrix.
    return $comparisonMatrix[$differenceStringLength,$compareStringLength]
}

function Get-LevenshteinDistanceOrig+{
    [cmdletbinding()]
    param(
        [string]$CompareString,
        [string]$DifferenceString
    )

    # Collect the string lengths
    $compareStringLength = $CompareString.Length
    $differenceStringLength = $DifferenceString.Length
    ##Write-Verbose "Comparision String: '$CompareString' with length '$compareStringLength'"
    ##Write-Verbose "Difference String: '$DifferenceString' with length '$differenceStringLength'"

    # If either of the strings are empty the LD is known and we can stop
    if($compareStringLength -eq 0){return $differenceStringLength}
    if($differenceStringLength -eq 0){return $compareStringLength}

    # Build the comparison matrix and populate the first column and first row.
    $comparisonMatrix = New-Object 'object[,]'  ($differenceStringLength+1),($compareStringLength+1)

    # Populate the first row
    for($index=0; $index -le $compareStringLength; $index++){
        $comparisonMatrix[0,$index]=$index
    }

    # Populate the first column
    for($index=0; $index -le $differenceStringLength; $index++){
        $comparisonMatrix[$index,0]=$index
    }

    # Calculate the Levenshtein distance by working through each position in the matrix.
    # Working the columns
    for($columnIndex=1; $columnIndex -le $compareStringLength; $columnIndex++){
        # Cycle each character in the list
        $compareCharacter = $compareString[$columnIndex-1]

        # Working the rows
        for($rowIndex=1; $rowIndex -le $differenceStringLength; $rowIndex++){
            # Cycle each character in the list
            $differenceCharacter = $differenceString[$rowIndex-1]

            ##Write-Verbose "Matrix location: [$rowIndex, $columnIndex]"
            ##Write-Verbose "Compare character: $compareCharacter - Difference character: $differenceCharacter"
            # Calculate the cost
            $cost=if($compareCharacter -ceq $differenceCharacter){0}else{1}
            ##Write-Verbose "Cost: $cost"

            # The cell immediately above plus 1
            $cellAbove = $comparisonMatrix[($rowIndex-1), $columnIndex] + 1
            ##Write-Verbose "Cell Above: [$($rowIndex-1), $columnIndex] + 1 = $cellAbove"

            # The cell immediately to the left plus 1
            $cellLeft =  $comparisonMatrix[$rowIndex,($columnIndex-1)] + 1
            ##Write-Verbose "Cell Left: [$rowIndex,$($columnIndex-1)] + 1 = $cellLeft"

            # The cell diagonally above and to the left plus the cost
            $cellUpperLeft = $comparisonMatrix[($rowIndex-1),($columnIndex-1)] + $cost
            ##Write-Verbose "Cell Upper Left: [$($rowIndex-1),$($columnIndex-1)] + cost($cost) = $cellUpperLeft"

            # Select minumum of the of the last 3 cells calculations and assign it to the current matrix position.
            $comparisonMatrix[$rowIndex,$columnIndex] = $cellAbove,$cellLeft,$cellUpperLeft | Measure-Object -Minimum | select -ExpandProperty Minimum
        }
    }
    # The cacluated LD will now be in the bottom right of the matrix.
    return $comparisonMatrix[$differenceStringLength,$compareStringLength]
}

function Get-LevenshteinDistanceOrig{
    [cmdletbinding()]
    param(
        [string]$CompareString,
        [string]$DifferenceString
    )

    # Collect the string lengths
    $compareStringLength = $CompareString.Length
    $differenceStringLength = $DifferenceString.Length
    Write-Verbose "Comparision String: '$CompareString' with length '$compareStringLength'"
    Write-Verbose "Difference String: '$DifferenceString' with length '$differenceStringLength'"

    # If either of the strings are empty the LD is known and we can stop
    if($compareStringLength -eq 0){return $differenceStringLength}
    if($differenceStringLength -eq 0){return $compareStringLength}

    # Build the comparison matrix and populate the first column and first row.
    $comparisonMatrix = New-Object 'object[,]'  ($differenceStringLength+1),($compareStringLength+1)

    # Populate the first row
    for($index=0; $index -le $compareStringLength; $index++){
        $comparisonMatrix[0,$index]=$index
    }

    # Populate the first column
    for($index=0; $index -le $differenceStringLength; $index++){
        $comparisonMatrix[$index,0]=$index
    }

    # Calculate the Levenshtein distance by working through each position in the matrix.
    # Working the columns
    for($columnIndex=1; $columnIndex -le $compareStringLength; $columnIndex++){
        # Cycle each character in the list
        $compareCharacter = $compareString[$columnIndex-1]

        # Working the rows
        for($rowIndex=1; $rowIndex -le $differenceStringLength; $rowIndex++){
            # Cycle each character in the list
            $differenceCharacter = $differenceString[$rowIndex-1]

            Write-Verbose "Matrix location: [$rowIndex, $columnIndex]"
            Write-Verbose "Compare character: $compareCharacter - Difference character: $differenceCharacter"
            # Calculate the cost
            $cost=if($compareCharacter -ceq $differenceCharacter){0}else{1}
            Write-Verbose "Cost: $cost"

            # The cell immediately above plus 1
            $cellAbove = $comparisonMatrix[($rowIndex-1), $columnIndex] + 1
            Write-Verbose "Cell Above: [$($rowIndex-1), $columnIndex] + 1 = $cellAbove"

            # The cell immediately to the left plus 1
            $cellLeft =  $comparisonMatrix[$rowIndex,($columnIndex-1)] + 1
            Write-Verbose "Cell Left: [$rowIndex,$($columnIndex-1)] + 1 = $cellLeft"

            # The cell diagonally above and to the left plus the cost
            $cellUpperLeft = $comparisonMatrix[($rowIndex-1),($columnIndex-1)] + $cost
            Write-Verbose "Cell Upper Left: [$($rowIndex-1),$($columnIndex-1)] + cost($cost) = $cellUpperLeft"

            # Select minumum of the of the last 3 cells calculations and assign it to the current matrix position.
            $comparisonMatrix[$rowIndex,$columnIndex] = $cellAbove,$cellLeft,$cellUpperLeft | Measure-Object -Minimum | select -ExpandProperty Minimum
        }
    }
    # The cacluated LD will now be in the bottom right of the matrix.
    return $comparisonMatrix[$differenceStringLength,$compareStringLength]
}

Please note case sensitivity of the above functions. For case insensitive Levenshtein Distance:

permanently: change -ceq to -eq in their two occurrences, or
ad hoc, in a particular case: use .ToUpper() or .ToLower() functions, e.g. as

Get-LevenshteinDistance -CompareString $strC.ToUpper() -DifferenceString $strD.ToUpper()
# or
Get-LevenshteinDistance -CompareString $strC.ToLower() -DifferenceString $strD.ToLower()


Wait until all files are loaded asynchronously, then run code I'm an experienced programmer but not too great at JavaScript so I'm looking to see if I'm doing this 'right'. I want to have several files loaded in (Ajax or really AJAJ) and, once loaded, run some final code. My basic idea is:

var jobsToDo = 2; // global

function getAsync(url, func) {
    var client = new XMLHttpRequest();
    var handler = function() {
        func();
        jobsToDo--;
        if(jobsToDo == 0)
            allLoaded();
    };
    client.open('GET', url);
    client.onreadystatechange = handler;
    client.send();
}

getAsync('file1.json', function() {
    // process file
});
getAsync('file2.json', function() {
    // process file
});

function allLoaded() {
    // ...
}

Is this correct? Is it a reasonable solution, or are there better ways? Actually onreadystatechange already feels like the wrong thing; is there a better handler that will wait until I get a 200, or should I expand my function above to exit unless the state is appropriate?

Also, since I don't do much JavaScript these days (or all the cool kids calling it ECMAScript now?): will I run into trouble with the local function being passed around?

My current code has only one file being loaded, so I'm trying to extract the basic functionality here to keep the code DRY.

Meta: I don't know if this belongs here or on SO; feel free to move if I made the wrong choice.

1) Nah, the kids still call it JavaScript, but ECMAScript works too. More power to you for knowing both names :)

2) Yes, I'd say you should wait for a less ambiguous state to occur before considering the request done. But there's not really anything better than onreadystatechange (as far as I recall) to check for the request's success. So you still have to code a "ready-state change" handler, just make it check client.status === 200 (and possibly some headers and whatnot) before deciding what to call next.

Other than that, though, your code's good. The hardcoded 2 would need to go for this to be of general use, but the basic idea of decrementing a counter is solid.

You could take a look at the futures and promises pattern. There are several JS-implementations of this you can use.

jQuery has such an implementation (via the Deferred object) built into it's Ajax-system, allowing you to do, say:

var file1 = $.getJSON("file1.json"), // $.getJSON returns a Deferred
    file2 = $.getJSON("file2.json"),
    all   = $.when(file1, file2);    // and $.when groups several Deferreds

// example usage - you can do the same for the individual files
all.done(function () {
  // something to call when all files have been successfully loaded
});

all.fail(function () {
  // something to call in case one or more files fail
});

all.always(function () {
  // something to always call (like, say, hiding a "loading" indicator)
});

Point is that Deferred lets you attach handlers asynchronously - the requests are sent the moment you call getJSON, but you can attach handlers whenever. And you can add several handlers to the same state, if you want. If the response has already been received, the handler will simply be called immediately.

Now, I know this is all jQuery, and perhaps you don't want to use that. If so, more power to you (again). It is however a neat approach and a simple API - or (in my opinion) one to emulate if you so choose.

Seeing @Kevindra's simple answer (go upvote!), I'm reminded of a different pattern I've used before:

 // a simple factory function
 function makeCounter(limit, callback) {
   return function () {
     if( --limit === 0 ) {
       callback();
     }
   }
 }

 // ....

 // make a "done" function set up to expect 2 invocations
 // before calling its callback
 var done = makeCounter(2, function () {
   // all done!
 });

 getAsync("file1.json", function () {
   // process file and call the done() function
   done();
 )};

 getAsync("file2.json", function () {
   // process file and call the done() function
   done();
 )};


Checking if a number is prime Is there a better way to check if a number is a prime?

#include <iostream>
#include <math.h>

using namespace std;

bool doAgain();
bool isPrime(int num);

int main()
{
    do {
        int num = 0;
        do {
            cout << "Enter a positive integer to check: ";
            cin >> num;
        } while(num < 1);
        if(isPrime(num)) {
            cout << "Prime!!!" << endl;
        } else {
            cout << "Not a prime." << endl;
        }
    } while(doAgain());
    cout << "Bye!" << endl;
}

bool doAgain() {
    while(true) {
        cout << "Again? (Y/N) ";
        char again;
        cin >> again;
        if(again == 'Y') {
            return true;
        } else if(again == 'N') {
            return false;
        }
    }
}

bool isPrime(int num) {
    if(num < 2) {
        return false;
    } else if(num == 2) {
        return true;
    } else if(num % 2 == 0) {
        return false;
    }
    for(int i = 3, max = sqrt(num); i < max; i += 2) {
        if(num % i == 0) {
            return false;
        }
    }
    return true;
}

Unfortunately, your isPrime function returns incorrect results for the squares of odd numbers.

This is a pretty common oversight though, so don't feel too bad.

A better implementation of isPrime looks more like this:

bool isPrime(int num) {
    if (num <= 3) {
        return num > 1;
    } else if (num % 2 == 0 || num % 3 == 0) {
        return false;
    } else {
        for (int i = 5; i * i <= num; i += 6) {
            if (num % i == 0 || num % (i + 2) == 0) {
                return false;
            }
        }
        return true;
    }
}

In the first if, we handle the special cases of 0 through 3 as well as all of the negative numbers.

In the second, we eliminate all of the multiples of 2 and 3.

Finally, in the catch-all else, we're handling everything else.

By starting at 5 and incrementing by 6, we're able to skip all of our multiples of 2 and 3 which we already eliminated.  So we're checking 2/3rds of the numbers your original implementation checks.

Moreover, because we're dealing with integers, i * i <= num is a bit better than i < sqrt(num) (which actually needs to be <=).


Tell, Don't Ask when dealing with displaying model attributes or a null replacement I have the following model:

# == Schema Information
#
# Table name: clients
#
#  id         :integer          not null, primary key
#  first_name :string(255)
#  last_name  :string(255)
#  email      :string(255)
#

class Client < ActiveRecord::Base
  def full_name
    if first_name.blank? && last_name.blank?
      'Name: unknown'
    else
      first_name + ' ' + last_name
    end
  end

  def email_address
    email.blank? ? 'Email: unknown' : email
  end
end

Which has the following spec:

RSpec.describe Client, type: :model do

  describe 'full_name' do
    it 'should return first_name + last_name if not blank' do
      @client = create(:client, first_name: 'John', last_name: 'Doe')
      @client.full_name.should == 'John Doe'
    end

    it "should return 'Name: unkonwn' if name is blank" do
      @client = create(:client, first_name: '', last_name: '')
      @client.full_name.should == 'Name: unknown'
    end
  end

  describe 'email_address' do
    it 'should return email if email is not blank' do
      @client = create(:client, email: 'example@example.com')
      @client.email_address.should == 'example@example.com'
    end

    it "should return 'Email: unkonwn' if email is blank" do
      @client = create(:client, email: '')
      @client.email_address.should == 'Email: unknown'
    end
  end
end

As you can see I am creating methods (email_address, full_name) to display attributes (or a combination of attributes) in views.  If the attribute is blank I want to return a string saying the value is unknown.

I have many more attributes that I would like to display this way, but it seems inefficient to come up with alternative method names for methods that simply return the attribute or the "blank alternative".

As an example, these are some of the other attributes: sex, age, source.  Rather than calling client.sex or client.age I would create a method and call client.name_of_sex or client.age_string -- it starts to get awkward (I went from email to email_address in the above example).

The alternative I know is to put logic in my views:

<% if client.email.blank? %>
  Email: unknown
<% else %>
  <%= client.email %>
<% end %>

This is inappropriate for a number of reasons, but my current alternative (the example above) doesn't seem to be optimal.

Is this a point where I should get into making a view-model? Is there some other way I could elegantly perform the actions I want?  Or is the way I'm doing it a reasonable thing to do (I'm going to have a lot of attributes I want to display this way)?

Definitely, this does not belong in the model. The orthodox way is to create a helper. Note how you can use the pattern Object#presence:

module ApplicationHelper
  def show_field(field, value)
    value.presence || "#{field}: unknown"
  end
end

And use it in the views:

<%= show_field("Name", user.full_name) %>

You can go a step further and use OOP view presenters if you like the idea (personally I don't use them).


Palindrome Validator This code returns true if the sentence is a palindrome, and false otherwise. Is there a better way of doing this?

def palindrome?(sentence)

  array_stripped_sentence = sentence.reverse.downcase.delete('').split('')
  array_stripped_sentence == sentence.downcase.delete(' ').split('')

end

puts palindrome?("Never odd or even")

You can call reverse on String directly.

While it doesn't particularly apply here, instead of splitting on '', you should use the method chars.

Who says you'll be passing a sentence? Does this method not work for single words? If I call palindrome? 'radar' will I get an error?

I'd recommend using tr to strip out all non-alphabetic characters, instead of just deleteing spaces.

def palindrome?(testing)
  stripped = testing.downcase.tr('^a-z', '')
  stripped.reverse == stripped
end


Reverse digits and add until a palindrome appears The following code is a C solution to the following problem UVA 10018.

  The Problem

  The "reverse and add" method is simple: choose a number, reverse its
  digits and add it to the original. If the sum is not a palindrome
  (which means, it is not the same number from left to right and right
  to left), repeat this procedure.

  For example:

  195 Initial number
591

786
687

1473
3741

5214
4125

9339 Resulting palindrome

  In this particular case the palindrome 9339 appeared after the 4th
  addition. This method leads to palindromes in a few step for almost
  all of the integers. But there are interesting exceptions. 196 is the
  first number for which no palindrome has been found. It is not proven
  though, that there is no such a palindrome.

  Task

  You must write a program that give the resulting palindrome and the
  number of iterations (additions) to compute the palindrome.

  You might assume that all tests data on this problem:

  will have an answer
  will be computable with less than 1000 iterations (additions)
  will yield a palindrome that is not greater than 4,294,967,295.

  The Input

  The first line will have a number N with the number of test cases, the
  next N lines will have a number P to compute its palindrome.

  The Output

  For each of the N tests you will have to write a line with the
  following data : minimum number of iterations (additions) to get to
  the palindrome and the resulting palindrome itself separated by one
  space.

  Sample Input

3
195
265
750

  Sample Output

4 9339
5 45254
3 6666

My solution is as follows:

#include <stdio.h>

/* only works for unsigned longs */
unsigned long reverse(unsigned long original)
{
  unsigned long number = original;
  unsigned long reversed = 0;
  unsigned long place = 1;
  unsigned long i, q;

  for(i = 1000000000; i > 0; i = i / 10) {

    q = number / i;

    if ((q > 0) || (reversed > 0)) {
      reversed = reversed + (q * place);
      number = number - (q * i);
      place = place * 10;
    }
  }

  return(reversed);
}

int is_palindrome(unsigned long original)
{
  unsigned long reversed;
  reversed = reverse(original);
  return(reversed == original);
}

unsigned long reverse_add(unsigned long original, int *iterations)
{
  if (is_palindrome(original)) {
    return(original);
  }

  if (*iterations >= 1000) {
    return(0);
  }

  *iterations = *iterations + 1;
  reverse_add(original + reverse(original), iterations);
}

int main()
{
  int n = 0;
  unsigned long start;
  unsigned long end;
  int iterations = 0;

  while (scanf("%ld\n", &start) == 1) {
    if (n == 0) {
      n = start;
    }
    else {
      iterations = 0;
      end = reverse_add(start, &iterations);
      printf("%d %ld\n", iterations, end);
    }
  }

  return(0);
}

reverse()

Your reverse() function only works for numbers below 1010.  Although that upper bound may be good enough to solve this particular challenge, it's not correct in general.  (Keep in mind that the standard says that an unsigned long is at least 32 bits — possibly larger.)

Here is a simpler implementation that works in general:

unsigned long reverse(unsigned long original) {
    unsigned long reversed = 0;
    for (; original; original /= 10) {
        reversed = (10 * reversed) + (original % 10);
    }
    return reversed;
}

is_palindrome()

For every iteration, you end up calling reverse() twice: once to test whether a palindrome has appeared, and again to obtain the next number to add.  You would be better off eliminating this function, and rolling the palindrome check into the reverse_add() loop itself.

reverse_add()

For clarity, I would rename the function to reverse_add_until_paindrome().

Conventionally, C functions are designed so that a status code is returned, and the data being manipulated are passed by reference.  I recommend redesigning the interface so that it returns the number of iterations, as that is closer in spirit to a status code.  The number itself can be passed by reference.  Notice what happens then: it becomes an in-out parameter!  The rationale for the convention makes itself apparent.

int reverse_add_until_palindrome(unsigned long *n) {
    int i;
    unsigned long rev;
    for (i = 0; (rev = reverse(*n)) != *n; i++) {
        *n += rev;
    }
    return i;
}

The problem statement guarantees that you will never be given a P that will require more than 1000 iterations.  That's just for your information; I don't think you need to verify that fact.  (If you do want to bail out, you should add code in main() to detect that the computation was aborted.)

main()

The way you try to use one scanf() to read both N and the Pi is awkward.  Not only does that make the code less clear, it also imposes a penalty of an extra conditional on every subsequent loop.  In addition, it's wrong: the format string for an int is "%d", whereas the format string for an unsigned long is "%lu".

You read n, but never make use of it.  That works, I suppose.  I prefer to use it to see whether all of the expected input was received.

#include <stdio.h>
#include <stdlib.h>

int main() {
    int n;
    unsigned long p;

    if (1 != scanf("%d", &n)) {
        return EXIT_FAILURE;
    }
    while (n-- && scanf("%lu", &p)) {
        int iterations = reverse_add_until_palindrome(&p);
        printf("%d %lu\n", iterations, p);
    }
    return (n == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}


Selecting correct carton based on qty I'm attempting to create a function that will select the correct carton based on the qty of the carton content.

Here are my cartons that have the number of items they can hold:

SMALL_PASCAL = 300
BIG_PASCAL = 600
BABY_BOX = 1200
A485_1201 = 1800
A4140_1901 = 3000
A485 = 5000

And here is the method that will return the CartonType:

/// <summary>
/// Get Carton Type
/// </summary>
/// <param name="qty"></param>
/// <returns></returns>
[Test]
private static CartonType GetCartonType(int qty)
{
     if (qty <= 300)
     {
         return CartonType.SMALL_PASCAL;
     }
     else if (qty > 300 && qty <= 600)
     {
         return CartonType.SMALL_PASCAL;
     }
     else if (qty > 600 && qty <= 1200)
     {
         return CartonType.BABY_BOX;
     }
     else if (qty > 1200 && qty <= 1800)
     {
         return CartonType.A485_1201;
     }
     else if (qty >1800 && qty <=3000)
     {
         return CartonType.A4140_1901;
     }
     else // 5000 or more.
     {
         return CartonType.A485;
     }
}

Calling the method like so:

int qty = 1540;
Console.WriteLine(GetCartonType(qty));

Output:

A485_1201

Is there a better way to achieve this rather than an if statement? Also, I just had a thought that what if the qty is like 10,000? I would then require 2 A485.

Is there a better way to achieve this rather than an If statement?

Yes. You could have an enumeration of the container types and their capacities, in increasing order by capacity,
loop over in order,
and as soon as you find one that's big enough, return it.

It's perhaps easier to see after you simplify the if-else chain,
by removing redundant conditions, for example:

 if (qty <= 300)
 {
     return CartonType.SMALL_PASCAL;
 }
 if (qty <= 600)
 {
     return CartonType.SMALL_PASCAL;
 }
 if (qty <= 1200)
 {
     return CartonType.BABY_BOX;
 }
 // ...


Character Creation in Text Adventure; Proper use of Main and Global Values? I'm still learning python so I decided to experiment by making a tiny text adventure. I'm only a student in highschool so I haven't had the chance to learn many common techniques used in code as most of my learning has been self taught, so I wanted to take this opportunity to have some kind of review of some of the techniques I use in this code.

Here it is in it's entirety:

class Character:
    def __init__(self, class_name, strength, dexterity, intelligence):
        self.class_name = class_name
        self.strength = strength
        self.dexterity = dexterity
        self.intelligence = intelligence
        self.health = strength*10

    def print_stats(self):
        return self.class_name + " Strength: " + str(self.strength) + " Dexterity: " + str(self.dexterity) + " Intelligence: " + str(self.intelligence)

    def print_stat_strength(self):
        return "Strength: " + str(self.strength)

    def print_stat_dexterity(self):
        return "Dexterity: " + str(self.strength)

    def print_stat_intelligence(self):
        return "Intelligence: " + str(self.strength)

    def print_stat_health(self):
        return "Max Health: " + str(self.health)

    def change_stat_strength(self, new_value):
        self.strength = new_value

    def change_stat_dexterity(self, new_value):
        self.dexterity = new_value

    def change_stat_intelligence(self, new_value):
        self.intelligence = new_value

def character_select():
    global player_class
    barbarian = Character("Barbarian", 20, 10, 10)
    archer = Character("Archer", 10, 20, 10)
    wizard = Character("Wizard", 10, 10, 20)

    print("Welcome to TEST TEXT ADVENTURE RPG. Choose a character!")
    print("(A): " + barbarian.print_stats())
    print("(B): " + archer.print_stats())
    print("(C): " + wizard.print_stats())

    while True:
        user_answer = input().upper()
        if user_answer == "A":
            player_class = barbarian
            break
        elif user_answer == "B":
            player_class = archer
            break
        elif user_answer == "C":
            player_class = wizard
            break
        else:
            print("Invalid input! Try again.")
    print("####CHARACTER CREATION COMPLETE####")

def main():
    character_select()
    print(player_class.print_stats())

main()

Looking at this, my main concern is how the main function is utilized. To my knowledge it is supposed to be relatively clean and made up of other functions, lacking a lot of original code of its own. Making the above, I came into an issue trying to use the player_class object that I created, since it was created in the character_select function, it could only be used there unless it was global. The global fix seems kind of clunky so I was wondering the proper way to implement this kind of thing.

One other concern that I can identity could be the built in functions in the Character class. Do I need to make a separate function for every single stat like strength, dexterity, intelligence, or is it possible to do something like this?

def change_stat(self, stat_to_change, new_number):
    self.stat_to_change = new_number

Now I'm asking this question because I tried the above code on my own to try to cut down on the amount of similar functions for each individual stat, but I couldn't seem to use it how I imagined.

Those are my main concerns but anybody reading this is welcome to bring up any other improper coding technique. I really don't want bad habits to manifest themselves and continue to appear if I can help it.

There is a way to pass the attribute name which you want to change, you can use setattr:

def change_stat(self, stat_to_change, new_number):
    setattr(self, stat_to_change, new_number)

However, there is no difference to what would happen if you just say self.strength = 10 instead of self.change_stat("strength", 10).

Regarding the need for global for the player character, there isn't. Just return the chosen class from the character_select function:

def character_select():
    barbarian = Character("Barbarian", 20, 10, 10)
    archer = Character("Archer", 10, 20, 10)
    wizard = Character("Wizard", 10, 10, 20)

    print("Welcome to TEST TEXT ADVENTURE RPG. Choose a character!")
    print("(A): " + barbarian.print_stats())
    print("(B): " + archer.print_stats())
    print("(C): " + wizard.print_stats())

    while True:
        user_answer = input().upper()
        if user_answer == "A":
            return barbarian
        elif user_answer == "B":
            return archer
        elif user_answer == "C":
            return wizard
        else:
            print("Invalid input! Try again.")

You could make it even nicer if you used a dictionary:

from collections import OrderedDict

def character_select():
    classes = OrderedDict([("A", Character("Barbarian", 20, 10, 10)),
                           ("B", Character("Archer", 10, 20, 10)),
                           ("C", Character("Wizard", 10, 10, 20))])

    print("Welcome to TEST TEXT ADVENTURE RPG. Choose a character!")
    for key, cls in classes.items():
        print("({}): {}".format(key, cls.print_stats()))

    while True:
        user_answer = input().upper()
        try:
            return classes[user_answer]
        except KeyError:
            print("Invalid input! Try again.")

You can then use it like this in your main function:

def main():
    player_class = character_select()
    print(player_class.print_stats())

You should probably rename your Character.print_stats function. Right now it sounds like it would actually print the stats, but it just returns them. So I would make your class like this:

class Character:
    def __init__(self, class_name, strength, dexterity, intelligence):
        self.class_name = class_name
        self.strength = strength
        self.dexterity = dexterity
        self.intelligence = intelligence
        self.health = strength*10

    def __str__(self):
        return "{self.class_name} " \
               "Health: {self.health} " \
               "Strength: {self.strength} " \
               "Dexterity: {self.dexterity} " \
               "Intelligence: {self.intelligence}".format(self=self)

If you are using Python 3.6, you can use f"{self.class_name} ..." instead of "{self.class_name} ...".format(self=self), which uses f-strings.

Because I named the function __str__, you can now do:

barbarian = Character("Barbarian", 20, 10, 10)
print(barbarian)
# Barbarian Health: 200 Strength: 20 Dexterity: 10 Intelligence: 10

And the loop in character_select becomes this, because str.format automatically calls str on its arguments, which will then invoke the Character.__str__ function:

for key, cls in classes.items():
    print("({}): {}".format(key, cls))

__str__ is one of the magic (or dunder) methods, which can give your class features so it acts like built-in classes. Have a look at this nice guide for a list and explanation of all of them.

Finally, you should add a if __name__ == "__main__": guard to allow importing your functions from other scripts:

if __name__ == "__main__":
    main()


Sampling words from randomly chosen text files I've developed a script to randomly select 50 elements from a list of .txt files. This process is repeated 100 times. The script joins the randomly selected text files to one long string and then filters out the longest substring.

I want to run this script on a Droplet on Digital Ocean. The server, however, kills the script. When I randomly select only 3 elements, it works. Am I running out of memory? How can I tackle this problem?

Here are the files I randomly select items from. And this is my code:

# coding: utf-8

import glob
from collections import Counter
import pickle
import random

de_list_soz = pickle.load(open('de_list_soz.p', 'rb'))
str_seq_list = []

for str_seq in range(0,100):
    #creating random list
    random_list = []
    for item in range(0,50):
       list_item = random.choice(de_list_soz)
       random_list.append(list_item)

    #creating long string
    long_str = ''
    for de in random_list:

        input_file = open('txt_sr_de/txt_sr_de/' + de, 'r')
        text = input_file.read()
        text = text.replace('\n', ' ').replace('\xa0', '').replace('  ', '')
        #Removing these automated notifications
        text = text.replace('Wichtiger Hinweis:Diese Website wird in älteren Versionen von Netscape ohne graphische Elemente dargestellt. Die Funktionalität der Website ist aber trotzdem gewährleistet. Wenn Sie diese Website regelmässig benutzen, empfehlen wir Ihnen, auf Ihrem Computer einen aktuellen Browser zu installieren.Zurück zur Einstiegsseite Drucken Grössere Schrift', '')
        text = text.replace('Vorwärts ähnliche Leitentscheide suchenähnliche Urteile ab 2000 suchen Drucken nach oben', '')
        text = text.replace('Bundesgericht Tribunal fédéral Tribunale federale Tribunal federal', '')
        text = text.replace('Navigation Neue Suche Zurück zum Suchresultat Rang: Zurück 180', '')
        text = text.replace('Navigation Neue Suche Zurück zum Suchresultat Rang:1 ähnliche Leitentscheide suchenähnliche Urteile ab 2000 suchen Drucken nach oben', '')
        text = text.replace('  ', ' ')

        long_str = long_str + text

    times=3

    for n in range(1,int(len(long_str)/times+1)):
        substrings=[long_str[i:i+n] for i in range(len(long_str)-n+1)]
        freqs=Counter(substrings)
        if freqs.most_common(1)[0][1]<3:
            n-=1
            break
        else:
            seq=freqs.most_common(1)[0][0]

    str_seq_list.append(seq)

pickle.dump(str_seq_list, open('SOZIALRECHT_DE.p', 'wb'))

Since your code performs multiple things at once, you should split it into functions for readability. A first naive rewrite can yield:

from collections import Counter
import random

def filter_text(text):
    return (text
            .replace('\n', ' ')
            .replace('\xa0', '')
            .replace('  ', '')
            #Removing these automated notifications
            .replace('Wichtiger Hinweis:Diese Website wird in älteren Versionen von Netscape ohne graphische Elemente dargestellt. Die Funktionalität der Website ist aber trotzdem gewährleistet. Wenn Sie diese Website regelmässig benutzen, empfehlen wir Ihnen, auf Ihrem Computer einen aktuellen Browser zu installieren.Zurück zur Einstiegsseite Drucken Grössere Schrift', '')
            .replace('Vorwärts ähnliche Leitentscheide suchenähnliche Urteile ab 2000 suchen Drucken nach oben', '')
            .replace('Bundesgericht Tribunal fédéral Tribunale federale Tribunal federal', '')
            .replace('Navigation Neue Suche Zurück zum Suchresultat Rang: Zurück 180', '')
            .replace('Navigation Neue Suche Zurück zum Suchresultat Rang:1 ähnliche Leitentscheide suchenähnliche Urteile ab 2000 suchen Drucken nach oben', '')
            .replace('  ', ' '))

def create_string_from_files(files, root='txt_sr_de/txt_sr_de', sample=50):
    #creating random list
    random_list = []
    for item in range(sample):
       list_item = random.choice(files)
       random_list.append(list_item)

    #creating long string
    long_str = ''
    for de in random_list:

        input_file = open(os.path.join(root, de), 'r')
        text = input_file.read()

        long_str = long_str + filter_text(text)
    return long_str

def extract_most_common_sequence(text, minimum_occurences=3, times=3):
    for n in range(1,int(len(text)/times+1)):
        substrings=[text[i:i+n] for i in range(len(text)-n+1)]
        freqs=Counter(substrings)
        if freqs.most_common(1)[0][1] < minimum_occurences:
            n-=1
            return seq
        else:
            seq=freqs.most_common(1)[0][0]

def main(files, repeat=100):
    str_seq_list = []

    for str_seq in range(repeat):
        long_str = create_string_from_files(files)
        str_seq_list.append(extract_most_common_sequence(long_str))

    return str_seq_list

if __name__ == '__main__':
    import pickle
    de_list_soz = pickle.load(open('de_list_soz.p', 'rb'))
    str_seq_list = main(de_list_soz)
    pickle.dump(str_seq_list, open('SOZIALRECHT_DE.p', 'wb'))

Now we can start cleaning things up.

In create_string_from_files, you are basically reinventing random.sample:

>>> import random
>>> random.sample('abcdefghi', 4)
['h', 'e', 'c', 'f']

You also happen to open files but never close them: use the with statement to automatically handle that.

Lastly, concatenating long sentences manually is not memory efficient. Let's ask str.join to do it for us for now. But in order to do it nicely, we need to split the function further:

def read_and_filter_file(filename):
    with open(filename) as input_file:
        text = input_file.read()
    return filter_text(text)

def create_string_from_files(files, root='txt_sr_de/txt_sr_de', samples=50):
    return ''.join(
        read_and_filter_file(os.path.join(root, de))
        for de in random.sample(files, samples)
    )

The same kind of improvements can be made to the main function by replacing the "create empty list + for loop + append" template by a more efficient list-comprehension:

def main(files, repeat=100):
    return [
        extract_most_common_sequence(create_string_from_files(files))
        for _ in range(repeat)
    ]

The extract_most_common_sequence function can also be optimized by using some itertool recipe. I’m thinking about a variation of the pairwise recipe since your list-comprehension, with n being 2 is pretty much it. i.e. using:

>>> long_str = 'This is a test'
>>> n = 2
>>> [long_str[i:i+n] for i in range(len(long_str)-n+1)]
['Th', 'hi', 'is', 's ', ' i', 'is', 's ', ' a', 'a ', ' t', 'te', 'es', 'st']
>>> n = 4
>>> [long_str[i:i+n] for i in range(len(long_str)-n+1)]
['This', 'his ', 'is i', 's is', ' is ', 'is a', 's a ', ' a t', 'a te', ' tes', 'test']

So using something like:

def tuplewise(iterable, length):
    tees = itertools.tee(iterable, length)
    for i, t in enumerate(tees):
        for _ in xrange(i):
            next(t, None)
    return itertools.izip(*tees)

For Python 2 or:

def tuplewise(iterable, length):
    tees = itertools.tee(iterable, length)
    for i, t in enumerate(tees):
        for _ in range(i):
            next(t, None)
    return zip(*tees)

for Python 3, you can simplify the writting and save a bit on memory usage like so:

def extract_most_common_sequence(text, minimum_occurences=3, times=3):
    sequence = ''
    for n in range(1, int(len(text)/times+1)):
        freqs = Counter(tuplewise(text, n))
        (most_frequent, higher_frequency), = freqs.most_common(1)
        if higher_frequency < minimum_occurences:
            break
        sequence = ''.join(most_frequent)
    return sequence

Now I defined sequence = '' as a security measure in the unlikely event that the text is quite short an no letter appear more than twice. It's just to make the function return something in each and every cases.

Now that the code is more readable, let's tackle that memory issue of yours.

The thing to note is that aggregating the content of 50 files in one single string in memory is likely to blow things up. So instead, let's use some disk storage. We'll use the tempfile module for that. The idea is to store the filtered bits of text into a single file and read that file (over and over again) to get the desired most common sequence. We thus need to adapt our tuplewise a bit to either read one character at a time and yield words of the requested size or, better as I/O is concerned, repeatedly read a block of data of a fixed size and grab caracters out of it one by one to produce words of the desired size:

import os
import random
import itertools
import tempfile
from collections import Counter

def filter_text(text):
    return (text
            .replace('\n', ' ')
            .replace('\xa0', '')
            .replace('  ', '')
            #Removing these automated notifications
            .replace('Wichtiger Hinweis:Diese Website wird in älteren Versionen von Netscape ohne graphische Elemente dargestellt. Die Funktionalität der Website ist aber trotzdem gewährleistet. Wenn Sie diese Website regelmässig benutzen, empfehlen wir Ihnen, auf Ihrem Computer einen aktuellen Browser zu installieren.Zurück zur Einstiegsseite Drucken Grössere Schrift', '')
            .replace('Vorwärts ähnliche Leitentscheide suchenähnliche Urteile ab 2000 suchen Drucken nach oben', '')
            .replace('Bundesgericht Tribunal fédéral Tribunale federale Tribunal federal', '')
            .replace('Navigation Neue Suche Zurück zum Suchresultat Rang: Zurück 180', '')
            .replace('Navigation Neue Suche Zurück zum Suchresultat Rang:1 ähnliche Leitentscheide suchenähnliche Urteile ab 2000 suchen Drucken nach oben', '')
            .replace('  ', ' '))

def read_and_filter_file(filename):
    with open(filename) as input_file:
        text = input_file.read()
    return filter_text(text)

def create_string_from_files(output, files, root='txt_sr_de/txt_sr_de', samples=50):
    for filename in random.sample(files, samples):
        output.write(read_and_filter_file(os.path.join(root, filename)))

def read_chunks(file_object, chunk_size, block_size=4096):
    word = file_object.read(chunk_size)
    yield word
    while True:
        data = file_object.read(block_size)
        if not data:
            return
        for character in data:
            # Drop first character and append the next one
            word = word[1:] + character
            yield word

def extract_most_common_sequence(input_file, minimum_occurences=3, times=3):
    input_file.seek(0, os.SEEK_END)
    file_size = input_file.tell()
    sequence = ''

    for n in range(1, int(file_size/times+1)):
        input_file.seek(0)  # Be sure to read the whole file each time
        freqs = Counter(read_chunks(input_file, n))
        (most_frequent, higher_frequency), = freqs.most_common(1)
        if higher_frequency < minimum_occurences:
            break
        sequence = most_frequent
    return sequence

def main(files):
    with tempfile.NamedTemporaryFile(mode='w+') as storage:
        create_string_from_files(storage, files)
        return extract_most_common_sequence(storage)

if __name__ == '__main__':
    import pickle
    de_list_soz = pickle.load(open('de_list_soz.p', 'rb'))
    str_seq_list = [main(de_list_soz) for _ in range(100)]
    pickle.dump(str_seq_list, open('SOZIALRECHT_DE.p', 'wb'))

I changed main into returning a single element rather than a whole list as it feels cleaner with this implementation. The whole repetition thing is done in the test code now.


Refactor code to solve the 'Line is too long' error in rubocop Method.create(domain_id: domain.id, type_notification: type, message: title, url: url, owner_id: self.user.id)

How can I reduce this line to 80 ?

I'd just write the call with one line per key/value:

Method.create(
  domain_id: domain.id,
  type_notification: type,
  message: title,
  url: url,
  owner_id: user.id
)

Note that the explicit reference to self in method calls is not idiomatic (check the unofficial Ruby style guide for this and other useful tips).

Also, since Method is a class name already in the core, is not a good idea to have a model what that name.


Sum of numbers that equal 100 I was asked to find all the numbers that added up to 100 with no duplicates in Ruby. I float between Python, GoLang, and Ruby in a day. I am always concerned if I am doing something in the most optimal way.

require 'pp'
sumdata = [ 95, 5, 95, 5 ]

def find_sum(data)
  result = []
  processed = []
  data.each_with_index do |item, index |
    next if processed.include? index
    data.each_with_index do |other, other_index|
      next if processed.include? other_index # next if already processed
      # add tuple to list
      if item + other == 100
        result << [item, other].sort
        processed << index
        processed << other_index
      end
    end
  end
  result
end

This is built assuming you want no duplicates in the results, since the goal is a little unclear.
Quickly Finding Combinations
The function you should be using is Array.combination(n), which generates an enumerator that yields all combinations of size n from the array.  Once you have that it is a trivial matter to only select the combinations that sum to the target value.
My function here finds all combinations of size 2 to size data.length and then selects only the combinations with the correct sum.
def find_sum(data, target_sum)
  (2..data.length)
    .flat_map { |n| data.combination(n).to_a }
    .select { |arr| arr.inject(:+) == target_sum }
    #.uniq   # can call this to remove duplicates if necessary, depending on what the input data looks like
end

data = (1..9).to_a
pp find_sum(data, 10)
#[[1, 9],
# [2, 8],
# [3, 7],
# [4, 6],
# [1, 2, 7],
# [1, 3, 6],
# [1, 4, 5],
# [2, 3, 5],
# [1, 2, 3, 4]]


Recomendations on where to set an object properties (Repository Pattern) Simple question:

What would you rather do?

public class FooRepository
{
    public Foo GetById(int id)
    {
        Foo foo = null;
        using (IDataReader data = /* Get Foo's data */)
        {
            // Let the Foo object set it's own fields using the data object
            foo = new Foo(data);
        }
        return foo;
    }
}

Or...

public class FooRepository
{
    public Foo GetById(int id)
    {
        Foo foo = null;
        using (IDataReader data = /* Get Foo's data */)
        {
            // Let the Repository class set the Foo object fields
            foo = new Foo();
            foo.Id = Convert.ToInt32(data["FooID"]);
            foo.Name = data["Name"].ToString();
        }
        return foo;
    }
}

Assuming you can't use an ORM (NHibernate, L2S, EF) to handle this for you and that you will need to create Foo in other methods of the repository.

Any explanation will be welcome!

Part of the goal for using the Repository pattern is to separate the storage from the domain.  By passing the IDataReader in to a Foo object, I feel like the domain gains too much knowledge of the underlying storage mechanism.

So, between the options listed, I prefer setting the object's properties in the Repository.  That being said, if the object doesn't make sense without a certain property, it should still demand it during its creation.  However, even in this case, the Repository would still be responsible for setting all the properties requested.


String compare, sort, regex split I am writing simple code to accept/read a input of lines from a text file, split it into my class variables, sort them and finally display the input in the ordered form. But I have been struggling for the past couple of days to split the read line in appropriate manner.

static void Main(string[] args)
{
    List<MyClass> myClassList = new List<MyClass>();

    string strpath = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location) + "\\input.txt";
    if (File.Exists(strpath))
    {
        using (var sr = new StreamReader(strpath))
        {
            string[] strFirst, strSecond;
            while (sr.Peek() >= 0)
            {
                strFirst = Regex.Split(sr.ReadLine().Replace(" ", ""), "([0-9]+)");
                strFirst = Regex.Split(sr.ReadLine().Replace("|", ">>"), "([0-9]+)");

                myClassList.Add(new MyClass
                {
                   Number = strFirst[1], Name = strFirst[2], Separator = strFirst[1].Split(">>")}
                });

            }
        }
    }

    myList = GetResults(myClassList);

    foreach (MyClass myclass in myList)
        {
            Console.WriteLine(myclass[i].Number + myclass[i].Name + myclass[i].Separator);
            Console.WriteLine(myclass.Name);
        }

        Console.ReadLine();
    }

Sample input line from text file, which I want to process:

var input = "5 string one | 5.2 string two | 5.2.1 string three\r\n" +
                    "5 string four >> 5.6 string five >> string six\r\n" +
                    "1 string seven | 1.1 string eight | 1.1.1 string nine\r\n";

Output:

MyClass
Number[0] = 5
Name[0] = string one
Separator[0] = |

Number[1] = 5.2
Name[1] = string two
Separator[1] = |

Number[2] = 5.2.1
Name[2] = string three
Separator[2] =

Try this:

class Program
{
    static void Main(string[] args)
    {
        //var input = File.ReadAllText("input.txt");

        var input = "5 string one | 5.2 string two | 5.2.1 string three\r\n"+
                    "5 string four >> 5.6 string five >> string six\r\n" +
                    "1 string seven >> 1.1 string eight || 1.1.1 string nine\r\n";

        var matches = Regex.Matches(input, @"([0-9. ]*)(string \w*)([\|> $]*)",
            RegexOptions.Multiline);
        var template = "Number[{0}] = {1}\r\n" +
                       "Name[{0}] = {2}\r\n" +
                       "Separator[{0}] = {3}\r\n";
        var sb = new StringBuilder();
        sb.AppendLine("MyClass");
        int counter = 0;
        foreach (Match item in matches)
        {
            sb.AppendLine(String.Format(template,
                counter++,
                item.Groups[1].ToString().Trim(),
                item.Groups[2].ToString().Trim(),
                item.Groups[3].ToString().Trim()));
        }
        Console.WriteLine(sb.ToString());
    }
}

The output is:

MyClass
Number[0] = 5
Name[0] = string one
Separator[0] = |

Number[1] = 5.2
Name[1] = string two
Separator[1] = |

Number[2] = 5.2.1
Name[2] = string three
Separator[2] =

Number[3] = 5
Name[3] = string four
Separator[3] = >>

Number[4] = 5.6
Name[4] = string five
Separator[4] = >>

Number[5] =
Name[5] = string six
Separator[5] =

Number[6] = 1
Name[6] = string seven
Separator[6] = >>

Number[7] = 1.1
Name[7] = string eight
Separator[7] = ||

Number[8] = 1.1.1
Name[8] = string nine
Separator[8] =

Based on the request in your comment, I'm adding ordering of the output. Just replace the foreach statement above  with the following lines and make sure you add a using System.Dynamic; to the top of your class file:

List<dynamic> items = new List<dynamic>();
foreach (Match match in matches)
{
    dynamic item = new ExpandoObject();
    item.Index = counter++;
    item.Number = match.Groups[1].ToString().Trim();
    item.Name = match.Groups[2].ToString().Trim();
    item.Separator = match.Groups[3].ToString().Trim();
    items.Add(item);
}

var sb = new StringBuilder();
sb.AppendLine("MyClass");
foreach (var item in items.OrderBy(i => i.Number))
{
    sb.AppendLine(String.Format(template,
    item.Index,
    item.Number,
    item.Name,
    item.Separator));
}


Four-function and exponentiation console calculator This code works as it is supposed to:

#include<iostream>
#include<cmath>
#include<cstdlib>

using namespace std;

int main()
{
    char cAgain, type;
    int x, y=0;
    double addition(double num1, double num2), subtraction(double num1, double num2), division(double num1, double num2),
        multiplication(double num1, double num2), exponential(double num1, double num2), num1, num2, total;
    do
    {
        cout << "How many operations would you like to do?" << endl;
        cin >> x;
        if (x <= 0)
        {
            cout << "Error: You must have 1 or more operations." << endl;
        }
        else
        {
            while (x != 0, x--)
            {
                y++;
                cout << "Enter in your "<<y<< " operation. (First Number (+,-,*,/,^) Second Number)" << endl;
                cin >> num1 >> type >> num2;
                switch (type)
                {
                case '+':
                    total = addition(num1, num2);
                    cout << num1 << " + " << num2 << "= " << total<< endl;
                    break;
                case'-':
                    total = subtraction(num1, num2);
                    cout << num1 << " - " << num2 << "= " << total<< endl;
                    break;
                case'/':
                    total = division(num1, num2);
                    cout << num1 << " / " << num2 << "= " << total<< endl;
                    break;
                case'*':
                    total = multiplication(num1, num2);
                    cout << num1 << " * " << num2 << "= " << total << endl;
                    break;
                case'^':
                    total = exponential(num1, num2);
                    cout << num1 << " ^ " << num2 << "= " << total << endl;
                    break;
                }
            }
        }
        cout << "Would you like to run the program again.(Y/N)" << endl;
            cin >> cAgain;
    } while (cAgain == 'Y' || cAgain == 'y');
return 0;
}

double addition(double num1, double num2)
{
    double total;
    total = num1 + num2;
    return (total);
}
double subtraction(double num1, double num2)
{
    double total;
    total = num1 - num2;
    return (total);
}
double division(double num1, double num2)
{
    double total;
    total = num1 / num2;
    return (total);
}
double multiplication(double num1, double num2)
{
    double total;
    total = num1 * num2;
    return (total);
}
double exponential(double num1, double num2)
{
    double total;
    total = pow(num1,num2);
    return (total);
}

It starts by asking for the number of operations to perform. You enter a number, such as 3. Then it asks you to enter in your operation, such as 5+5, which is 10. Then it asks for a second, such as 10^2, which is 100. Then it asks for a third, such as 100-10, which is 90. Then it asks if you'd like to start over.

I'll mention some additional points that haven't been mentioned yet, but this does look like it can still be cleaned out quite a bit.

You could define those functions above main() so that prototypes won't be needed.  Otherwise, put them above main(), as already mentioned in another answer.
These:

#include<iostream>
#include<cmath>
#include<cstdlib>

would look nicer with spaces:

#include <iostream>
#include <cmath>
#include <cstdlib>

Regarding spacing, you have some inconsistent spacing around the cases.  Try not to neglect this as it could make your code more awkward to read for others.
type, x, and y are all declared outside of the loop, but are only used inside of the loop.  In order to help keep track of them, declare them right where they're first used.  Doing this with every variable can help with maintaining cleaner code, especially with larger programs.
This is not doing what you expect:

while (x != 0, x--)

The comma operator returns the result of the last expression, which is the x--. The other expressions are evaluated (from left to right) but the results of the expression are ignored.  Thus, the result of x != 0 is evaluated with no side affects and the result dropped. This makes the expression equivalent to while (x--).
You don't check for division by 0, so your program can break if this operation is done.  You may throw an exception for that and handle it, so that you can get a different input.  Just make sure this division doesn't happen.
You don't need return 0 at the end of main().  The compiler will do this return for you after reaching this point.


Method Selection Engine I have this working code that selects which method to call based on values in
the parameter:

@Override
public Iterator<DBObject> __find(DBObject ref, DBObject fields,
        int numToSkip , int batchSize , int limit, int options){
    Preconditions.checkNotNull(ref, "Reference object can't be null");

    LOG.info("Reference object=" + ref);

    DBObject query = (DBObject) ref.get("$query");
    DBObject orderby = (DBObject) ref.get("$orderby");

    // Remove special fields
    dataCleansing(ref);

    if (query != null){
        LOG.info("Query object=" + query);
        if (orderby != null){
            LOG.info("Sort object=" + orderby);
            Iterator<DBObject> it = _store.getSortedObjectsLike(query, orderby);
            return it;
        } else {
            Iterator<DBObject> it = _store.getObjectsLike(query);
            return it;
        }
    } else {
        if (orderby != null){
            LOG.info("Sort object=" + orderby);
            Iterator<DBObject> it = _store.getSortedObjects(orderby);
            return it;
        } else { // query and ordeby are both null
            Iterator<DBObject> it = _store.getObjects();
            return it;
        }
    }
}

I really think there's some way to better address this selection of functions than just plain if-else statement.

As conditions might grow and more objects passed for condition checking. Is there an "elegant" way to deal with this kind of "choose which method to execute based on parameters values and existence" ?

One possibility is to create a method that accepts all possible parameters, and have that method gracefully handle null values, or use the Null Object pattern for those parameters. getSortedObjectsLike(query, orderby); seems like a good candidate, if you could pass nulls for query and orderby or new EmptyQuery() and new NoOrderBy().

pro : no need for method selection, simply evaluate whether a parameter is available or not, and pass null or Null Object if unavailable.
pro : simplified API
con : adding parameters in the future will involve increasing the complexity of the one method that handles all parameters (can probably be mitigated with some refactoring)
con : implementation of the method that handles all parameters may degrade into the kind of method selection we try to avoid here.

Alternatively you could use introspection, and select a method based on the available parameters.

pro : adding future parameters (if done judiciously) will not affect the method selection implementation
con : every added parameter will double the number of methods needed on the API (for every existing one, we'll need one that also has the new parameter)
con : harder to debug
con : hides dependencies
con : query and orderBy are currently the same type, introspection can't tell them apart


Android app for sending username and password requests I'm developing an Android app that performs several requests to a server using the AndroidAsynchronousHttpClient. One of these requests (as an example) is responsible to send the username and password. I created a class Sign that has a static method (see below) to which I provide a callback to handle the server's response. Is this considered a bad practice?

I'm asking this because I'm having a hard time testing with Mockito.

public class Sign {

public static void signin(String username, String password,
        final SigninResponseHandler responseHandler) {

    RequestParams params = new RequestParams();
    params.put("username", username);
    params.put("password", password);

    MyAPIClient.getInstance().post(path, params,
            new JsonHttpResponseHandler()
            {
                @Override
                public void onSuccess(JSONObject responseObject)
                {
                    String responseAPIStatus = responseObject.getString("status");
                    if (status.isEquals("success")
                    {
                        responseHandler.callback(true, null);
                    }

                }

                @Override
                public void onFailure(Throwable e, JSONObject errorResponse)
                {
                    responseHandler.callback(false, new CustomError(e));
                }
            });
}

Overdoing static is a code smell.  Basically you have to balance things.

Too much static code typically means not enough code in the objects, or not much object oriented structure.  Since objects provide boundaries between the in-object code and the rest of the system, they allow "leverage points" for future change (and for testing).  You can, for example, subclass an object to provide a variety of implementations of a particular class method.

With static methods, no such polymorphisim is possible.  This can lead to switch statements embedded within the static method (to function on key fields, acting somewhat like the state pattern).  The primary reason this tends to become a smell is that switch statements put the burden of code maintenance outside of the object.  In extreme cases, it begins to feel like the object is not encapsulating it's behavior, but rather the switch statement dictates external behavior depending on the data fields within the object.  Such code becomes difficult to maintain over time.

I would avoid static methods as much as reasonably possible; however, in some cases it is a better choice to use them.  For example, one would not typically benefit from subclassing the java core types.  So if I needed to write a method that operated on String objects, I would probably prefer to put that as a static method on a StringUtils class.  On the other hand, if it were a class that I had written, I would probably go out of my way to stay more object-oriented.

In your example...

MyAPIClient.getInstance().post(path, params,
            new JsonHttpResponseHandler()
            {
                @Override
                public void onSuccess(JSONObject responseObject)
                {
                    String responseAPIStatus = responseObject.getString("status");
                    if (status.isEquals("success")
                    {
                        responseHandler.callback(true, null);
                    }

                }

                @Override
                public void onFailure(Throwable e, JSONObject errorResponse)
                {
                    responseHandler.callback(false, new CustomError(e));
                }
            });

is going to be very hard to test.  If only you had made an interface on the MyAPIClient, you could do something like.

public SignIn(String username, String password,
    final SigninResponseHandler responseHandler,
    final MyAPIClient) {
   ...
}

which would make SignIn easily be unit testable, like so

SignIn signIn = new SignIn("bob", "supersecret", new JsonHttpResponseHandler() {...},
    new MockAPI() {... });
signIn.perform();

while the mainline code would look like

SignIn signIn = new SignIn("bob", "supersecret", new JsonHttpResponseHandler() {...},
    MyAPIClient.getInstance());
signIn.perform();


Converting int value to String without using toString and parseInt method I am studying about the converting int to string and string to int without using toString and parseInt method. I already made methods. However, I don't believe that my way of doing was the best. I would like to improve these function.

For example, the first converts from String to int:

public static int StringToint(String number)
        int eachnumber = 0;
    int intConvert = 48;
    int reVal = 0;
    int index;
    int maxlen =number.length() - 1;
        for(index = 0 ; index <= maxlen ; index++)
    {
        eachnumber = number.charAt(index);

        eachnumber = eachnumber  - intConvert;

        reVal = reVal + (eachnumber * (int) Math.pow(10, maxlen - index));
    }
    return reVal;
}

Second, for converting int to String:

public static String IntToString(int number)
{
    int StringConvet = 48;

    int eachDigit = number;
    int afterDivide = number;
    String reVal = "";

    while(afterDivide >0)
    {
        eachDigit = afterDivide % 10;
        afterDivide = afterDivide / 10;
        if(eachDigit == 0)
        {
            reVal += "0";
        }
        else if(eachDigit == 1)
        {
            reVal += "1";
        }
        else if(eachDigit == 2)
        {
            reVal += "2";
        }
        else if(eachDigit == 3)
        {
            reVal += "3";
        }
        else if(eachDigit == 4)
        {
            reVal += "4";
        }
        else if(eachDigit == 5)
        {
            reVal += "5";
        }
        else if(eachDigit == 6)
        {
            reVal += "6";
        }
        else if(eachDigit == 7)
        {
            reVal += "7";
        }
        else if(eachDigit == 8)
        {
            reVal += "8";
        }
        else if(eachDigit == 9)
        {
            reVal += "9";
        }
    }
    String reVal2 = "";
    for(int index =  reVal.length() -1 ; index >= 0 ; index--)
    {
        reVal2 += reVal.charAt(index);
    }
    return reVal2;
}

I am sure that the StringToint method is \$O(n)\$ and intToString method will be \$O(n^2)\$. However, I am really sure that there are ways to improve these two methods. Is there any way to improve these two functions?

To improve your intToString() method you should consider using a StringBuilder, and specifically the method StringBuilder.append(int).

Iterate digits in your int, and for each digit you can append(eachDigit) to the StringBuilder element. This will also reduce the complexity of intToString() to \$O(n)\$ since you do not need to create a new String instance each iteration. To get a String object from the StringBuilder, use StringBuilder.toString(). Or, if you are not allowed, you can use StringBuilder.subString(0).

You should also use a StringBuilder.append() (using the same idea) to reverse the resulting string (your second loop in your code).

Since it is not homework (as per comments), I have no problems providing a code snap. It should look something like this:

public static String intToString(int n) {
    if (n == 0) return "0";
    StringBuilder sb = new StringBuilder();
    while (n > 0) {
        int curr = n % 10;
        n = n/10;
        sb.append(curr);
    }
    String s = sb.substring(0);
    sb = new StringBuilder();
    for (int i = s.length() -1; i >= 0; i--) {
        sb.append(s.charAt(i));
    }
    return sb.substring(0);
}

Notes:

You can also use StringBuilder.reverse() instead of the second loop.
In here, \$O(n)\$ means linear in the the number of digits in the input number (n is the number of digits in the input number - not the number itself!) If you are looking for the complexity in terms of the initial number (it is \$O(\log(n))\$) since you divide your element by 10 each iterations, you have a total of \$\log_{10}(\text{number})\$ iterations for each loop, which results in \$O(\log(\text{number}))\$.


Java constructor should use setter? I have a simple Java model Class called Person which have a constructor that receives a JSONObject. In that constructor I get the values from the JSONObject and put in my instance variables. This is just a simple question but which way is best?

public Person(JSONObject personJsonObject) {
    // Without setter
    this.name = personJsonObject.getString("name");
    // With setter passing String
    setName(personJsonObject.getString("name"));
    // With setter passing JSONObject
    setName(personJsonObject);
}

And when my setter isn't just a simple attribution but a evaluation, which one should I use?

public Person(JSONObject personJsonObject) {
    // Without setter
    this.fullName = personJsonObject.getString("name") + personJsonObject.getString("lastName");
    // With setter passing String
    setFullName(personJsonObject.getString("name"), personJsonObject.getString("lastName"));
    // With setter passing JSONObject
    setFullName(personJsonObject);
    // Or should I put this logic in the getFullName method? The caveat is if there is a huge calculation in the getter method. It wouldn't be efficient.
}

Thanks for your answer.

If you have setter methods (setName, setFullName) in your class, then your constructor should call the setter methods.

setName(personJsonObject.getString("name"));

If you don't have setter methods in your class (your class is immutable), then your constructor should perform the evaluation.

this.name = personJsonObject.getString("name");

You should only pass to a method what it needs.  In this case, your setName method takes a String as a parameter, which is what the method needs.  If your method needs 3 or more values from the personJsonObject (arbitrary rule of mine), then you would pass the personJsonObject to the method.


Encrypting file from source AEAD I am trying to encrypt/decrypt file using source code directly. The issue is how to properly do it after reading the file.

int crypto_aead_encrypt(
    unsigned char *c, unsigned long long *clen,
    const unsigned char *m, unsigned long long mlen,
    const unsigned char *ad, unsigned long long adlen,
    const unsigned char *nsec,
    const unsigned char *npub,
    const unsigned char *k
)

Working code: (plaintext is char *)

for (start = 0; start < fileLength; start += 32)
    {
        int end = (start + 32);
        end = end > fileLength ? fileLength : end;

        strncpy(msg, plaintext + start, end - start);

        // encrypt
        crypto_aead_encrypt(ct, &clen, msg, mlen, ad, adlen, NULL, nonce, key);

        // decryption

        crypto_aead_decrypt(msg2, &mlen2, NULL, ct, clen, ad, adlen, nonce, key);
}

This is the function, and the max limit of the message m is 32 bits, which would make encrypting/decrypt very long for large files (ex. 1GB). Is there a way to perform the encrypting as fast as if it were from the library. Neglecting the code above, ex. how does OpenSSL do it when -in is provided?

The way I have it is to read the file, save it to an char * and then take 32 bits and enc each one. So I am wondering if there is a proper more correct way to do it.

Edit: reference implementation like ascon or photon-beetle

I think you're confused here. The GCM algorithm has a 96 bit nonce, and fills the rest of the 128 bit block size with a 32 bit counter. So you have 2^32 full blocks at your disposal, or about 64 GiB. That's - uh - slightly more than 32 bits (well, OK, the counter starts at 2, so you'd have 48 bytes less than that, better keep it at 32 GiB or 48 GiB).