Session Handling Setup Just wanted to run through my PHP session handling and get some feedback and tips with regard to what is good (if anything!), what could be better and what is either plain wrong, or using out of date methods.

Just as a bit of background, I have only ever dabbled with PHP (and many years ago that was) and want to try and embrace the OOP side of it.

Every page that is required to be part of a session has the following at the top:

<?php
include_once 'Session.php';

$session = new Session();
$session->authenticate();
?>

Session.php is as follows:

class Session {
    const password = "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8";
    private $sessionInfo;

    public function __construct() {
        session_start();
        if (!isset($_SESSION['sessionInfo'])) {
            $sessionInfo = new SessionInfo();
            $_SESSION['sessionInfo'] = $sessionInfo;
        } else {
            $sessionInfo = $_SESSION['sessionInfo'];
        }
    }

    public function authenticate() {
        if (isset($_SESSION['loggedin'])) {
            if ($_SESSION['loggedin'] === true) {
                return;
            }
        }
        header('Location: login.php');
    }

    public function logIn($password) {
        if (sha1($password) === self::password) {
            $_SESSION['loggedin'] = true;
            header('Location: index.php');
        }
    }

    public function logOut() {
        unset($_SESSION['loggedin']);
        session_destroy();
        header('Location: login.php');
    }

    public function getSessionInfo() {
        return $this->sessionInfo;
    }
}

?>

SessionInfo is currently just a blank class, it's intended purpose is to act as an information store for the current session.

login.php:

<?php
include_once 'Session.php';
    if(isset($_POST['submit'])) {
        if(isset($_POST['password'])) {
            $session = new Session();
            $session->logIn($_POST['password']);
        }

    }
?>
<html>
    <body>
        <form method="post" action="<?php echo $_SERVER['PHP_SELF'];?>">
            <input type="password" name="password" />
            <input type="submit" name="submit" />
        </form>
    </body>
</html>

Do I need to call die() (or exit() seeing as they are identical) after a failed authenticate to terminate any page which may attempt to load? Or do I need to return a boolean and then terminate from the calling page?

Please disregard the use of SHA1 and the hashed password in the file, I will be extending this to use a MySQL background, and hope to incorporate the use of a salted hash. I will be using prepared statements, will this be sufficient to protect from SQL injection and allow me to not bother filtering $_POST[...]?

Also, I realise that the location headers may better be stored as a class constant in case I wish to change the locations.

Let's see the code:

Your Session class does too many things:

Session management
Login
Data validation
Logout
Authentication
Inclusion management

Your class has no clean way to destroy a session.
You don't verify if a session is active.
Your constructor seems a little off. Sessions are a singleton. Use a single static final class.
You don't need to repeat public on every method;
You are storing a class on a $_SESSION. I would avoid that like plague. You should start and end the session. And you should have a method to save the session (that runs session_write_close() on shutdown or at any given time).
There's no way to check if a session was opened before or not, or if it was saved already or not.

Fixing these points and splitting your code into classes that do each thing in separate will make your code better in every conceivable way.
A while ago I've used JSON to store the session data. It generates files a bit longer, but it is faster to save. Consider making your own session management system, based out on PHP's way.
Since HDD is cheaper than CPU cycles, I would consider using such way to load/save your sessions.
Completely outside the scope of this review, you could use a RAMdisk with the /tmp partition to squeeze every tiny piece of performance you get. Use only 10% of your RAM for such purpose.

Before doing anything to store your password, I recommend a good read:
How to securely hash passwords?

One of the points is that you shouldn't use SHA1, but instead use bcrypt or password_verify (PHP5.6 and up) in the future.

Also, you should be extremely careful with SQL injection and other possible attacks, especially evolving different encodings.
I sound paranoid, and I know that. But I just want a safe code.


Mystery sum with placeholder digits I came across this coding problem.

  Back in primary school, maybe you were sometimes asked to solve a
  fill-in-the-blank sum - or "mystery sum" - in which certain digits are
  removed and you had to figure out what they originally were.

  Now, it's time for serious business: sums with no digits, where every
  digit is replaced by a letter. Within each seperate sum, a letter
  replaces one digit and it is always the same letter for that one
  digit.

  For example, in the sum below: A+BB=ACC The aim is to figure out which
  digits are hidden behind the letters A, B and C so that the sum turns
  out correct. Here, the solution is that A replaces 1, B replaces 9 and
  C replaces 0; resulting in: 1+99=100

  Finally, there is the constraint that the length of the given word is
  equal to the length of the hidden number. Thus, in the sum
  CHEVAL+VACHE=OISEAU, CHEVAL is indeed a 6 digit number: the C (more
  precisely the first letter of any word) cannot correspond to a zero.

  For each given sum, there is only one possible solution to the
  problem. You are asked to write the result value of the sum to the
  standard output.

  INPUT: One line : a sum in the form WORD1+WORD2=RESULT, in which each
  word is a sequence of capital letters.

  OUTPUT: The result of the solved sum.

  CONSTRAINTS: Each sum has 2 to 20 words, each made up of 1 to 20
  capital letters

I solved it using brute force checking against each permutation of assigned integers to the unique characters. Can anybody suggest a better strategy or algorithm?

public static void mysterySum(String equation) {
String[] breakUp = equation.split("=");
String[] lhs = breakUp[0].split("\\+");
String rhs = breakUp[1];

HashSet<Character> charSet = new HashSet<Character>();
for (char charVal : equation.toCharArray()) {
    if (!(charVal == '+' || charVal == '=')) {
        charSet.add(new Character(charVal));
    }
}

char[] refCharVals = new char[10];

int i = 0;
for (Character charVal : charSet) {
    refCharVals[i++] = charVal;
}
while (i < 10)
    refCharVals[i++] = '=';

String refString = new String(refCharVals);
StringBuilder currRef = new StringBuilder();
checkAllPermutations(refString, currRef, lhs, rhs);

}

private static void checkAllPermutations(String string, StringBuilder outstr,   String[] lhs, String rhs) {

if (resultMatched)
    return;

HashSet<Character> current = new HashSet<>();
if (string.length() == 0) {
    HashMap<Character, Integer> refChars = getRefMap(outstr);

    int sumLhs = 0;
    for (String strNumber : lhs) {
        int number = getnumber(strNumber, refChars);
        sumLhs += number;
    }

    int rhsNumber = getnumber(rhs, refChars);
    if(rhsNumber == sumLhs) {
        resultMatched = true;
    System.out.println(rhsNumber);}
    return;
}

char[] chars = string.toCharArray();

for (int i = 0; i < chars.length; i++) {
    if (current.add(chars[i])) {
        outstr.append(chars[i]);
        checkAllPermutations(string.substring(0, i) + string.substring(i + 1, string.length()), outstr, lhs, rhs);
        outstr.setLength(outstr.length() - 1);
        if(resultMatched) break;
    }

}

}

private static int getnumber(String strNumber,  HashMap<Character, Integer> refChars) {
int number = 0;
for (int i = 0; i < strNumber.length(); i++) {
    number = number * 10 + refChars.get(strNumber.charAt(i));
}
return number;

}

private static HashMap<Character, Integer> getRefMap(StringBuilder outstr) {
HashMap<Character, Integer> refChars = new HashMap<>();

for (int i = 0; i < outstr.length(); i++) {
    if (outstr.charAt(i) != '=')
        refChars.put(outstr.charAt(i), i);
}

return refChars;
}

As for a review of the code I would suggest to avoid acronyms (also recommend, as @200_success did, to better indent the code, in case this is how your code is indented).

As for the solution, I would strongly avoid brute force ones. This is a cryptarithmetic problem and, if you work only on base 10, you have 10! possible assignments, but if you generalize (for any base) then the problem is NP-complete. I would suggest to look for solutions for cryptarithmetic problems (usually they are described on texts/sources on AI).


C program to read a file containing numbers, and then print ones divisible by 3 to another file The program reads a file containing a list of numbers, and then writes the ones divisible by 3 to another file, and prints the remaining ones (non-divisible) to a third file.

Declaring, opening, and closing the files is alone over 50% of the code, is there anyway to make it more compact?

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

int main(void){
    FILE *in_file; /*input file*/
    FILE *out_file_divis; /*FIle that contains numbers divisible by 3*/
    FILE *out_file_remain; /*File that contains the rest of the numbers*/

    if((in_file = fopen("total", "r")) == NULL){
        perror("total");
        exit(EXIT_FAILURE);
    }

    if((out_file_divis = fopen("divis", "w")) == NULL){
        perror("divis");
        exit(EXIT_FAILURE);
    }

    if((out_file_remain = fopen("remain", "w")) == NULL){
        perror("remain");
        exit(EXIT_FAILURE);
    }

    int number;

    while(fscanf(in_file, "%i", &number) == 1){
        if(number%3 == 0){
            fprintf(out_file_divis, "%i\n", number);
        }

        else{
            fprintf(out_file_remain, "%i\n", number);
        }
    }

    fclose(in_file);
    fclose(out_file_divis);
    fclose(out_file_remain);

    return 0;
}

The easiest way to make this more compact is to write a function to handle opening a file and just pass in the name and mode you want. Something like this:

FILE* open_file_or_exit(const char* filename, const char* mode)
{
    FILE* fileHandle = NULL;
    if((fileHandle = fopen(filename, mode)) == NULL){
        perror(filename);
        exit(EXIT_FAILURE);
    }
    return fileHandle;
}

Your main() then becomes:

int main(void){
    FILE *in_file = open_file_or_exit("total", "r");
    FILE *out_file_divis = open_file_or_exit("divis", "w');
    FILE *out_file_remain = open_file_or_exit("remain", "w");
    // ... rest of main ...
}

Any time you find yourself repeating the same few lines of code, that's a good sign that it should go into a function.


Java AES CBC encryption/decryption I've looked at multiple examples of Java AES CBC mode encryption but I couldn't find a proper solution that's safe to use. I ended up writing my own utility class. My questions is: Is this code safe to use? I've read some stuff about padding oracle attacks but I don't really understand how they work.

I've tested the code and it seems to work properly:

CryptoUtils.removeCryptoRestriction();

String string = "Hello world! This is a test string. Have a nice day!";
byte[] iv = CryptoUtils.generateIv();
byte[] key = CryptoUtils.generateKey();

byte[] encrypted = CryptoUtils.doCrypto(Cipher.ENCRYPT_MODE, key, iv, string.getBytes("UTF-8"));
System.out.println(new String(encrypted));

byte[] decrypted = CryptoUtils.doCrypto(Cipher.DECRYPT_MODE, key, iv, encrypted);
System.out.println(new String(decrypted));

Output:

��zI Fğ��O>��_�@3�ܷ+|ZI�m���M�4�;���ygr8G�ܪ8�6u���M
Hello world! This is a test string. Have a nice day!

Here's the util code:

public static byte[] doCrypto(int mode, byte[] keyBytes, byte[] ivBytes, byte[] bytes) throws GeneralSecurityException {
    Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5PADDING");
    SecretKeySpec secretKeySpec = new SecretKeySpec(keyBytes, 0, keyBytes.length, "AES");
    IvParameterSpec ivParameterSpec = new IvParameterSpec(ivBytes);
    cipher.init(mode, secretKeySpec, ivParameterSpec);
    return cipher.doFinal(bytes);
}

public static void doCrypto(int mode, byte[] keyBytes, byte[] ivBytes, File in, File out) throws GeneralSecurityException, IOException {
    Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5PADDING");
    SecretKeySpec secretKeySpec = new SecretKeySpec(keyBytes, 0, keyBytes.length, "AES");
    IvParameterSpec ivParameterSpec = new IvParameterSpec(ivBytes);
    cipher.init(mode, secretKeySpec, ivParameterSpec);

    try (FileInputStream fileInputStream = new FileInputStream(in); FileOutputStream fileOutputStream = new FileOutputStream(out)) {
        byte[] buffer = new byte[1024];

        for (int i = 0; i != -1; i = fileInputStream.read(buffer)) {
            byte[] updateBytes = cipher.update(buffer, 0, i);
            if (updateBytes != null) fileOutputStream.write(updateBytes);
        }
        byte[] finalBytes = cipher.doFinal();
        if (finalBytes != null) fileOutputStream.write(finalBytes);
    }
}

public static byte[] generateIv() {
    SecureRandom secureRandom = new SecureRandom();
    byte[] iv = new byte[16];
    secureRandom.nextBytes(iv);
    return iv;
}

public static byte[] generateKey() throws NoSuchAlgorithmException {
    KeyGenerator keyGenerator = KeyGenerator.getInstance("AES");
    keyGenerator.init(256);
    SecretKey secretKey = keyGenerator.generateKey();
    return secretKey.getEncoded();
}

public static void removeCryptoRestriction() {
    Security.setProperty("crypto.policy", "unlimited");
}

The file encryption works too.

Thanks in advance!

You should move to an authenticated mode of encryption like CCM, EAX, OCB or GCM. The fact that you are generating a random sequence of bytes from the OS's PRNG is sound. It looks like the argument provided to the crypto object is already padding the resulting cipher text of the file stream.

The top bit about removing the restrictions is suspect.

Padding attacks are a side channel attack in crypto that allows an attacker to perform cryptanalysis on a block of cipher text because the plain text input would determine the resulting cipher texts length.  By padding the resulting cipher text it makes that side channel more difficult.


Minimum steps to 1 to find minimum operations to transform n to 1 I was doing this JavaScript problem called "Minimum steps to 1." You can find the problem here.

  Given a positive integer - num, Following is a list of possible operations which can be performed on it:

  num / 2, If number is divisible by 2
  num / 3, If number is divisible by 3
  num - 1

  With these 3 available operations, find out the minimum number of steps required to reduce the number to 1.

  For example:

  For num = 1, no. of steps needed - 0
  For num = 2, no. of steps needed - 1 (num/2)
  For num = 6, no. of steps needed - 2 (num/3 followed by num/2)
  For num = 9, no. of steps needed - 2 (num/3 followed by num/3)

function minStepstoOne(n) {
  let steps = [];
  steps[0] = 0;
  steps[1] = 0;

  for(let i = 2; i <= n; i ++) {
    let minChoice = steps[i - 1];

    if(i % 2 == 0) {
      let divideByTwo = steps[i/2]
      minChoice = Math.min(divideByTwo, minChoice);

    }

    if(i % 3 == 0) {
      let divideByThree = steps[i/3]
      minChoice = Math.min(divideByThree, minChoice);

    }

  steps[i] = minChoice + 1;

  }
  return steps[n];
}

//console.log(minStepstoOne(9));

Your function is right.

However, for dynamic programming I always propagate state n to all states n+1 (Forward). Your code calculates state n+1 from state n (Backward). In this case both methods are okay and it is mostly a personal preference. But in other cases the backward method may run into problems that state n is not yet calculated.

function minStepstoOne2(n) {
  var steps = Array(n+1).fill(n);
  steps[1] = 0;

  for(let i = 1; i < n; i ++) {
    steps[i + 1] = Math.min(steps[i + 1], steps[i] + 1);
    if (i * 2 <= n)
      steps[i * 2] = Math.min(steps[i * 2], steps[i] + 1);
    if (i * 3 <= n)
      steps[i * 3] = Math.min(steps[i * 3], steps[i] + 1);
  }
  return steps[n];
}


Get 5 to 10 characters and flip their case This small programme is an exercise that my teacher gave us.
n is an alphabetic input that its length must be between 5 and 10. You have to insert the characters of the input one by one to a list called T. Then, add the reverse case of letters to another list called V. So if there is a capital letter in T it would be lower in list V and you have to only print V at the end.
This is my code, is there any better solution or cleaner one perhaps?
T = []
V = []
n = input("Enter a character")
while len(n) not in range(5, 11) or n.isalpha() == False:
    n = input("Enter a characters between 5 and 11, and only characters ")

print(n)
for i in range(len(n)):
    T.append(n[i])

for i in range(len(T)):
    if T[i].isupper():
        V.append(T[i].lower())
    else:
        V.append(T[i].upper())

print(V)

A flexible approach to getting user input is to use a while-true loop, breaking
when conditions are met. The structure is easy to remember and allows you to
handle user mistakes without awkward setup or code repetition:
while True:
    x = input('...')
    if x is OK:
        break

I realize the variable names are coming from your teacher's instructions, but
both of you should strive for better variable names. Better does not always
mean longer (brevity and clarity are both worthy goals and they are at odds
sometimes); but it does mean using variable names that are meaningful
in context. For example, letters is a better name for a string of letters
than n, which is a purely abstract name (even worse, it adds confusion
because n is often/conventionally used for numeric values).
There is no need to build T character by character. Python strings are
iterable and therefore directly convertible to lists.
Python strings, lists, and tuples are directly iterable: just iterate over the
values and don't bother with the soul-crushing tedium imposed by many less-cool
programming languages where you must iterate over indexes. And for cases when
you need both values and indexes, use enumerate().
Use simple comments as sign-posts and organizational devices to make
your code more readable. This habit will serve you well as you try
to write bigger, more complex programs.
# Get user input.
while True:
    prompt = 'Enter 5 to 10 letters, without spaces or punctuation: '
    letters = input(prompt)
    if len(letters) in range(5, 11) and letters.isalpha():
        break

# List of the original letters.
T = list(letters)

# List of letters with case flipped.
V = []
for c in letters:
    c2 = c.lower() if c.isupper() else c.upper()
    V.append(c2)

# Same thing, in one shot.
V = [
    c.lower() if c.isupper() else c.upper()
    for c in letters
]

# Check.
print(T)
print(V)


Inserting multiple rows in MySQL I am building a store. When a user decides to make a purchase I need to store it in the database, but since the purchase might be of more than 1 item I'm wondering which way is the best to insert multiple rows, here are my 3 options.

I use a database object that combines PDO with PDOStatement, which is why I don't define $stmt anywhere.

The input is:

$input = array(
    'user_id' => 15,
    'food_id' => 2,
    'quantity' => 15
);

The table I'm inserting into:

 CREATE TABLE `users_foods` (
 `user_id` int(10) unsigned NOT NULL,
 `food_id` int(10) unsigned NOT NULL,
 KEY `user_id` (`user_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8

Option one

$insert_data = array();

while($input['quantity']){
    $insert_data[] = array($input['user_id'], $input['food_id']);
    $input['quantity']--;
}

$db->insert('users_foods', array('user_id', 'food_id'), $insert_data);

Respectively used functions are

public function insert($table, array $columns, array $values){
    $valArr = array();
    $data = array();

    $id = 0;
    if(!empty($values)){
        if(is_array($values[0])){
            foreach($values as $k => $v){
                foreach($v as $key => $val){
                    $data[':i'.$id] = $val;
                    $v[$key] = ':i'.$id;
                    $id++;
                }
                $valArr[] = '('.join(',', $v).')';
            }
        }else{
            foreach($values as $key => $val){
                $data[':i'.$id] = $val;
                $v[$key] = ':i'.$id;
                $id++;
            }
            $valArr[] = '('.join(',', $v).')';
        }
    }

    return $this->prepare('INSERT INTO '.$table.' ('.join(',', $columns).') VALUES '.join(',', $valArr))->execute($data);
}

private function parseWhereClause(array $where, $iteration = 0, $id = 0, $sd = null){
    $sql = array();
    $data = array();

    if(count($where) == 3 && !is_array(reset($where))){
        if(is_array($where[2])){
            foreach($where[2] as $expr){
                $sql[] = ':w'.$iteration.$id;
                $data[':w'.$iteration.$id++] = $expr;
            }
            switch(strtolower($where[1])){
                case 'in': $sql = $where[0] . ' ' . $where[1] . ' ('.join(',', $sql).')'; break;
                case 'between': $sql = $where[0] . ' ' . $where[1] . ' '.join(' AND ', $sql); break;
            }
        }else{
            $sql = $where[0] . ' ' . $where[1] . ' :w'.$iteration.$id;
            $data[':w'.$iteration.$id] = $where[2];
        }
    }else{
        if(!empty($where)){
            $id = 0;

            foreach($where as $c => $expressions){
                $recur = $this->parseWhereClause($expressions, ++$iteration, $id++, $c);
                $sql[] = $recur['sql'];
                $data += $recur['data'];
            }

            $sql = '('.join(' '.$sd.' ', $sql).')';
        }else{
            $sql = '1';
        }
    }

    return array(
        'sql' => $sql,
        'data' => $data
    );
}

This method will only run one large query.

Option two

$db->prepare('INSERT INTO users_foods (user_id, food_id) VALUES (?, ?)');

while($input['quantity']){
    $db->execute(array($input['user_id'], $input['food_id']));
    $input['quantity']--;
}

not quite sure how this prepare-execute thing works under the hood though, but I believe that this will run 15 queries (because quantity is 15 in the example).

Option three

MySQL

CREATE PROCEDURE `buy_food`(IN `user_id` INT UNSIGNED, IN `food_id` INT UNSIGNED, IN `quantity` INT UNSIGNED)
BEGIN

 WHILE quantity > 0 DO
   INSERT INTO users_foods (`user_id`, `food_id`) VALUES (user_id, food_id);
   SET quantity = quantity - 1;
 END WHILE;

END

PHP

$db->prepare('CALL buy_food(?, ?, ?)')->execute(array(

    $input['user_id'],
    $input['food_id'],
    $input['quantity']

));

Which one do you think is the best way to go and why?

It is a maxim in computer science that there are really "only three numbers": zero, one, and many.  You have a case of many, and you should treat it as such.  Otherwise, someday a customer might want to buy a hundred of some food item, and performance will be poor.

Therefore, I recommend…

Option four

Instead of having the table represent current supply levels, keep a transaction log, where the quantity column stores purchases as positive values and consumption as negative values.

INSERT INTO user_food_purchase (user_id, food_id, quantity) VALUES (?, ?, ?);

Then, adjust the rest of your application to accommodate the schema change.  For compatibility, you could define a view:

CREATE VIEW user_food AS
SELECT user_id, food_id, SUM(quantity) AS quantity
    FROM user_food_purchase
    GROUP BY user_id, food_id;


Generate the Factorial of Given Number This is a console application that generates the factorial of a given number. Is there any way I can improve my code ?

        int number = Convert.ToInt32(Console.ReadLine());
        int digitFactorial = number;
        for (int counter = number - 1; counter > 0; counter--)
        {
            digitFactorial *= counter;
        }
        Console.WriteLine(digitFactorial);
        Console.ReadLine();

As Slicer wrote in his comment you should create a method/function for the algorithm.
And you could make it as an extension method:

public static class NumberExtensions
{
  public static ulong Factorial(this ulong number)
  {
    if (number < 2) return 1;

    checked
    {
      ulong factorial = number;
      for (ulong i = 2; i < number; i++)
      {
        factorial *= i;
      }
      return factorial;
    }
  }
}

Here I've widened the domain to ulong and I iterate upwards from 2 as it is more intuitive and easier to read. Further I've encapsulated the method in a checked statement in order to throw on overflow, because factorial grows quickly out of even the scope of ulong.

Usage

  while (true)
  {
    Console.Write($"Enter a number: ");
    if (ulong.TryParse(Console.ReadLine(), out ulong number))
    {
      try
      {
        Console.WriteLine($"Factorial of {number} = {number.Factorial()}");
      }
      catch (OverflowException ex)
      {
        Console.WriteLine($"Entered number is too big: {number}");
      }
      catch (Exception ex)
      {
        Console.WriteLine($"An Error occured: {ex.Message}");
      }
    }
    else
    {
      break;
    }
  }


Sending & Receive Image Over TCP/IP I've created a program that can send & receive a screenshot. However, I feel like my method is a little bit...inefficient. I have to write to the stream twice, because from what I've observed, in order for the image to form properly, I must know the exact file size aka. the length. So, what I do is write the length to the the stream, than write the image to the stream. Is there a way to combine this into one step, or optimize it more?

Here's my code for the client

 else if (plainText.Contains("screenshot")) //check if received command is asking for screenshot
        {
            MemoryStream ms = new MemoryStream();
            Bitmap bitmap = new Bitmap(Screen.PrimaryScreen.Bounds.Width, Screen.PrimaryScreen.Bounds.Height);
            Graphics graphics = Graphics.FromImage(bitmap);
            graphics.CopyFromScreen(0, 0, 0, 0, bitmap.Size);
            bitmap.Save(ms, System.Drawing.Imaging.ImageFormat.Jpeg);//save bitmap to MemoryStream so I can read length
            writeToStream(ms.ToArray().Length.ToString()); //Write the length to the NetWorkStream
            writeToStream(ms.ToArray()); //Write the actual image to the NetworkStream
            ms.Close(); //close the MemoryStream when done
        }

And for my server:

private void screenShotBTN_Click(object sender, EventArgs e)
    {
        screenShotBTN.Enabled = false;

        NetworkStream stream = getSelectedClient().GetStream();

        writeBuffer = Encoding.ASCII.GetBytes("screenshot");

        stream.Write(writeBuffer, 0, writeBuffer.Length); // send screenshot request

        int data = 0;

        byte[] readBuffer = new byte[getSelectedClient().ReceiveBufferSize];

        data = stream.Read(readBuffer, 0, readBuffer.Length);

        string pictureSize = Encoding.ASCII.GetString(readBuffer, 0, data);

        Console.WriteLine(pictureSize); //for debugging purposes so I can see the length

        string x = new Random().Next().ToString();

        FileStream f = new FileStream(x + ".bmp", FileMode.Create, FileAccess.Write);

        while (new FileInfo(x + ".bmp").Length != Convert.ToInt32(pictureSize))
        {
            data = stream.Read(readBuffer, 0, readBuffer.Length);
            f.Write(readBuffer, 0, data);
        }

        f.Close();

        Process.Start(x + ".bmp");

        screenShotBTN.Enabled = true;
    }

ms.Close(); //close the MemoryStream when done

It's a good thing that you close the ms (MemoryStream) but there are two more to dispose.

The bitmap and graphics instances also need to be disposed/closed. The same applies to the NetworkStream but I got the impression that you don't know the using statement that allows you to automatically handle resources so:

NetworkStream stream = getSelectedClient().GetStream();
..
stream.Close();

would become:

using(var networkStream = getSelectedClient().GetStream())
{
    ..
} // at this point it will be closed/disposed by the runtime

The benefit of this is that it will be correctly disposed even if an exception occurs. Without it you would need to write the try/finally yourself (what a using actually does for you (or rahter the compiler))

NetworkStream networkStream = null;

try
{
    networkStream = getSelectedClient().GetStream();
    ..
}
finally
{
    networkStream?.Close();
}

You can use the same pattern for bitmaps, graphics and anything else that is IDisposable.


Project Euler #2 (Fibonacci Sequence) Challenge: The Fibonacci sequence is generated by adding the previous two terms.

  By starting with 1 and 2, the first 10 terms will be: 1, 2, 3, 5, 8,
  13, 21, 34, 55, 89.

  Consider a Fibonacci sequence whose values do not exceed four million,
  find the sum of the even-valued terms.

My solution:

import java.util.ArrayList;

public class Euler2 {
    public static void main(String[] args) {
        final long START = System.nanoTime(),
            MAX_NUM = 4000000L;
        Long result = 0L;

        ArrayList<Long> sequence = new ArrayList<Long>() {
            { add(1L); add(2L); }
        };

        int i = 0;
        Long nextTerm = sequence.get(i++) + sequence.get(i);
        while (nextTerm <= MAX_NUM) {
            sequence.add(nextTerm);
            nextTerm = sequence.get(i++) + sequence.get(i);
        }

        /* Leveraging the absence of consecutive even numbers
        and the fact that Odd + Even is Odd, and Odd + Odd is even,
        retrieve every third term after an even term until no longer possible
        */
        int j = 1;
        while (j <= i) {
            result += sequence.get(j);
            j += 3;
        }

        System.out.print("Result: " + result +
            ".\nTime used for calculation in nanoseconds: " +
            (System.nanoTime() - START) + "."
        );
    }
}

Sample output:

  Result: 4613732.
  Time used for calculation in nanoseconds: 1651793

Without the cost of readability is there an adjustment that would improve performance?
I considered doing this with Streams but decided it wasn't necessary. Was I correct or would its use offer notable enhancement?

My Assumptions about your requirements

Here are a few points, assuming that you want performance and maintainability.

Assumption: Since you did not put the generation of the sequence and the sum of the evens in separate methods, I assume that a separate generation of the sequence is explicitly not a requirement and therefore it's okay if all functionality goes into one single method.

Runtime Comparison of solutions

Runtime comparison on my machine:

Your solution: ~950000ns
My simple solution (no loop unrolling, same structure as your code, except for the points mentioned below): ~3300ns
Best solution known to me: ~1700ns

Key points to improve performance of your code

without trading in maintainability.

The ArrayList is slowing the code down somewhat. As long as the code is interpreted, it is quite slow because interpreted method calls are slow. Once it is JIT-compiled the method calls no longer matter. Still, using ArrayList is unnecessary, as we will see below.
Your usage of Long for result involves unnecessary boxing / unboxing in the line result += .... Use long instead, result doesn't need to be Long. This speeds up your program by 1%.
You're concatenating Strings while time is still running. That tampers your  time calculation. Call System.nanoTime() right after the algorithm, not later.
Also, you might want to perform the runs to your algorithm multiple times in order to rule out effects of things like the OS scheduler or the code not being JIT-compiled when you measure performance.

Details about the even check

Given the requirements, you could check the numbers to sum up right away. Why do you check the numbers later?

I believe you fell into a premature optimization pit without considering the odds of your trade. You've traded the cheap operation "n is even", which is if ((n & 1) == 0), for a very expensive one - calling virtual method add() and calling virtual method get() inside the loops.

Your observation about every third number in the Fibonacci sequence being even is correct, the conclusion for the performance optimization is wrong.

!Spoiler Alert!

Fibonacci solutions below.

The typical Fibonacci loop

The typical Fibonacci loop looks like this:

long prevprev, prev = 1, current = 1;
while (current <= MAX_NUM) {
    prevprev = prev;
    prev = current;
    current = prev + prevprev;
}

Because we shall start with 1, 2 instead of 1, 1, which produces the same sequence except that it's missing the first member, we would use current = 2 instead.

So here's how to calculate the result:

long result = 0;
long prevprev, prev = 1, current = 2;

while (current <= MAX_NUM) {
    if ((current & 1) == 0) // if (isEven(current))
       result += current;
    prevprev = prev;
    prev = current;
    current = prev + prevprev;
}

loop unrolling to skip 2/3rd of the checks.

You can still use your smartness about the even/odd/odd pattern of the Fibonacci sequence.

There is a way in which you can ensure that you only sum up the evens without using a condition. We can use loop unrolling for that. Please note that as such, loop unrolling in Java is absolutely pointless. It only helps in this case because it enables us to get rid of 2 even checks for every 3 Fibonacci numbers.
The result is not maintainable, and because of the JIT compiler being smarter than all humans except its creators and those that studied it extremely well (I haven't), we usually would never even think of loop unrolling in Java.

Here's a code fragment which demonstrates the unrolled loop:

public static long sumOfEvenFibonacciNumbersUntilMaxNum() {
    long result = 0;

    long prevprev, prev = 1, current = 2;

    // Use the fact that every third is even: 1, 1, 2, 3, 5, 8.
    // We start with 2, so the first one in three is even.
    // Sum overflow over MAX_NUM inside the loop irrelevant:
    // The values of current which are too big are not used for result.
    while (current <= MAX_NUM) {
        result += current;// even
        prevprev = prev;
        prev = current;
        current = prev + prevprev;
        prevprev = prev;
        prev = current;
        current = prev + prevprev;
        prevprev = prev;
        prev = current;
        current = prev + prevprev;
    }
    return result;
}

This trades 6 additional assignments and 3 additional additions in the last loop for avoiding n checks of (current & 1) == 0 and reduces the number of current <= MAX_NUM checks by 3.

If you think that this unrolled loop looks like it can be optimized further, you're right, we can actually optimize the loop logic, see next step.

Further algorithmic analysis

The Fibonacci calculation is a window of three numbers over the Fibonacci sequence with the current number F(n) and two precursors F(n-1) and F(n-2) for remembering the intermediate results in order to perform the production of F(n) := F(n-1) + F(n-2).

You noticed correctly, that every third member of the Fibonacci sequence is even.

This happens to be the same size as the window that we need to move in order to perform the calculation.
We can utilize this.
As always, good naming helps.
Therefore, let's change the names of the variables as well.

public static long sumOfEvenFibonacciNumbersUntilMaxNum() {
    long sumOfEvens = 0;
    long odd1, odd2 = 1, even = 2;

    while (even <= MAX_NUM) {
        sumOfEvens += even;
        odd1 = even + odd2;
        odd2 = odd1 + even;
        even = odd2 + odd1;
    }
    return sumOfEvens;
}

Given the self-explanatory variable names and the comment, I claim this still is perfectly well maintainable code. The only drawback is that it cannot generate the Fibonacci sequence separately. Hence my initial assumption about the requirements.

P.S.:
Of course we'd make MAX_NUM a parameter, I just didn't discuss this because I think it's completely irrelevant for the problem and obvious to the typical audience of this class of problem.

Comparison of the add solution with the multiply solution.

There also exists a solution which uses the fact that you can generate the sequence of evens without the intermediate odds.

Let's compare them.

// Add alternative
public static long sumOfEvenFibonacciNumbersUntilMaxNum() {
    long sumOfEvens = 0;
    long odd1, odd2 = 1, even = 2;

    while (even <= MAX_NUM) {
        sumOfEvens += even;
        odd1 = even + odd2;
        odd2 = odd1 + even;
        even = odd2 + odd1;
    }
    return sumOfEvens;
}

// Multiply alternative
public static long sumOfEvenFibonacciNumbersUntilMaxNum() {
    long sumOfEvens = 0;
    long prevPrevEven, prevEven = 0, even = 2;

    while (even <= MAX_NUM) {
        sumOfEvens += even;
        prevPrevEven = prevEven;
        prevEven = even;
        // << 2 instead of * 4 matters here to reuse value 2 in the byte code.
        even = (prevEven << 2) + prevPrevEven;
    }
    return sumOfEvens;
}

Now we really have to go down to machine code in order to understand what's going on. This of course has limited value in Java, as there are too many unknowns like if it is JIT compiled, how the JIT works, and how fast the CPU is.

The byte code of the multiply solution is 3 instructions shorter, which happen to be in the loop. If the byte code is interpreted only, the multiply solution wins.

If the code is compiled, we need to think of what machine instructions are going on inside the loop.

Add loop:

; R0: result = 0
; R1: odd1
; R2: odd2 = 1
; R3: even = 2
; R4: MAX_VALUE
.label
    cmp R3, R4
    bgt .done
    add R0 <- R0 + R3
    ; begin difference
    add R1 <- R3 + R2
    add R2 <- R1 + R3
    add R3 <- R2 + R1
    ; end difference
    bra .label
.done

Multiply loop:

; R0: result = 0
; R1: prevPrevEven
; R2: prevEven = 0
; R3: even = 2
; R4: MAX_VALUE
.label
    cmp R3, R4
    bgt .done
    add R0 <- R0 + R3
    ; begin difference
    mov R1 <- R2
    mov R2 <- R3
    lsl R3 <- R3 << 2
    add R3 <- R3 + R1
    ; end difference
    bra .label
.done

Whether the multiply solution is faster or the add solution is faster depends on how this pseudo assembly translates into the real machine code.
The key questions are:

Is add Rn <- Rx + Ry available? If not, i.e. only add Rn <- Rn + Rx is available, each add Rn <- Rx + Ry with n != x and n != y requires an additional mov instruction.
Is lsl Rn <- Rn << 2 available? If not, we need one more register. And if so, are enough registers available?

On CPUs like ARM, on which add Rn <- Rx + Ry is available, the Add solution is fastest.
On CPUs like 80x86 or 680x0 / CPU32, on which add can only modify an existing register but not store the result in a new register, additional move instructions would be inserted for each add instruction which does not use a source register also as destination register. So the Multiply solution is faster on 80x86 and 680x0 / CPU32.

EDIT Note: The original answer contained statements about method calls being expensive, which is wrong, and a description of loop unrolling which was partially wrong and thus misleading. The credit for fixing these bugs in the answer goes to jrolfl, who pointed this out and patiently discussed this with me in chat. Thank you!


Javascript event handling via switch The application I am developing with has many input fields that interact with one another via the onchange event.
When developing started, the goal was to create a framework that makes it easy to define event actions for the input fields and keep the action functions together.

The back then the following was implemented:

<input type='text' class='process' data-fn='function1;function2'>

And in Javascript using jQuery (leaving out security and validation checks here):

$("body").on("change", ".process", function(e){
     process($(this).attr("data-fn"),$(this),e);
});

function process(functionString, caller, event){
    var functions = functionString.split(";");
    for (var index = 0; index < functions.length; ++index) {
        switch (functions[index]){
            case "function1":
                    //do something
               break;
            case "function2":
                    //do something
               break;
        }
    }
}

The amount of needed functions was highly underestimated. The switch now has 110 cases.
The advantage of the algorithm as it is now is that I can make the break optional, creating functions with multiple "entrances".
The disadvantage is no local variable scope inside each case.

Is there a huge problem that I don't see with this approach (like switch performance etc.)?

I am interested if this is bad coding or just an interesting approach to the problem.

Additionally, does a software design pattern exist, that is defined to do such functionality?

I guess a better approach would have been the following, but now that it is that way..

function process(functionString, caller, event){
    var functions = functionString.split(";");
    for (var index = 0; index < functions.length; ++index) {
        if (functionObj.hasOwnProperty(functions[index]){
            functionObj[functions[index]](caller,event);
        }
    }
}

var functionObj = {
    function1: function (caller, event){
        //do something;
    },
    function2: function (caller, event){
        //do something;
    }
}

This sort of reminds me of how React implements Flux, where there's a huge switch list, and it becomes very unwieldy fast. Another developer created the Reflux library using the same concept in your alternative, a huge action map. So you're getting there. Let's just tweak your approach a bit.

First is you can use the noop approach, so you avoid the if and the function is always a function. It's pretty functional programming, but it streamlines your code, avoiding branching, unnecessary assignments and all that stuff.

Next, functionObj could be named better, like registeredFunctions. Also create an interface for you to add easily, as well as check for duplicates to prevent collision, or just register multiple ones under the same name.

Next is to allow your callers to define multiple args by not limiting the args. Here, we pass in arguments using apply. That way, the callers can provide any number of args they want and the receiving functions can still receive them. Of course, we slice off the first argument, which is your string of functions. You can assign any value as context for your registered function, but I prefer functions like these to have a context of null. Context becomes confusing when calling functions dynamically, and without a set convention.

var noop = function(){}
var registeredFunctions = {/* map of functions */}

function process(){
  var args = Array.prototype.slice.call(arguments, 1);
  functionString.split(";").forEach(function(functionName){
    (registeredFunctions[functionName] || noop).apply(null, args);
  });
}

An alternative would be to keep the huge switch, but instead of putting the actual code inside it, you call a function. There, you have a new scope. You just need to pass in everything you need into the function.

var args = Array.prototype.slice.call(arguments, 1);
switch(something){
  case 1:
  case 2:
    doSomething.apply(null, args);
    break;
  case 3:
    doSomethingElse.apply(null, args);
  default:
    doSomethingDefault.apply(null, args);
}


Managing JS objects while preserving references Scenario

I have foo:

{
    name: 'Mr. Foo',
    value: 4
}

I receive bar from the server:

{
    name: 'Mrs. Bar',
    description: 'Wife of Mr. Bar'
}

I wish to update foo to be identical to bar (containing only the same properties and values in bar, and nothing more), but I need every reference to now know about these new values. This means doing foo = bar won't work because anything that had a reference to foo still has reference to foo's original values and will keep just looking at those.

Possible Solution

I have been using angular.extend() in the cases that I know for a fact the properties will always be the same, but if I do that in this scenario, foo will wind up with name, value, and description properties, when I need only name and description.

I came up with the following "replacement" function as a part of a utility service designed to solve this problem, but I'm wondering if there's a more elegant way to do this (something that perhaps looks less prone to obscure bugs than this does).

replaceObj: function (obj, updates) {
    if (obj && !Array.isArray(obj)) {
        // Extend original object
        angular.extend(obj, updates);

        // Get object keys
        var objKeys = Object.keys(obj);
        var updatesKeys = Object.keys(updates);

        // Delete original object properties that are not included in the updates object
        objKeys.forEach(function(objKey) {
            if (updatesKeys.indexOf(objKey) === -1)
                delete obj[objKey];
        });
    } else {
        console.error('utility.replaceObj called without valid parameters.');
    }
}

Instead of checking each property in the result to see whether it was in the source object, simply delete all of the properties in the destination object and then use angular.extend or Object.assign to copy from the source.

function replaceObj(target, source) {
    Object.keys(target).forEach(function(key) {
        delete target[key];
    });
    Object.assign(target, source);
}

Also, if you do want to check whether a property is in an object, using updates.kasOwnProperty(objKey) is more straightforward than making a separate array of keys and using indexOf.


ROT47 function implementation According to Wikipedia, here below is the definition of the algorithm:

  ROT13 ("rotate by 13 places", sometimes hyphenated ROT-13) is a simple letter substitution cipher that replaces a letter with the letter 13 letters after it in the alphabet. ROT13 is a special case of the Caesar cipher, developed in ancient Rome.

  ROT47 is a derivative of ROT13 which, in addition to scrambling the basic letters, also treats numbers and common symbols. Instead of using the sequence A–Z as the alphabet, ROT47 uses a larger set of characters from the common character encoding known as ASCII. Specifically, the 7-bit printable characters, excluding space, from decimal 33 '!' through 126 '~', 94 in total, taken in the order of the numerical values of their ASCII codes, are rotated by 47 positions, without special consideration of case. For example, the character A is mapped to p, while a is mapped to 2.

I already implemented it in C++ but this time, I have implemented it in SQL Server. Here below is the user-defined function I wrote:

CREATE FUNCTION [dbo].[ROT47]
(
  @PLAIN_TEXT nvarchar(MAX)
)
RETURNS nvarchar(MAX)
AS
BEGIN
  DECLARE @ENCRYPTED_TEXT nvarchar(MAX) = N''
  DECLARE @LENGTH_TEXT int = 0
  DECLARE @c nvarchar = N''
  DECLARE @i int = 1

  SET @LENGTH_TEXT = LEN(@PLAIN_TEXT)

  WHILE (@i <= @LENGTH_TEXT)
  BEGIN
    SET @c = SUBSTRING(@PLAIN_TEXT, @i, 1)

    IF (ASCII(@c) BETWEEN ASCII(N'!') AND ASCII(N'~'))
    BEGIN
      SET @c = char(ASCII(N'!') + (ASCII(@c) - ASCII(N'!') + 47) % 94)
      SET @ENCRYPTED_TEXT = @ENCRYPTED_TEXT + @c
    END

    SET @i = @i + 1
  END

  RETURN @ENCRYPTED_TEXT
END

Below is the query I wrote to test my UDF:

DECLARE @PLAIN_TEXT nvarchar(MAX) = N'HelloWorld'
DECLARE @ENCRYPTED_TEXT nvarchar(MAX)
DECLARE @DECRYPTED_TEXT nvarchar(MAX)

SET @ENCRYPTED_TEXT = ( SELECT dbo.ROT47(@PLAIN_TEXT) )
SET @DECRYPTED_TEXT = ( SELECT dbo.ROT47(@ENCRYPTED_TEXT) )

SELECT @PLAIN_TEXT AS PLAIN_TEXT,
       @ENCRYPTED_TEXT AS ENCRYPTED_TEXT,
       @DECRYPTED_TEXT AS DECRYPTED_TEXT

As expected, the above query gives the following result:

+--------------+------------------+------------------+
|  PLAIN_TEXT  |  ENCRYPTED_TEXT  |  DECRYPTED_TEXT  |
+--------------+------------------+------------------+
|  HelloWorld  |    w6==@(@C=5    |    HelloWorld    |
+--------------+------------------+------------------+

What do you think about my implementation ? Is there a way to improve it ? I know loops are things you're trying to avoid in SQL Server but is there a way to avoid using a loop in my case ?

Most functions are ugly in SQL Server. When there is something that needs to be done that is ugly, it's what they are used for IMHO. With that being said, here is a non-looping way. You'd have to run it in your environment to see which is faster, your method or one similar to this. I'd suspect this one to start being a lot faster as the size of the string grew. In that case, you may want to use the table version below.

Note, in your OP you stated "without special consideration to case". This method, since it uses ASCII conversions, is respective to case. However... this can be altered by using UPPER or LOWER to the entire string to maintain consistency.

DECLARE @PLAIN_TEXT nvarchar(MAX) = N'HelloWorld'

--split your string into a column, and compute the decimal value (N)
if object_id('tempdb..#staging') is not null drop table #staging
select
    substring(a.b, v.number+1, 1) as Val
    ,ascii(substring(a.b, v.number+1, 1)) as N
    --,row_number() over (order by (select null)) as RN
into #staging
from (select @PLAIN_TEXT b) a
    inner join
         master..spt_values v on v.number < len(a.b)
where v.type = 'P'

--select * from #staging

--create a fast tally table of numbers to be used to build the ROT-47 table.

;WITH
    E1(N) AS (select 1 from (values (1),(1),(1),(1),(1),(1),(1),(1),(1),(1))dt(n)),
    E2(N) AS (SELECT 1 FROM E1 a, E1 b), --10E+2 or 100 rows
    E4(N) AS (SELECT 1 FROM E2 a, E2 b), --10E+4 or 10,000 rows max
    cteTally(N) AS
    (
        SELECT  ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) FROM E4
    )

----uncomment this out to see the encrypted mapping...
--select
--  s.Val
--  ,s.N
--  ,e.ENCRYPTED_TEXT
--from #staging s
--left join(
--  select
--  N as DECIMAL_VALUE
--  ,char(N) as ASCII_VALUE
--  ,case
--      when 47 + N <= 126 then char(47 + N)
--      when 47 + N > 126 then char(N-47)
--  end as ENCRYPTED_TEXT
--  from cteTally
--  where N between 33 and 126) e on e.DECIMAL_VALUE = s.N

--Here we put it all together with stuff and FOR XML
select
    PLAIN_TEXT = @PLAIN_TEXT
    ,ENCRYPTED_TEXT =
        stuff((
        select
            --s.Val
            --,s.N
            e.ENCRYPTED_TEXT
        from #staging s
        left join(
        select
            N as DECIMAL_VALUE
            ,char(N) as ASCII_VALUE
            ,case
                when 47 + N <= 126 then char(47 + N)
                when 47 + N > 126 then char(N-47)
            end as ENCRYPTED_TEXT
        from cteTally
        where N between 33 and 126) e on e.DECIMAL_VALUE = s.N
        FOR XML PATH(''), TYPE).value('.', 'NVARCHAR(MAX)'), 1, 0, '')

drop table #staging

USING A TABLE OF VALUES TO CIPHER

declare @table table (ID int, PLAIN_TEXT nvarchar(4000))
insert into @table
values
(1,N'HelloWorld'),
(2,N'AnotherWord'),
(3,N'SomeNewWord')

--split your string into a column, and compute the decimal value (N)
if object_id('tempdb..#staging') is not null drop table #staging
select
    substring(a.b, v.number+1, 1) as Val
    ,ascii(substring(a.b, v.number+1, 1)) as N
    --,dense_rank() over (order by b) as RN
    ,a.ID
into #staging
from (select PLAIN_TEXT b, ID FROM @table) a
    inner join
         master..spt_values v on v.number < len(a.b)
where v.type = 'P'

--select * from #staging

--create a fast tally table of numbers to be used to build the ROT-47 table.

;WITH
    E1(N) AS (select 1 from (values (1),(1),(1),(1),(1),(1),(1),(1),(1),(1))dt(n)),
    E2(N) AS (SELECT 1 FROM E1 a, E1 b), --10E+2 or 100 rows
    E4(N) AS (SELECT 1 FROM E2 a, E2 b), --10E+4 or 10,000 rows max
    cteTally(N) AS
    (
        SELECT  ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) FROM E4
    )

--Here we put it all together with stuff and FOR XML
select
    PLAIN_TEXT
    ,ENCRYPTED_TEXT =
        stuff((
        select
            --s.Val
            --,s.N
            e.ENCRYPTED_TEXT
        from #staging s
        left join(
        select
            N as DECIMAL_VALUE
            ,char(N) as ASCII_VALUE
            ,case
                when 47 + N <= 126 then char(47 + N)
                when 47 + N > 126 then char(N-47)
            end as ENCRYPTED_TEXT
        from cteTally
        where N between 33 and 126) e on e.DECIMAL_VALUE = s.N
        where s.ID = t.ID
        FOR XML PATH(''), TYPE).value('.', 'NVARCHAR(MAX)'), 1, 0, '')
    from @table t


Numeric center in Java I got a proposed exercise from a Java2 book. This one wants me to find out the numeric centers.

A numeric center splits two list of numbers that make the same result when you add the numbers in each list. For example, 6 is the first numeric center because 1+2+3+4+5=15 and 7+8=15 too. The second numeric center is 35 because 1+2+3..+34=595 and 36+37+38+39=595 too.

 //I'll call the Numeric Center cn from here.
public class cnumerico {
    public static void main(String args[]){
        int n, index, fsum, ssum;
        n=204; //The higher number we would get.
        index=1; //No explanation need I guess.
        fsum=0; //Stands for first sum.
        ssum=1; //Stands for second sum.
        do{
            for (int i=1;i<index;i++){
                fsum=fsum+i; //We make the first sum from 1 until it reaches the index.
                }
            for (int j=index+1;j<index*2;j++){
                if (ssum==fsum){
                    System.out.println(index+" es un centro numerico.");
                    break;
                    //In this if I check if the numbers are equal, if it is we found our cn so I stop the loop.
                    }else{
                        ssum=ssum+j;
                        }
                //If we still didn't find our cn this else does the sum from index(+1 because the cn musn't be in the sum).
                }
            index++;
            fsum=0;ssum=0; //Resetting the sums to start again.
        }while (index<n+1);
    }
}

It works, but I have a feeling that I could do it much simpler.

About the code itself(not the algorithm used):

Class name usually starts with a capital letter in Java.
Variables' names: it is better to use more meaningful names. For example, a name firstSum is much clearer then fsum.
Variable declaration and initialization: it is a good practice to make a scope of each variable as narrow as possible. It means that fsum and ssum should be declared inside the loop's body.
If the number of iterations of a loop is known, it makes more sense to use for loop then do ... while. Conversely, when the number of iterations is not known, using while is better than using for and break.
Surrounding operators by whitespaces makes the code more readable:

for (int j = index + 1; j < index * 2; j++)

looks better than

for (int j=index+1;j<index*2;j++){

You should use proper and consistent indentation(closing bracket indented by 4 spaces looks strange).
I would declare a magic number(204) as a final static field of this class, not as a local variable(and a name n is not good for it because it is not meaningful).

So an improved version of your code can look like this:

public class NumericalCenter {

    private final static int MAX_CANDIDATE = 204;

    public static void main(String args[]) {
        for (int candidate = 1; candidate <= MAX_CANDIDATE; candidate++) {
            int firstSum = 0;
            for (int i = 1; i < candidate; i++)
                firstSum += i;
            int secondSum = 0;
            int i = candidate + 1;
            while (secondSum < firstSum) {
                secondSum += i;
                i++;
            }
            if (firstSum == secondSum)
                System.out.println(candidate + " is a numerical center");
        }
    }
}

Now about the algorithm itself: let's assume that a candidate number is fixed(let's call it x). Then we get an equation:
$${n(n+1) \over 2} - {x(x+1) \over 2} = {x(x-1) \over 2}$$

It is a quadratic equation(with respect to n), so it can be solved in O(1)(which gives O(MAX_CANDIDATE) time complexity in total). However, it is not necessary for MAX_CANDIDATE = 204.


Avoid duplicate conditional checks in multiple boolean conditions I have a method that returns either true or false based on a value contained by a java.util.List.

List<Long>list=new ArrayList<Long>(){{
    add(1L);
    add(2L);
    add(3L);
}};

public boolean checkId(Long id)
{
    return list.contains(id);
}

Based on the return value of this method and several other conditions a test is required to be performed.

boolean isIdChanged=true;
boolean isAjaxRequest=true;

Long id=1L;
Test test = new Test();

if(id!=null && id>0 && test.checkId(id) && isIdChanged || id!=null && id>0 && test.checkId(id) && !isAjaxRequest)
{
    //... do something
}

Assuming isIdChanged, isAjaxRequest and id have dynamic values.

In the if test, it can be noticed that the conditional expression id!=null && id>0 && test.checkId(id) is combined (repeated) on both side of the short-circuit || operator.

Is there a precise way to avoid this duplicate check?

One way to assign the result of this test to a boolean variable first like so,

boolean check=id!=null && id>0 && test.checkId(id);

and then the conditional test in the if statement can be trimmed like,

if(check && isIdChanged || check && !isAjaxRequest)
{
    //... do something
}

But I doubt it is still not sufficient. There should be a more precise way to achieve this.

Based on the comment below, the actual code is as follows (it is related to the JSF component library, Primefaces).

@Override
public List<StateTable> load(int first, int pageSize, String sortField, SortOrder sortOrder, Map<String, String> filters)
{
    int rowCount = stateService.rowCount().intValue();

    if(rowCount<=currentPage(first, pageSize)*pageSize-pageSize)
    {
        first-=pageSize;
    }

    if(id!=null&&id>0&&stateService.checkId(id)&&(isIdChanged||!FacesContext.getCurrentInstance().getPartialViewContext().isAjaxRequest()))
    {
        int currentRow=(int) (stateService.getCurrentRow(id)-1);  // Returns the current row number from the database.
        final DataTable dataTable = (DataTable) FacesContext.getCurrentInstance().getViewRoot().findComponent("form:dataTable");
        first=currentRow-(currentRow%pageSize);
        dataTable.setFirst(first); //Sets the starting index of the page holding the row based on the id supplied as a query-string.
        isIdChanged=false;
    }

    setRowCount(rowCount);
    return stateService.getList(first, pageSize, sortOrder, sortField);
}

In this method test has been replaced by stateService. The method is intended to fetch data from a database in a page-wise manner.

The first two parameters as their names imply, indicate the starting index where the rows are to be retrieved and the number of rows in a page - the page size respectively. The rest may be unrelated to the question.

The first line inside the method returns the number of rows in a given table.

The immediate conditional check evaluates to true only during the deletion of rows held by a table structure (Primefaces DataTable) on the client-side. When a user deletes rows on the last page, the previous page is opened automatically as soon as he/she deletes all of the rows in the last page.

Afterwards the statements inside an if block are meant to highlight a row. When an id is supplied as a GET request parameter, the page containing that row should be opened automatically and the row corresponding to the id (supplied as a query-string) is highlighted by a different background colour of that row on the client-end.

Something similar happens, when you click this link pointing to a question on StackOverflow which automatically opens up the fifth page containing the answer this link is pointing to.

And finally, the number of rows are set by calling a super class method setRowCount() and the List is returned from the database.

Why don't you just write it like this?  It should be logically equivalent.

if (id != null && id > 0 && test.checkId(id) && (isIdChanged || !isAjaxRequest))
{
    // do stuff
}

On a side note, I would move the null and positive id checks into your "checkId" method.  In fact, I think in this case, these checks are redundant and unnecessary.  You're just checking if an id is in an existing list that you're maintaining.  If it's null, it's unlikely it will exist in the list (if you are preventing arbitrary values from being added).  An id outside of the range of expected values will just not be in the list in the first place and therefore is unneeded as well.

But, if you must perform these checks, do it within the checkId method instead.

public boolean checkId(Long id)
{
    // we don't have null ids
    if (id == null)
        return false;
    // we're expecting positive ids
    if (id <= 0)
        return false;

    return list.contains(id);
}

This reduces your outer condition to simply:

if (test.checkId(id) && (isIdChanged || !isAjaxRequest))


String converting and parsing a few rows of a .csv file There are not many available resources for refactoring go code.  I'm a novice gopher who would appreciate any feedback on a small program which reads a .csv file, string converts and parses a few rows in order to perform a sub method.

package main

import (
    "encoding/csv"
    "fmt"
    "io"
    "log"
    "os"
    "strconv"
)

const file = "csvdata/csvtest.csv"
// what should stay in main()?  What should be refactored?
func main() {
    f, err := os.Open(file)
    if err != nil {
        log.Fatalf("Error reading all lines: %v", err)
    }
    defer f.Close()

    reader := csv.NewReader(f)
    reader.Comma = ';'

    for {
        record, err := reader.Read()
        if err == io.EOF {
            break
        } else if err != nil {
            log.Print(err)
            os.Exit(-1)
        }
        // need to refactor variables, could I use a type struct?
        var num string = (record[2])
        var name string = (record[3])
        var eia string = (record[5])
        var cia string = (record[6])
        var inc_percent = (record[7])
        var estIncTxn string = (record[8])
        var inc_diff float64
        // I would like to turn this chunk into it's own function
        for i := 0; i < len(record[i]); i++ {
            estInc, err := strconv.ParseFloat(eia, 64)
            actInc, err := strconv.ParseFloat(cia, 64)
            inc_diff = (actInc - estInc)
            if err == nil {
        //How could I turn the following into a template?
                fmt.Println("============================================================\n")
                fmt.Printf("Account: %+s - %+s exceeded the IncAmt by %+v same as $%+v\n", num, name, inc_percent, inc_diff)
                fmt.Printf("over the monthly incoming amount of $%+v. Currently, the declared\n", actInc)
                fmt.Printf("profile is established at $%+v with an expectancy of (%+v).\n", estInc, estIncTxn)
            } else {
                log.Fatalf("Error converting strings: +v", err)
            }

        }
        fmt.Println()
    }
}

Here is a single line of the csv file.

30;4;102033657;COCACOLA;53764;15000.00;73010.58;387%;4;30;650%;15000.00;89558.96;497%;3;5;66%;DDA

Sample of the output:

  ==============================================================================

  Account: 102033657 - COCACOLA exceeded the IncAmt by 387% same as
  $58010.58 over the monthly incoming amount of $73010.58. Currently,
  the declared profile is established at $15000 with an expectancy of
  (4).

  ==============================================================================

Eventually I would be capturing the outgoing amount as well for each record (just to give everyone an idea of where I'm going with this).  I'm assuming a template would be the best way to approach this situation?  How could this be refactored?

for i := 0; i < len(record[i]); i++ {

... I don't think that does what you intended. It seems like just luck from how the data is that it works, or did I miss something? :-) I think you can just take the for {} loop out altogether.

I'd move the ParseFloat() calls up to where you assign the record[] values to named variables. Be sure to check the err return from both calls.

Your idea of using a struct is also good.

type SomeRecord struct {
    Num  int
    Name string
    Eia  float
    // etc
}

The for moving some of the code to a function: Have the function take a SomeRecord as a parameter and do the output.

I don't know if using a template is necessary for such a short string, but if you want to then outside of the loop read the template file with text/template, specifically template.ParseFiles and then execute the template when appropriate with the record data or a new data structure if it needs data not in the record. If this is all the program does it might make most sense to have the SomeRecord struct have the data that the template needs (so include IncDiff).


Python editing lists and converting to a dictionary After validation my errors list returns

[(False, {u'first_name': u'First name is too short'}), (False, {u'last_name': u'Last name is too short'}), (False, {u'confirm_password': u'Password is too short'}), (False, {u'email': u'Please enter a valid email'})]

I use a for loop to create a new list and only add the one index in every element.  I am trying to get rid of every False.  I am looking for a way to improve my code.

Here is my code

errors = []
errors.append(self.validate_length(first_name, 'first_name', 2, "First name is too short"))
errors.append(self.validate_length(last_name, 'last_name', 2, "Last name is too short"))
errors.append(self.password_match(password, confirm_password))
errors.append(self.validate_email(email_address))

error = []
print errors
for elements in range(0, len(errors)):
    try:
        errors[elements][1]
        error.append(errors[elements][1])
    except:
        pass

I have a few complaints:

You're using a for loop to iterate over the indexes of a list instead of directly iterating over the list, but you're not actually using the indexes for anything. I'd suggest

for element in errors:

instead, then using element instead of errors[elements]
The line errors[elements][1] doesn't actually appear to do anything, except throw an IndexError if the list is length 1 or shorter. If this is the intended behaviour, it could be made clearer using a check such as if len(errors[elements]) < 2: ..., and perhaps avoid using exceptions entirely.
An except which catches every type of error there is is rarely a good idea - it's unclear and risks swallowing exceptions you actually want to be warned about in the future. If I understand your code correctly, except IndexError would do the same thing, but be slightly clearer.

All in all, however, it appears you're using exceptions for a case where they aren't needed, which tends to make code harder to grok overall. I'd probably rewrite the loop to something along the lines of

for element in errors:
    if len(element) >= 2:
        error.append(element[1])

though a list comprehension such as error = [element[1] for element in errors if len(element) >= 2] could work too.


"Jedi's riddle" challenge The challenge can be found here.
In short we have a recursive function, and we need to find when \$z_i\$ is equal to one.
$$
\begin{align}
    z_1 &= 1\\
    z_i &= z_{i-1} * a\ \%\ n\\
    \text{find}\ z_i &= 1
\end{align}
$$
Input range:
$$ 2 \le a \lt n \le 10^9 $$
I have written the following. It is simple, but too slow.
import math

def solution(a, n):
    if math.gcd(a, n) > 1:
        return 0
    z = 1
    i = 1
    while True:
        z = z * a % n
        if z == 1:
            return i
        i += 1
l = map(int, input().split(' '))
print(solution(*l))

Code Review
Your code is a little hard to read.
You should have a blank line after the solution body, to separate it from the mainline code.
This code is hard to describe, document, and debug:
l = map(int, input().split(' '))
print(solution(*l))

What is l?  How many arguments are there?  If the wrong number of arguments are given as input, the problem doesn't occur immediately in the first statement but rather when attempting to execute the second.
It would be significantly clearer to write:
a, n = map(int, input().split())
print(solution(a, n))

Now we can see we expect two arguments to be given, and we have names for the arguments.  If too many or too few arguments are given, the first line will raise the exception, which should mean it will be easier to debug.
Note: I'm using .split() instead of .split(' ') so that it will split the string on any white space character, including tabs, and multiple white-space characters will be considered a single delimiter.  It is slightly friendlier.
Adding type hints and a docstring would be useful.  Also, a main guard should be used.
Improved code:
import math

def solution(a: int, n: int) -> int:
    """
    Jedi Challenge:

    Return the smallest positive integer such that
        a^i = 1 (mod n)
    Returns 0 if there is no such number

    Constraints:
        2 <= a < n <= 10^9
    """

    if math.gcd(a, n) > 1:
        return 0

    z = 1
    i = 1
    while True:
        z = z * a % n
        if z == 1:
            return i
        i += 1

if __name__ == '__main__':
    a, n = map(int, input().split())
    print(solution(a, n))

Performance
This is not really a programming challenge; it is a math challenge.
First, due to the modulo operation, z = z * a % n will only ever produce a value between 0 and n-1.  This means there are only n possible results at each iteration step.  However, if 0 is produced, each iteration step afterwards will only ever produce 0, leaving n-1 non-trivial results.
If the k-th iteration produces 1, the code yields the result k.  But if iteration continued, since it has reached its initial starting point, it would produce the same sequence of values in a never ending cycle.
If you chose a different starting value, that did not appear in the above cycle, it would must have one of 3 fates.  It would either terminate at zero, create its own loop of unique values, or possibly merge into the existing sequence.
You have already determined that if gcd(a, n) != 1 then the iteration sequence would eventually reach zero, without finding a solution.
In fact, if n is prime, then \$a^{n-1} \equiv 1 \mod n\$, and therefore the iteration will produce the 1 after n-1 iterations, and must have produced all of the possible values between 1 and n-1.
So, as one optimization, if you can determine n is prime, you could immediately output the answer n-1.  If not, then you could revert to doing the iterative calculations.
If n is not prime, you might want to look at the prime factorization of n, and see if you can determine a relation between it and the result.  (Hint: there is.)


comparing lists of tuples I have the following code, comparing two lists that contains equally structured tuples:
list_one = [('id_01','aaa','bbb'), ('id_02','aaa','bbb'), ('id_03','aaa','bbb'), ('id_04','aab','bbc')]
list_two = [('id_01','aaa','bbb'), ('id_02','aaa','bbb'), ('id_03','aad','bbd')]

for tupl in list_one:
    for tup in list_two:
        if tupl[0] == tup[0] and (tupl[1] != tup[1] or tupl[2] != tup[2]):
                print("There is a difference on "+
                      "{} between the two lists".format(tup[0]))

this code would print
There is a difference on id_03 between the two lists
which is the expected result.
But I wonder if there is a better way to achieve the same result, maybe whitout iterating through list_two for every item of list_one, but any suggestion will be appreciated.

I think you would be better off using set. It provides features like set difference by itself, thereby reducing a lot of work at your end:
set_one = set([('id_01','aaa','bbb'), ('id_02','aaa','bbb'), ('id_03','aaa','bbb'), ('id_04','aab','bbc')])
set_two = set([('id_01','aaa','bbb'), ('id_02','aaa','bbb'), ('id_03','aad','bbd')])
for difference in (set_two - set_one):
    print(f"There is a difference on {difference[0]} between the two lists")


Create a Christmas Tree This function draws a Christmas tree (with an angel on top) with text (AKA, ASCII art).

#define ARRSZ(x) (sizeof(x)/sizeof(x[0]))

void drawChristmasTree (int height) {
    const char * const angel[] = { " ._.", "(\\o/)", ")/|\\(", "/_*_\\", };
    const char stars[] = "***************************************";
    int i;
    int offset = 20 - height;

    if ((height < ARRSZ(angel)) || (height > ARRSZ(stars)/2)) {
        printf("The height has to be at least %zu and no more than %zu.\n",
               ARRSZ(angel), ARRSZ(stars)/2);
        return;
    }

    for (i = 0; i < height; ++i) {
        printf("%*s%s\n",
               height - (i < ARRSZ(angel) ? ARRSZ(angel) : i + 1), "",
               (i < ARRSZ(angel)) ? angel[i] : &stars[ARRSZ(stars) - 2*i]);
    }
    printf("%*s%c\n", height - 2, "", '*');
    printf("%*s%c\n", height - 2, "", '*');
}

For size 10, this code produces:

       ._.
      (\o/)
      )/|\(
      /_*_\
     *******
    *********
   ***********
  *************
 ***************
*****************
        *
        *

Compiler warnings

Always compile with warnings turned on.  Your compiler should have warned you that offset is an unused variable.  My compiler also complained about passing an unsigned int as an argument to printf("%*") with height - (i < ARRSZ(angel) ? ARRSZ(angel) : i + 1).

Error handling

The output should either be a Christmas tree or nothing at all.  Printing an error message in place of a tree is just mean.

If you must print an error message, print it to stderr!

Code formatting

Why not make the angel look like an angel in your code?  Granted, the double backslashes interfere with the alignment, but at least you could try.

const char * const angel[] = { " ._.",
                              "(\\o/)",
                               ")/|\\(",
                               "/_*_\\", };

Looping

With those conditional expressions within the loop, you might as well split it into two loops:

for (i = 0; i < ARRSZ(angel); ++i) {
    printf("%*s%s\n", height - (int)ARRSZ(angel), "", angel[i]);
}
for (; i < height; ++i) {
    printf("%*s%s\n", height - i - 1, "", stars + ARRSZ(stars) - 2*i);
}

Here, I've also rewritten &stars[ARRSZ(stars) - 2*i] using the more idiomatic pointer arithmetic.

Alternate solution

If you ensure that each line of the angel has the same length, then you could also write the printf() calls as

const char * const angel[] = { " ._. ",
                              "(\\o/)",
                               ")/|\\(",
                               "/_*_\\", };

for (i = 0; i < ARRSZ(angel); ++i) {
    printf(" %*s\n", (int)(height - ARRSZ(angel) + ARRSZ(angel[0]) / 2), angel[i]);
}
for (; i < height; ++i) {
    printf("%*.*s\n", height + i - 2, 2 * i - 1, stars);
}


Make sequential fadeIn() promises more manageable I've two versions of promise code. The page is a simple one - an unordered list with a few items. I am trying to make the items show up (fadeIn) one-after-another sequentially. This exercise is done to sort of explain people what promises are...

Version 1:

$('li').hide();

var fadeInFunc = function (obj) {
    var deferred = $.Deferred();
    obj.fadeIn('slow', function () {
        deferred.resolve();
    });
    return deferred.promise();
};

var funcWrapper = function (obj) {
    return function(){
        return fadeInFunc(obj);
    };
};

$('input').click(function () {
    fadeInFunc($('#li1'))
        .then(funcWrapper($('#li2')))
        .then(funcWrapper($('#li3')));
});

Version 2:

$('li').hide();

var fadeInFunc = function (obj) {
    var deferred = $.Deferred();
    obj.fadeIn('slow', function () {
        deferred.resolve();
    });
    return deferred;
};

var execute = function () {
    return fadeInFunc(this);
};

$('input').click(function () {
    fadeInFunc($('#li1'))
        .then(execute.bind($('#li2')))
        .then(execute.bind($('#li3')));
});

Both work fine. However, I want someone to dive in and try to understand the code better. Moreover, this code becomes unmanageable when you have variable number of li elements.

Here is the version for variable number of li elements:

var liCount = Math.floor(Math.random() * 6) + 1;
$('p').html('liCount: ' + liCount);
var ul = $('ul');
for(var i = 0; i< liCount; i++){
    ul.append('
Item ' + (i+1) + '
');
}
var liArray = $('li').hide();

var fadeInFunc = function (obj) {
    var deferred = $.Deferred();
    obj.fadeIn('slow', function () {
        deferred.resolve();
    });
    return deferred;
};

var funcWrapper = function(o){
    return function() { return fadeInFunc(o); };
};

$('input').click(function () {
    var prev;
    liArray.each(function(idx, obj){
        var o = $(obj);
        if(idx === 0){
            prev = fadeInFunc(o);
        }
        else{
            prev = prev.then(funcWrapper(o));
        }
    });
});

You don't actually need promises for this one. Here's an overly simplified version that takes advantage of the fact that you can call the containing function by a given name.

// Let's turn a jQuery set to an array coz it doesn't have shift, which we'll use
var liArray = $('li').hide().get();

$('input').click(function handle(){
  // Unqueue one from the set and fade it in. Note that shift() alters the array so
  // the next time we call it, the second one is in front of the queue. Also wrapping
  // what we shifted with jQuery because get() returns an array of DOM elements of
  // the jQuery set
  var li = $(liArray.shift());

  // If we shifted out nothing, then that's the end of this.
  if(!li) return;

  // Set the callback of the fadeIn to the same function. That way, if the animation
  // completes, then it fires again.
  li.fadeIn(handle);
});

If the idea is to teach promises, then the following might be better. Here's an interesting approach where we use Array.prototype.reduce to carry over a dummy deferred, attach the handlers and kick it off, starting the fadeIn process.

var liArray = $('li').hide().get();

// So here we create a deferred object and during its creation, fade in the element.
// When the animation completes, it calls on `deferred.resolve` which resolves the
// fade promise.
function fade(item) {
  return $.Deferred(function (deferred) {
    $(item).fadeIn(deferred.resolve);
  }).promise();
}

// Here we use reduce to attach a handler for each item as well as allow us to carry
// over a value to the next iteration - the result value of `then()`.
function reducer(promise, item) {
  return promise.then(function () {
    return fade(item);
  });
}

// Here we create a dummy promise which allows our reducer to attach the handlers.
// Then we use reduce to attach a handler for each item on the promise and then kick
// of the whole operation by resolving the dummy promise.
$('input').click(function handle() {
  var root = $.Deferred();
  liArray.reduce(reducer, root.promise());
  root.resolve();
});

Both examples assume liArray is an array containing li elements, whether dynamically generated or just fetched from the page, either way it doesn't matter as long as they're already rendered. Also went for an array instead of a jQuery object because array operations like shift() and reduce() are very handy.


Multivariable Gradient Descent in Numpy Just recently started learning ML, first I've gone through the notes of Ng's Coursera stuff. While I have nothing against Octave, I'm trying to solve exercises in Python. It's my beginning with that kind of algorithms, though I got mathematical background, so sorry for a bit messy code. My questions:
Is there a way to make it more readable, and where to find datasets with solutions to test? Also is that conversion to float in gradient descent main loop unavoidable?

import numpy as np

def scaling(X):
    """mean normalization"""
    l = []
    for k in range(X.shape[1]):
        x = X[:, k]

        def f(e):
            tmp0 = sum(x) / len(x)
            tmp1 = max(x) - min(x)
            return (e - tmp0) / tmp1

        m = list(map(float, (list(map(f, x[:, 0])))))
        l.append(m)
    return np.matrix(l).transpose()

l_cost_function = []  # lists to record data for debugging
l_iterations = []

def multivariate_g_d(A, y, alfa):
    """computes gradient descent"""
    X = np.c_[np.ones(len(A), A]
    tmp = np.matrix([0, 0, 0], dtype=np.float64)
    theta = np.matrix([0, 0, 0], dtype=np.float64)
    cnt = 0
    m = len(X)
    ma = alfa * (1 / m)
    delta = J_m(X, y, theta)
    while delta > 0.00001:
        beg = J_m(X, y, theta)
        for j in range(X.shape[1]):
            tmp[:, j] = theta[:, j] - ma * float((((X *   theta.transpose()) - y.transpose()).transpose()) * X[:, j])
        theta = tmp
        end = J_m(X, y, theta)
        l_cost_function.append(end)
        l_iterations.append(cnt)
        delta = abs(end - beg)
        cnt += 1
    return (theta, cnt)

def J_m(A, y, theta):
    """computes cost function """
    n = len(A)
   return (1 / (2 * n)) * float(sum([x * x for x in A * theta.transpose() - y.transpose()]))

Without sample inputs I can't run your whole code.  And I prefer not to guess.

The use of np.matrix suggests it was translated from MATLAB/Octave code.  That array subclass, in numpy, is always 2d, which makes it behave more like MATLAB matrices, especially old versions.  Transpose always has effect; row and column indexing returns 2d matrices; and * is matrix multiplication (as opposed to element wise, the .* of MATLAB).

I'll focus on the scaling function.  I don't see it being used, but it's simple and typical of the other functions.

import numpy as np
<your code>

X = np.arange(12).reshape(3,4)
print(X)
Xm = np.matrix(X)
print(scaling(Xm))

produces:

[[ 0  1  2  3]
 [ 4  5  6  7]
 [ 8  9 10 11]]
[[-0.5 -0.5 -0.5 -0.5]
 [ 0.   0.   0.   0. ]
 [ 0.5  0.5  0.5  0.5]]

scaling doesn't work with X an array, because x = X[:, k] would be 1d, which conflicts with the x[:, 0] use.

But I can perform this same operation without row iteration.

def ascaling(X, n=0):
    # array scaling
    ctr = X.mean(axis=n, keepdims=True)
    rge = X.max(axis=n, keepdims=True)-X.min(axis=n, keepdims=True)
    return (X - ctr)/rge
print(ascaling(X,0))
print(ascaling(X,1))

producing

[[-0.5 -0.5 -0.5 -0.5]
 [ 0.   0.   0.   0. ]
 [ 0.5  0.5  0.5  0.5]]
[[-0.5        -0.16666667  0.16666667  0.5       ]
 [-0.5        -0.16666667  0.16666667  0.5       ]
 [-0.5        -0.16666667  0.16666667  0.5       ]]

In this case, the equivalent code, assumng X is np.matrix is simpler

def mscaling(X, n=0):
    # matrix scaling
    ctr = X.mean(axis=n)
    rge = X.max(axis=n)-X.min(axis=n)
    return (X - ctr)/rge

print(mscaling(Xm,0))
print(mscaling(Xm,1))

Maybe this example will make these operations clear:

Make a small 2d array:

In [861]: X=np.arange(6).reshape(2,3)
In [862]: X
Out[862]:
array([[0, 1, 2],
       [3, 4, 5]])

Sum rows, resulting in a 1d array of length 3 (the columns)

In [863]: X.sum(axis=0)
Out[863]: array([3, 5, 7])

Sum columns - again a 1d result

In [864]: X.sum(axis=1)
Out[864]: array([ 3, 12])

But if I add keepdims, the result is 2d, (2,1) shape:

In [866]: X.sum(axis=1,keepdims=True)
Out[866]:
array([[ 3],
       [12]])

sum applied to matrix does the same thing.

In [867]: np.matrix(X).sum(axis=1)
Out[867]:
matrix([[ 3],
        [12]])

The nice thing about keeping dims is that I can do math like

In [868]: X-X.sum(axis=1,keepdims=True)
Out[868]:
array([[-3, -2, -1],
       [-9, -8, -7]])

Without keepdims I'd have to do X-X.sum(axis=1)[:,None].

Your frequent use of transpose suggests that the mix of dimensions haven't been fully thought out.  MATLAB code does use x' or x.' quite a bit.  On the other hand beginner numpy coders try to apply transpose to 1d arrays, and wonder why nothing happens.


Extracting the location for a page with fewer conditionals I've been given the job of trying to reduce the line count for a file that helps generate a PDF for a web app. The class itself is pdfHelper, and contains a number of methods used to generate a PDF such as getting the location, custom methods for formatting lowagie.text table cells etc.

I came across this particular method:

    public String getLocationToString(Page aPage) {
    if (aPage.getSiteDetail() != null || aPage.getLocationDescription() != null) {
        if (aPage.getSiteDetail() != null && aPage.getLocationDescription() == null) {
            return aPage.getSiteDetail();
        } else if (aPage.getSiteDetail() != null && aPage.getLocationDescription().getLocationLevel() == null) {
            return aPage.getSiteDetail();
        } else if (aPage.getSiteDetail() != null && aPage.getLocationDescription().getLocationLevel() != null) {
            return aPage.getSiteDetail().trim().toString() + "-" + aPage.getLocationDescription().getLocationLevel();
        } else {
            return "N/A";
        }
    } else {
        return "N/A";
    }
}

The method getSiteDetail() returns a String, getLocationDescription retrieves a LocationDescription object from the Page object. getLocationLevel() is a method for the LocationDEscription object and also returns a String.

Is there a way to shorten this method? I can't seem to figure out a way to reduce the amount of conditionals in this method (admittedly I'm not much of a programmer yet).

In advance sorry for the formatting but I code in C# so the it's probably slightly different than you're used to:

Using else if

Although this doesn't actually improve number of if statements, why are you using else if? If the previous if statement is satisfied you're returning a value anyway, so the code won't execute the next if statement anyway. This also renders your else keyword pointless too because of the same reasons

Combining conditionals

You are returning aPage.getSiteDetail() in two places, you can combine these if statements together:

if (aPage.getSiteDetail() != null &&
    (aPage.getLocationDescription() == null || aPage.getLocationDescription().getLocationLevel() == null))
{
    return aPage.getSiteDetail();
}

After this there's no point in checking if aPage.getLocationDescription() == null || aPage.getLocationDescription().getLocationLevel() == null because if aPage.getLocationDescription() then aPage.getLocationDescription().getLocationLevel() has to be null:

Repeated method calls

You can really improve the readability of your code and minimise the number of external method calls your making by assigning the return value of your method calls to a variable:

string siteDetails = aPage.getSiteDetail();
// Here you need to replace var with whatever the return value of aPage.getLocationDescription() is
var locationDescription = aPage.getLocationDescription();
string locationLevel = locationDescription != null ? locationDescription.getLocationLevel() : null;

Pointless Conditionals

If aPage.getSiteDetail() != null || aPage.getLocationDescription() != null but none of the rest of your conditionals are satisfied you return "N/A" but you also do this if both are null anyway. So your first conditional is pointless, because you continuously evaluate those values later on making a final code looking something like this:

public String getLocationToString()
{
    string siteDetails = aPage.getSiteDetail();
// Here you need to replace var with whatever the return value of aPage.getLocationDescription() is
    var locationDescription = aPage.getLocationDescription();
    string locationLevel = locationDescription != null ? locationDescription.getLocationLevel() : null;

    if (siteDetails != null))
    {
        if (locationDescription == null)
        {
            return siteDetails;
        }
        if (locationLevel != null)
        {
            return siteDetails.trim() + "-" + locationLevel;
        }
    }

    return "N/A";
}

Ternary Operators

I already used these when assigning a value to locationLevel but you could also do it for the following two if statements which means you'd only have one if in your whole function:

if (locationDescription == null)
{
    return siteDetails;
}
if (locationLevel != null)
{
    return siteDetails.trim() + "-" + locationLevel;
}

Becomes:

return locationDescription == null ? siteDetails : siteDetails.trim() + "-" + location.level;

Could even become:

return siteDetails.trim() + (locationDescription == null ? "" : location.level);


Function to parse an XML and find specific regex considering the size attribute of the document I'm writing a code that, starting from an XML file:

stores the index of child elements of a tag and the child elements as
key, values in a dictionary (function get_xml_by_tag_names);
deletes keys whose values contain a certain string (the specific text
size) and puts these keys and the corresponding values into a second
dictionary (def search_delete_append);
joins, for each dictionary, the dict values and extracts their
text(def main);
replaces certain values with "" (def main);
counts the occurrences of specific regex I specify (def find_regex).

It works, but the "main" function needs to be more cleaned up. My concerns specifically regard the part in which I have to list the regex I'm interested in- they're multiple and it can become messy. Another problem is that the cleaning of the XML can be done in another separate function, but so I haven't managed to do it.

Here is the code:

import re
from xml.dom import minidom
from xml.etree import ElementTree as ET
from bs4 import BeautifulSoup

def get_xml_by_tag_names(xml_path, tag_name_1, tag_name_2):

    data = {}
    xml_tree = minidom.parse(xml_path)
    item_group_nodes = xml_tree.getElementsByTagName(tag_name_1)
    for idx, item_group_node in enumerate(item_group_nodes):
        cl_compile_nodes = item_group_node.getElementsByTagName(tag_name_2)
        for _ in cl_compile_nodes:
            data[idx]=[item_group_node.toxml()]
    return data

def find_regex(regex, text):
    l = []
    matches_prima = re.findall(regex, text)
    print("The number of", {regex}," matches is ", len(matches_prima))

def search_delete_append(dizionario, dizionariofasi):
    deletekeys = []
    insertvalues = []
    for k in dizionario:
        for v in dizionario[k]:
            if "7.489" in v:
                deletekeys.append(k)
                dizionariofasi[k] = v

    for item in deletekeys:
        del dizionario[item]

def main():
    dict_fasi = {}
    data = get_xml_by_tag_names('output2.xml', 'new_line', 'text')
    search_delete_append(data, dict_fasi)

    testo = []
    for value in data.values():
        myxml = ' '.join(value)
        tree = ET.fromstring(myxml)
        tmpstring = ' '.join(text.text for text in tree.findall('text'))
        for to_remove in (" < ", " >", ".", ",", ";", "-", "!", ":", "’", "?", "<>"):
            tmpstring = tmpstring.replace(to_remove, "")
        testo.append(tmpstring)
    testo = ''.join(testo)
    #print(testo)
    find_prima = re.compile(r"\]\s*prima(?!\S)")
    #print(find_regex(find_prima, testo))
    #################

    testo_fasi = []
    values = [x for x in dict_fasi.values()]
    myxml_fasi = ' '.join(values)
    find_CM = re.compile(r"10\.238")
    print(find_regex(find_CM, myxml_fasi)) #quanti CM ci sono?
    #print(myxml_fasi)
    for x in dict_fasi.values():
        xxx= ''.join(x)
        tree2 = ET.fromstring(xxx)
        tmpstring2 = ' '.join(text.text for text in tree2.findall('text'))
        testo_fasi.append(tmpstring2)
    testo_fasi = ''.join(testo_fasi)
    print(testo_fasi)
    find_regex(find_prima, testo_fasi)

if __name__ == "__main__":
    main()

Mixed languages

Regardless of one's opinion on whether English is the lingua franca of programming, mixing languages is a bad idea. This would be better in all-Italian (where possible; libraries are still in English) or all-English than a mixture.

Type hints

Use them; for instance

def get_xml_by_tag_names(xml_path: str, tag_name_1: str, tag_name_2: str) -> dict:

Unused variable

Here:

l = []

Items iteration

for k in dizionario:
    for v in dizionario[k]:

should be

for k, values in dizionario.items():
    for v in values:

Magic numbers

Move this:

"7.489"

to a named constant variable somewhere.

Multiple replacement

This:

    for to_remove in (" < ", " >", ".", ",", ";", "-", "!", ":", "’", "?", "<>"):

would be easier as a single regular expression and a single call to sub.

Intermediate list

values = [x for x in dict_fasi.values()]
myxml_fasi = ' '.join(values)

can just be

myxml_fasi = ' '.join(dict_fasi.values())


Calculate how many coins, coin values are 50, 20, 10 and 5 The code should enter an integer between 5-95 and calculate how many coins, coin values are 50, 20, 10 and 5.

My concern is the CalcChange function and whether that is acceptable C code.

#include <stdio.h>

#define TRUE 1
#define FALSE 0

void GetNum(int &amount)
{
        printf("Enter an integer between 5 and 95 inclusive:\n");
        scanf("%d%*c", &amount);

        return;
}

int Verify(int amount)
{
        if(amount <= 4 || amount >= 95)
                return FALSE;

        return(TRUE);
}

//Function calculates number of coins for each denomination
//and returns the remainder as the new value of amount
//until remainder is <= 4

int CalcChange(int &amount, int denomination)
{

        int change = amount / denomination;

        amount = amount % denomination;

        return(change);

}

//Instead of wasting the remainder, lets donate it to charity
void PrintResult(int fifty, int twenty, int ten, int five, int remainder)
{
        printf("\n%d 50c coins\n%d 20c coins\n%d 10c coins\n%d 5c coins.\n", fifty, twenty, ten, five);
        printf("\nThe remainder of %d cents will be donated to charity.\n", remainder);

        return;
}

int main()
{
        int a, change, fifty, twenty, ten, five, remainder;

        GetNum(a);

        Verify(a);

        if(Verify(a))
        {
                fifty = CalcChange(a, 50);
                twenty = CalcChange(a, 20);
                ten = CalcChange(a, 10);
                five = CalcChange(a, 5);
                remainder = CalcChange(a, 1);

                PrintResult(fifty, twenty, ten, five, remainder);
        }
        else
        {
                printf("\n%d is out of range!\n", a);
        }

        return(0);
}

Invalid C code

OP’s C compiler may support various language extensions, but int &amount is not supported in standard C  @Ben Steffan. This makes the code less portable and harder to review.

// void GetNum(int &amount)
// scanf("%d%*c", &amount);
void GetNum(int *amount)  // Let caller pass the address of `amount` instead
scanf("%d%*c", amount);

Lack of error checking/weak end-of-line consumption

scanf("%d%*c", &amount); does not check the return value from scanf().  Should the user enter non-numeric data, nothing is scanned, amount will not be changed and offending input remains in stdin.
Alternative code;

int GetNum(int *amount) {
  printf("Enter an integer between 5 and 95 inclusive:\n");
  char buf[80];
  if (fgets(buf, sizeof buf, stdin)) return EOF;
  char ch;
  // Does input consists of one `int` and not trailing junk?
  if (sscanf(buf, "%d %c", amount, &ch) != 1) return 0; // Invalid input
  return 1; // success;
}

Unneeded ()

// return(change);
return change;

Consider standard true/false and bool.

// #define TRUE 1
// #define FALSE 0
// int Verify(int amount)
//   return FALSE;

#include <stdbool.h>
bool Verify(int amount)
  return false;

See also to use '#include ' in VS 2010
 and We also added the new headers ..., stdbool.h,

Thought for a likely next assignment:

Consider how to make 60c change when the available coins are only 50c,20c,20c,20c using the below algortim.

fifty = CalcChange(a, 50, 1);
twenty = CalcChange(a, 20, 3);
ten = CalcChange(a, 10, 0);


Sum of two elements of a list equaling a target In effort to learn OCaml, I'm going through some easy LeetCode problems and attempting to implement them. I arrive at the correct answer, but I'm still developing an intuition for canonical OCaml code. I'd appreciate any feedback on my implementation.

As mentioned, this is problem 1 on LeetCode. A quick summary of the problem: given an array of integers, return the indices of the 2 numbers that sum to a given "target".

For example, given nums: [2, 7, 11, 15] and target: 13, the result should be [0, 2] (the integers at indices 0 and 2 sum to 13).

Furthermore, we can assume a unique solution exists for the given array and target.

Here is my refined attempt at implementing this in OCaml:

let two_sum nums target =
  let rec aux i j =
    let n1 = List.nth nums i in
    let n2 = List.nth nums j in
      if n1 + n2 = target then [i; j]
      else if j = (List.length nums) - 1 then aux (i + 1) (i + 2)
      else aux i (j + 1)
  in aux 0 1;;

I appreciate any feedback.

Since the task is about using arrays, I would suggest to use OCaml arrays and not list. Because of lists, you need to use List.nth and that is quite costly.