Server code to save uploaded files This code basically saves an uploaded file to the server. I am wondering if there is  anything that I can do to tighten this code up.  I am very new to F# so I'm still having trouble breaking away from the C# way of doing things.

/// Create file paths
/// Returns a tuple (server path * link path * file number)
let createPath (file : HttpPostedFileBase) =

    // server directory path
    let serverDirPath =
        HttpContext.Current.Server.MapPath(@"~/Uploads")

    // get the file name
    let origFileName = file.FileName

    // get the extension
    let extension = Path.GetExtension(origFileName)

    // get the file size in bytes
    let fileSize = file.ContentLength

    // directory check
    let pathExists() = Directory.Exists(serverDirPath)

    // create directory
    let createDir() =
        if not (pathExists()) then
            Directory.CreateDirectory(serverDirPath) |> ignore
    createDir()

    // find current file name
    let findCurrentFileName() =

        // check if row exist
        let rowCount =
            query{
                for row in db.Uploads do
                select row
                count
            }

        // get file number
        let fileNumber =
            if rowCount < 1
                then
                    1
                else
                    query{
                        for row in db.Uploads do
                        select (row.FileNumber + 1)
                        head
                    }

        // final path
        let finalServPath =
            serverDirPath + @"\" + fileNumber.ToString() + extension

        // download link
        let linkPath =
            finalServPath.Replace(serverDirPath + @"\", @"~/Uploads/")

        finalServPath, linkPath, fileNumber

    findCurrentFileName()

/// Save file to server and path to db.
let SaveUpload (file: HttpPostedFileBase) (title : string) =

    // create the path including filename
    let servPath, linkPath, fileNumber = createPath file

    // save file to server
    file.SaveAs(servPath)

    // create new row for db table
    let newUpload =
        new dbSchema.ServiceTypes.Uploads(Title = title,
                                            FilePath = servPath,
                                            Size = file.ContentLength.ToString(),
                                            FileNumber = fileNumber,
                                            LinkPath = linkPath)
    // insert new row
    insertRowIn db.Uploads newUpload

    // save to db
    saveToDb()

I think that you're using comments unnecessarily. If you set a variable called fileNumber, the comment // get file number doesn't tell the reader anything new and only clutters the code.

// directory check
let pathExists() = Directory.Exists(serverDirPath)

// create directory
let createDir() =
    if not (pathExists()) then
        Directory.CreateDirectory(serverDirPath) |> ignore
createDir()

I don't see any reason to have createDir or pathExists as separate functions, since they're short and not reusable.

And as Daniel mentioned in a comment, the check is not actually necessary. So, I would simplify this code to just:

Directory.CreateDirectory(serverDirPath) |> ignore

// check if row exist
let rowCount =
    query{
        for row in db.Uploads do
        select row
        count
    }

// get file number
let fileNumber =
    if rowCount < 1
        then
            1
        else
            query{
                for row in db.Uploads do
                select (row.FileNumber + 1)
                head
            }

You can use headOrDefault here, to get rid of the first query:

// get file number
let fileNumber =
    query {
        for row in db.Uploads do
        select row.FileNumber
        headOrDefault
    } + 1

Also, you're not using any ordering in the query, are you sure the largest FileNumber is always going to be the first one?

let finalServPath =
    serverDirPath + @"\" + fileNumber.ToString() + extension

You can use Path.Combine() to concatenate parts of a path:

let finalServPath =
    Path.Combine(serverDirPath, fileNumber.ToString() + extension)


Implement bash auto completion in Python I created a script supposed to use python to autocomplete Python commands. The full code is available here, however, I can give you an example. For instance, let's consider:

command, the command I want to give completion to (it does not have to actually exist).
script.py, the auto completion script.

import os, shlex

def complete(current_word, full_line):
    split = shlex.split(full_line)
    prefixes = ("test", "test-中文", 'test-한글')
    items = list(i + "-level-" + current_word for i in prefixes)
    word = "" if len(split) - 1 < int(current_word) else split[int(current_word)]
    return list(i for i in items if i.startswith(word))

if __name__ == "__main__":
    os.sys.stdout.write(" ".join(complete(
            *os.sys.argv.__getitem__(slice(2, None)))))
    os.sys.stdout.flush()

The following script can be executed directly into the shell, or added in the .bashrc to keep the changes persistent.

__complete_command() {
    COMPREPLY=($(python3 script.py complete $COMP_CWORD "${COMP_LINE}"));
};
complete -F __complete_command command

Tested on Ubuntu 18.04 with python 3.8.
If you want, you can type the following in a new console

cd $(mktemp -d)
__complete_command() {
    COMPREPLY=($(python3 script.py complete $COMP_CWORD "${COMP_LINE}"));
};
complete -F __complete_command command
echo '
import os, shlex

def complete(current_word, full_line):
    split = shlex.split(full_line)
    prefixes = ("test", "test-中文", "test-한글")
    items = list(i + "-level-" + current_word for i in prefixes)
    word = "" if len(split) - 1 < int(current_word) else split[int(current_word)]
    return list(i for i in items if i.startswith(word))

if __name__ == "__main__":
    os.sys.stdout.write(" ".join(complete(
            *os.sys.argv.__getitem__(slice(2, None)))))
    os.sys.stdout.flush()' > script.py

Is it the most efficient way to do this?

Splitting words

The Python script splits the shell command's full line to words using shlex.
I see a few issues with this:

I'm not sure this will split the line exactly the same way as the shell would. Looking at help(shlex), I see "A lexical analyzer class for simple shell-like syntaxes", and I find that not very reassuring.
I think command line completion should be blazingly fast, so I look suspiciously at anything that needs to be import-ed, such as shlex.
Looking at the Programmable Completion section in man bash, it seems that Bash populates the COMP_WORDS array with the result of the split.

Therefore, it would be good to pass COMP_WORDS to the Python script,
which would eliminate all the above concerns.

One way to achieve this would be to call the Python script with:

python3 script.py complete "$COMP_CWORD" "${COMP_WORDS[@]}"

And then change the Python script accordingly:

#!/usr/bin/env python3

import sys

def complete(comp_cword, *comp_words):
    prefixes = ("test", "test-中文", "test-한글")
    word = comp_words[int(comp_cword)]
    items = (prefix + "-level-" + comp_cword for prefix in prefixes)
    return (item for item in items if item.startswith(word))

if __name__ == "__main__":
    sys.stdout.write(" ".join(complete(*sys.argv[2:])))
    sys.stdout.flush()

Avoid converting generators to list when not needed

No lists were needed in the original script, everything could have been just generator expressions.

Keep it simple

I don't understand why the script imports os and uses os.sys and nothing else in os.
You could just import sys instead.

I don't understand why sys.argv.__getitem__(slice(2, None)) was used instead of the simple and natural sys.argv[2:].

This line is complex, it takes attention to understand:

word = "" if len(split) - 1 < int(current_word) else split[int(current_word)]

This is a lot easier to understand:

if len(split) - 1 < int(current_word):
    word = ""
else:
    word = split[int(current_word)]

Looking further, word is used only in a filter .startswith(word).
That filter will match every string.
In which case, to maximize performance,
it would be best to not create word, and not do any filtering,
but return items directly:

if len(split) - 1 < int(current_word):
    return items

On even closer look,
I don't see how COMP_CWORD can ever be an index out of range.
So the check for bounds was unnecessary.
(Strictly speaking, an index out of bounds may be possible when splitting words with shlex, since that might not be identical to the shell's own word splitting. Even then, it would be a highly unlikely case, therefore a more Pythonic way to handle the situation would be using a try-expect for a IndexError.)

Use better names

The name i is really best reserved for loop counters.
(Even then, often you may find better names...)

In the suggested solution above I renamed the parameter names to match the shell variables they come from.
I find this reduces the cognitive burden when reading the documentation of the variables in man bash, and the implementation of the completion code in Python.


Infix to postfix conversion I have written a C++ program to convert an infix expression to postfix expression using recursion. I would like to know if it can be improved if possible. Can we improve it by not using a stack? I am using a vector<char> as a stack here.

#include <iostream>
#include <vector>
std::string inp;
int i, len;

std::vector<char> ops;
void convert()
{
    char ch = inp[i];
    if (i > len)
        return;
    else if (ch == '(')
    {
        i++;
        convert();
    }
    else if (ch == ')')
    {
        std::cout << ops[ops.size()-1];
        ops.pop_back();
        i++;
        convert();
    }
    else if (isdigit(ch))
    {
        i++;
        std::cout << ch;
        convert();
    }
    else if ((ch == '+') || (ch == '*'))
    {
        i++;
        ops.push_back(ch);
        convert();
    }
}

int main()
{
    std::cout << "Infix to postfix conversion using recursion" << std::endl;
    i = 0;
    inp = "(5+5)";
    len = inp.length();
    convert();
    std::cout << "\n";
    ops.clear(); i = 0;
    inp = "((5+5)*(6+6))";
    len = inp.length();
    convert();
    std::cout << "\n";
    ops.clear(); i = 0;
    inp = "((4+8)*((5+5)*(6+6)))";
    len = inp.length();
    convert();
    std::cout << "\n";
    return 0;
}

Don't use global variables

All your function parameters are global variables that introduce hard to track side effects.

If you want to use the function in multiple threads in parallel it will fail.

Just pass parameters and the code becomes much more readable as well, because you clearly see which variables are needed for the function to execute.

An interface that I'd like to have would be:

std::string convertInfixExpressionToPostfix(std::string infixExpression);

All the user of this function has to care about is to give the required input and retrieve the result.

This would make your calling code much more readable as well:

std::cout << convertInfixExpressionToPostfix("((5+5)*(6+6))") << "\n";

Not returning the result but printing it out is another such side effect which also hinders reuse. Instead of printing the result you would accumulate it in a string and return it (as given in my proposed interface).

Of course this interface does not fit your internal needs for the recursion so you would need another implementation function which does the actual calculation and has some more parameters.

Use specialized code were advisable

You state yourself that you use the std::vector as a stack. Why not use the std::stack, which has a much clearer interface for that.

Readability

Don't put two commands on one line like this:

ops.clear(); i = 0;

It makes it too easy to overlook one of them.

Know when to use switches

Your cascading ifs would be easier to handle with a switch:

switch(ch) {
case '(': /*...*/ break;
case ')': /*...*/ break;
case '0':
case '1':
case '2':
case '3':
case '4':
case '5':
case '6':
case '7':
case '8':
case '9': /*...*/ break;
case '+':
case '-': /*...*/ break;
}

Eventually you would retain the if(isDigit(ch)) and leave out the digit cases.

Unnecessary variable

Your len variable just duplicates the length of the inp string. The length() method has constant time complexity so don't be afraid to use it.


To construct a pyramid with uses of alphabets. Would you propose any better solution? Expected Input:
Enter the number of rows: 5
Expected Output:
        A
      A B A
    A B C B A
  A B C D C B A
A B C D E D C B A

I have solved this problem in this way->
//Write a program to build a pyramid with uses of alphabets
#include <stdio.h>
int
main(void){

//Put variables here
int space,num_of_rows,p=1,t=0;
char alphabet[100]={'A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z'};

printf("Enter the number of rows : ");
//Getting the input from the user
scanf("%d",&num_of_rows);

//Handling total row issues
for(int i=0; i<num_of_rows; i++){

//Handling space issues
for(space=0; space<num_of_rows-i; space++)
    printf(" ");

    //Handling alphabets
    for(int j=1; j<=p; j++){

            //To construct the first row and first position of each of the row as A
            if(i==0 || j==1){

                    alphabet[t]='A';

            }
            //To construct the last position of each of the row as A
            else if(j==p){

                    alphabet[t]='A';

            }
            //Handling the middle position issues
            else if(i+1>=j){

                    alphabet[t]++;

                }

            else{
                    alphabet[t]--;

                }
            printf("%c",alphabet[t]);

    }
    printf("\n");
    p+=2;

   }
 }

Now what I want to accomplish from you: How can I simplify my solution?. Will I face any problem for some conditions? Is it possible to solve this problem without using an array? Then what will be the strategy?. Thank you in advance.

General Observations
The code works as long as the user puts in correct limited values.
The indentation is a little off.
You can decrease the vertical spacing quite a bit, most of the blank lines aren't necessary.
Answers

Will I face any problem for some conditions?

If a user enters a negative number or a number greater than 100 this program will have problems with indexing. Always check user input for correctness and possible error conditions. The program will actually have problems if a user enters a number greater than 52 because there are only 26 letters in the alphabet.

How can I simplify my solution?.

You could break the program up into functions, it's not quite necessary with a program this simple, but it is getting close. Two obviously separate functions that could be implemented right now are get_and_validate_user_input() and print_pyramid(). While functions might seem to make the program more complex and longer each function would be a simpler implementation and building block of the overall implementation.

Is it possible to solve this problem without using an array? Then what will be the strategy?.

Yes. It would be possible to treat the numbers as a value in a range between 'A' and 'Z'.
One Declaration Per Line
This line in the code could cause problems in maintenance in a larger program
int space,num_of_rows,p=1,t=0;

Break it up into 3 lines and always initialize variables in C.
int space = 0;
int num_of_rows = 0;
int p = 1;
int t = 0;


URL-safe pseudorandom string generator in C# public static class SimpleToken
{
    const string TOKENALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-.";
    static string NewToken(int length = 16)
    {
        var rnd = new RNGCryptoServiceProvider();
        var tokenBytes = new byte[length];
        rnd.GetBytes(tokenBytes);
        var token =
            Enumerable
                .Range(0, length)
                .Select(i => TOKENALPHABET[tokenBytes[i] % TOKENALPHABET.Length])
                .ToArray();

        return new String(token);
    }
}

I needed a quick and dirty(?) way to generate long urls for onetime use. It's a simple login-scheme where a user enters his e-mail and gets a one-time URL for logging in. The URL is discarded after one use.

I.e. http://example.com/tokenlogin/3cuzLkh8GcANjqnWcijEeJIHphHx6ZDwfj-2XTR4bfkkqmzmmFYAY2tWsZWST1.5

const string TOKENALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-.";

I learnt the hard way that the problem with using . in a URL token sent by e-mail is that certain mail clients (Outlook in particular) will attempt to auto-detect URLs in a plain text email, but will exclude a trailing . from the inferred URL, so when your user clicks on the auto-generated link they send an invalid token. I suggest that you change . to _.

    static string NewToken(int length = 16)

Length in what units? Generally with cryptographic stuff it's clearer to explicitly use bits as the unit of length.

        var rnd = new RNGCryptoServiceProvider();

As already noted in comments, this is IDisposable and the standard using pattern is preferred.

        var token =
            Enumerable
                .Range(0, length)
                .Select(i => TOKENALPHABET[tokenBytes[i] % TOKENALPHABET.Length])
                .ToArray();

Firstly, this could be simplified to

        var token =
            tokenBytes
                .Select(b => TOKENALPHABET[b % TOKENALPHABET.Length])
                .ToArray();

But secondly, by only using 6 bits per byte you're throwing away 25% of the entropy which the system just produced for you. On busy servers, cryptographic-grade entropy is a valuable resource and you should only request as much as you need.


Function to check if a date is correct I have a method that checks if a date is correct or not, which returns a boolean if it is or not. In the method, I first check the format, then the date by using the function checkdate, then I check that time is of the format His.

/**
  * Function that checks the format, date, and time of a given timestamp.
  * @author : somar
  * @param string $date timestamp ex: 20161230154523
  * @return boolean resultat de check
  */
public function validateDate($date)
{
    if (preg_match("/^(((\d{4}((0[13578]|1[02])(0[1-9]|[12]\d|3[01])|(0[13456789]|1[012])(0[1-9]|[12]\d|30)|02(0[1-9]|1\d|2[0-8])))|((\d{2}[02468][048]|\d{2}[13579][26]))0229)){0,8}$/", substr($date, 0, 8))) {
        $formatCheck = true ;
    } else {
        $formatCheck = false ;
    }
    $dateCheck = checkdate(substr($date, 4, 2), substr($date, 6, 2), substr($date, 0, 4));
    $timeCheck = substr($date, 8, 2) <= 23 && substr($date, 10, 2) <= 59 && substr($date, 12, 2) <= 59;
    if ($dateCheck == true && $timeCheck == true && $formatCheck == true) {
        return true;
    } else {
        return false;
    }
}

I want to improve the performance, by reducing the amount of function calls and by using less checking instructions. Moreover, PHP prefers to not use regex, and I don't know how a workaround.

The reason PHP dislikes the use of Regex is because some people don't know how to use it.
You, for example, don't.
Regex is for matching input, not for validation.
And so you'd use it to change say your date format to ISO 8601:

> $date = '19700101000000';
> preg_replace('/(\d{4})(\d{2})(\d{2})(\d{2})(\d{2})(\d{2})/', '$1-$2-$3T$4:$5:$6Z', $date);
=> "1970-01-01T00:00:00Z"

The regex is simple as we know the data should be there. Not that it's correct.

Your code is also long-winded, rather than manually checking if the code is alright make PHP do it for you with strptime.
You know the format the date is in, %Y%m%d%H%M%S, and so you can use it:

> strptime($date, '%Y%m%d%H%M%S');
=> {"tm_sec":0,"tm_min":0,"tm_hour":0,"tm_mday":1,"tm_mon":0,"tm_year":70,"tm_wday":4,"tm_yday":0,"unparsed":""}

And so you could change the function to just use:

/**
  * Function that checks the format, date, and time of a given timestamp.
  * @author : somar
  * @param string $date timestamp ex: 20161230154523
  * @return boolean resultat de check
  */
public function validateDate($date)
{
    return (bool)strptime($date, '%Y%m%d%H%M%S');
}

To note, strptime seems to have a bug with %S:

> strptime('19700101000070', '%Y%m%d%H%M%S');
=> {"tm_sec":7,"tm_min":0,"tm_hour":0,"tm_mday":1,"tm_mon":0,"tm_year":70,"tm_wday":4,"tm_yday":0,"unparsed":"0"}

To fix this you could change the function to take ISO 8601 dates, and have a function that converts to this format.

/**
  * Function that checks the format, date, and time of a given ISO 8601 timestamp.
  * @author : somar
  * @param string $date timestamp ex: 2016-12-30T15:45:23Z
  * @return boolean resultat de check
  */
public function validateDate($date)
{
    return (bool)strptime($date, '%Y-%m-%dT%H:%M:%SZ');
}

public function compactDateToISO($date)
{
    return preg_replace('/(\d{4})(\d{2})(\d{2})(\d{2})(\d{2})(\d{2})/', '$1-$2-$3T$4:$5:$6Z', $date);
}


Calculator with history I'm the newbie in Java (and programming) and I've tried to make a simple calculator. Code is barbaric, I know, so I would like to make it more beatiful.

Main.java:

import java.util.Scanner;
import java.util.Vector;

public class Main {
    static Main mainobject = new Main();
    ActionsToDo act = new ActionsToDo();
    Solution solve = new Solution();
    public static Vector<Double> v1 = new Vector<Double>();

    public void setValues(double fn, double sn) {
        solve.solveExpression(chooseOperator(), fn, sn);
        act.chooseAction(fn, sn);
    }

    public static double getFirstNumber() {
        System.out.println("Input the 1st number");
        Scanner sc = new Scanner(System.in);
        return sc.nextDouble();
    }

    public static double getSecondNumber() {
        System.out.println("Input the 2nd number");
        Scanner sc = new Scanner(System.in);
        return sc.nextDouble();
    }

    public char chooseOperator() {
        Scanner sc = new Scanner(System.in);
        System.out.println("What to do?" +
                "\n + for add" +
                "\n - for minus" +
                "\n * for multiply" +
                "\n / for divide" +
                "\n % for mod" +
                "\n ^ for first number into the power of second number");
        String operator = sc.next();
        return operator.charAt(0);
    }

    public static void main(String[] args) {
        mainobject.setValues(getFirstNumber(), getSecondNumber());
    }
}

Solution.java

public class Solution {
    static Main objectMain2 = new Main();
    public void solveExpression (char checker, double fn, double sn) {
        switch (checker) {
            case '+':
                System.out.println((fn + sn));
                objectMain2.v1.addElement((fn + sn));
                break;
            case '-':
                System.out.println((fn - sn));
                objectMain2.v1.addElement((fn - sn));
                break;
            case '*':
                System.out.println((fn * sn));
                objectMain2.v1.addElement((fn * sn));
                break;
            case '/':
                System.out.println((fn / sn));
                objectMain2.v1.addElement((fn / sn));
                break;
            case '%':
                System.out.println((fn % sn));
                objectMain2.v1.addElement((fn % sn));
                break;
            case '^':
                System.out.println(Math.pow(fn, sn));
                objectMain2.v1.addElement(Math.pow(fn,sn));
                break;
        }
    }
}

ActionsToDo.java:

import java.util.Scanner;
public class ActionsToDo {
    static Main objectMain = new Main();
    public void chooseAction(double fn, double sn) {
        Scanner sc = new Scanner(System.in);
        System.out.println("Dormammu, I came to bargain! " +
                "Wanna do some extra calculations?" +
                "\n 1 - for 'Yes'" +
                "\n 2 - for 'No'" +
                "\n 3 - to change the numbers");
        int dormammu = sc.nextInt();
        switch (dormammu) {
            case 1:
                objectMain.setValues(fn, sn);
                break;
            case 2:
                System.out.println("Okay! Nobody misses you" +
                        "\n But here's the calculations you've done so far");
                for (int i = 0; i < objectMain.v1.size();i++) {
                    System.out.print(objectMain.v1.get(i) + " ");
                }
                break;
            case 3:
                objectMain.setValues(objectMain.getFirstNumber(), objectMain.getSecondNumber());
                break;
            default:
                System.out.println("Shit, wrong answer,  you'll have to calculate " +
                        "with these numbers again and then you can do whatever you want");
                objectMain.setValues(fn, sn);

        }
    }
}

Note that the different classes are used just for training of using them

Glad you're taking on Java!

You should study Object Oriented Programming (OOP), which is core in Java, a little bit more to use classes in a more suitable and efficient way.

The general rule of a class is "one class one purpose" which it looks like you're trying to apply, but since you don't have any objects, it becomes a bit arbitrary.

An example would be to have a class called Calcualtor that has functions for what a calculator could do, then you could have another class called InputHandler that handles the reading from the user interface (in this case that's the terminal). It's very bad to call the Main class in all classes, it's very backwards, main is supposed to handle the other classes and there's only one Main.

Because of the structure you have here you notice that you find yourself using static a lot, we call this "static overuse". If you apply OOP properly you won't need static unless there's an intended use, like a variable that never changes and is the same for all objects of that type.

You should be fine by just rearranging your methods into objects and call them from main:

public class Main {
    public static void main(String[] args) {
        InputHandler inputHandler = new InputHandler();
        inputHandler.start();
    }
}

Input Handler:

public class InputHandler {
    /* Notice how I put the calculator and scanner up in the
       object-scope, you can now re-use them throughout this object.
    */
    private Calculator calculator = new Calculator();
    private Scanner sc = new Scanner(System.in);
    private boolean keepOn = true;

    public void start() {
        System.out.println("Welcome to my calculator!");
        performAction(1);  // This is what we call a magic number. Don't do this, instead you could make variables for the options, or, you could make an enum.
        while(keepOn == true) {
            calculate();
        }
    }

    public void calculate() {
       int action = selectAction();
       performAction(action);
    }

    public double getFirstNumber() {
        System.out.println("Input the 1st number");
        return sc.nextDouble();
    }

    public double getSecondNumber() {
        System.out.println("Input the 2nd number");
        return sc.nextDouble();
    }

    public char chooseOperator() {
        System.out.println("What to do?" +
                "\n + for add" +
                "\n - for minus" +
                "\n * for multiply" +
                "\n / for divide" +
                "\n % for mod" +
                "\n ^ for first number into the power of second number");

        String operator = sc.next();
        return operator.charAt(0);
    }

    public int selectAction() {
        System.out.println("Dormammu, I came to bargain! " +
                "Wanna do some extra calculations?" +
                "\n 1 - for 'Yes'" +
                "\n 2 - for 'No'");
        int dormammu = sc.nextInt();
        return dormammu;
    }

    public double performCalculation() {
        char operand = chooseOperator();
        double firstNumber = getFirstNumber();
        double secondNumber = getSecondNumber();
        double result = calculator.calculate(firstNumber, secondNumber, operand);
        System.out.println("The result of " + firstNumber + operand + secondNumber + "is: " + result);
        return result;
    }

    public void performAction(int action) {
        switch (action) {
            case 1:
                performCalculation();
                break;
            case 2:
                System.out.println("Okay! Nobody misses you" +
                        "\n But here's the calculations you've done so far");
                for (int i = 0; i < calculator.calculations.size(); i++) {
                    System.out.println(calculator.calculations.get(i));
                }
                keepOn = false;
                break;
            default:
                System.out.println("Shit, wrong answer, try again!");
                calculate();
        }
    }
}

And the calculator:

public class Calculator {
    public Vector<Double> calculations = new Vector<>();

    public double calculate(double firstNumber, double secondNumber, char operand) {
        switch (operand) {
            case '+':
                calculations.addElement((firstNumber + secondNumber));
                return firstNumber + secondNumber;
            case '-':
                calculations.addElement((firstNumber - secondNumber));
                return firstNumber - secondNumber;
            case '*':
                calculations.addElement((firstNumber * secondNumber));
                return firstNumber * secondNumber;
            case '/':
                calculations.addElement((firstNumber / secondNumber));
                return firstNumber / secondNumber;
            case '%':
                calculations.addElement((firstNumber % secondNumber));
                return firstNumber % secondNumber;
            case '^':
                calculations.addElement(Math.pow(firstNumber, secondNumber));
                return Math.pow(firstNumber, secondNumber);
            default:
                System.out.println("Oops! Received an operand that's not supported! How'd that happen?");
                throw new UnsupportedOperationException();
        }
    }
}

This is just a small refactoring of your code. You should always treat user input with great care and use try-catch to make sure the input is what you expect. Your application will crash if you type no instead of 2.

Also notice how I used some recursion in performAction(int action) where if you do not get the expected answer it re-attempts to do the calculation. This however only works if the other answer is a digit. I'd recommend you try to read a String and then try to convert it to an int using Integer.valueOf("1") and catch errors if it's not a number.


Converting raw thesaurus data into nice JSON file I'm trying to improve my dirty data wrangling code into something nice, readable & reliable.

I believe I've done fairly well here, although maybe I should have some docstrings on my functions. Still, I believe it's fairly readable without them.

I'd love some feedback on ways to improve! This script parses raw data from Gray Ward's fantastic public-domain thesaurus into a more readable JSON file.

import sys
import os
import json

# Parses Grady Ward's public-domain thesaurus, available at
# ftp://svr-ftp.eng.cam.ac.uk/pub/comp.speech/dictionaries/moby/

def main():
    database = {}

    if len(sys.argv) < 3:
        print("Usage: make_database <input filename> <ouput filename>")
        sys.exit(1)

    filename = sys.argv[1]

    if not os.path.exists(filename):
        print(f"File {filename} doesn't exist.")
        sys.exit(1)

    # Read file into database
    with open(filename, "r") as file:
        try:
            parse_file(file, database)
            print("Finished reading the file.")
        except Exception as err:
            print(f"Failed to read file {filename} with error {err}")
            sys.exit(1)

    # Write database to output file as JSON
    output_filename = sys.argv[2]
    with open(output_filename, "w+") as file:
        try:
            data = json.dumps(database)
            file.write(data)
            print(f"Finished writing to file {output_filename}")
        except Exception as err:
            print(f"Failed to read write to file {output_filename} with error {err}")
            sys.exit(1)

def parse_file(file, database):
    for line in file:
        (word, thesaurus) = parse_line(line)
        database[word] = thesaurus

def parse_line(line):
    words = line.strip().split(",")
    return (words[0], words[1:])

if __name__ == '__main__':
    main()

I'm still a student -- I'm trying to improve my code readability & maintainability.

It is a commandline utility that can be called like:

make_database <input filename> <output filename>

Welcome to Code Review!

The current code looks clean enough. Since it will be a command line utility, I'd suggest taking up the argparse or click. argparse is a python inbuilt module, so your end package will not have a dependency other than the dictionary files.

The json package provide the method json.dump which can write directly to a file. No need to have extra memory consumption.

Instead of using print statements, I'd go with logging builtin module with possibly multiple handlers, if you want to stream logs to console or file etc.


Counting occurrences of "hi" in a string, except where followed by "x" I already solved this one from CodingBat, but I'm unsatisfied with my solution and I feel I could have made it much shorter. Most of the other recursion tasks could be solved by a one line conditional for the base case and a one line return statement usually using ternary operator. So, could I have made this any shorter or more readable?

  Given a string, compute recursively the number of times lowercase "hi" appears in the string, however do not count "hi" that have an 'x' immedately before them.

public int countHi2(String str) {
  if (str.length() <= 1) {
    return 0;
  }

  if (str.startsWith("x") && str.charAt(1) != 'x') {
    return countHi2(str.substring(2));
  }
  else if (str.startsWith("hi")) {
    return 1 + countHi2(str.substring(2));
    }
  else {
    return countHi2(str.substring(1));
  }
}

It doesn't get much shorter than that. But some other improvements are possible.

The current algorithm uses startswith method to check for x and hi, and as such it advances by 1 or 2 characters at a time. This is inefficient.

Another performance issue is creating many temporary strings in the process (because of substring).

It would be better to use indexOf instead of startswith, which will allow you to jump multiple characters at a time. Another big improvement would be to avoid temporary string generation, by creating a helper function that tracks the position to check. Something like this:

public int countHi2(String str) {
    return countHi2(str, 0);
}

public int countHi2(String str, int start) {
    start = str.indexOf("hi", start);
    if (start == -1) {
        return 0;
    }

    int count = 0;
    if (start == 0 || str.charAt(start - 1) != 'x') {
        count++;
    }
    return count + countHi2(str, start + 2);
}


Determine the complexity of sorting a binary array The challenge is to sort an integer array containing only ones and zeros with the lowest complexity possible.
As always, I am interested in any improvements suggested but in this question I am most interested in whether the complexity that I have assigned to the algorithm is correct (as well as the lowest possible). I have included two implementations that have two different running times. I believe that they are still both the same complexity. The larger purpose in posting this is to improve or verify my understanding of Complexity and big oh notation.
I assign the complexity as O(n) to this first algorithm since it only looks at every value in the array once, and only once.
Algorithm I
    /// <param name="value">A list of values to sort where all values are either one or zero.</param>
    /// <returns>Sorted value array.</returns>
    public static int[] SortOnesZeros(int[] value)
    {
        int j = value.Length - 1;
        int i = 0;
        while (i < j)
        {
            if (value[i] < 0 || value[i] > 1 || value[j] < 0 || value[j] > 1)
            {
                throw new Exception("Value array can only contain 1's and 0's.");
            }
            while (value[i] == 0)
            {
                i++;
            }
            while (value[j]== 1)
            {
                j--;
            }
            if (j > i && value[i] > value[j] )
            {
                int temp = value[j];
                value[j] = value[i];
                value[i] = temp;
                j--;
                i++;
            }
        }
        return value;
    }

I have a second implementation that I believe is also O(n) though it is longer since it runs through n twice, but it is not nested. I believe this is true because O(2n) reduces to O(n) since multiplied constants are not considered in big-oh complexity analysis. It is also longer because it changes every value on the second pass. No need to improve on this algorithm, I believe that the first algorithm is already better.:
Algorithm II
    public static int[] SortOnesZerosAlternate(int[] values)
    {
        int countOfZeros = 0;
        foreach(int value in values)
        {
            if (value == 0)
            {
                countOfZeros++;
            }
            if (value <0 || value > 1)
            {
                throw new Exception("Value array can only contain 1's and 0's.");
            }
        }
        for (int i = 0; i < countOfZeros; i++)
        {
            values[i] = 0;
        }
        for (int i = countOfZeros; i < values.Length; i++)
        {
            values[i] = 1;
        }

        return values;
    }

EDIT ***************************************
@superb rain is correct: The first algorithm has a mistake where, under the right conditions, it tries to sort values in the value array that are not actually on the list. This issue will occur on any list where all the values are the same. I have fixed this in my own code by making the change below:
            while (i < j && value[i] == 0)
            {
                i++;
            }
            while (i < j && value[j]== 1)
            {
                j--;
            }

I agree with your analysis that both algorithms have O(n) time complexity (where n is the number of elements in the array). An important factor to note is that both algorithms also use constant extra space.
However, you can include other metrics in your analysis too. Anything that may be expensive  to do can be considered. Two common metrics are the number of comparisons and the number of exchanges.
The number of comparisons tells you how often an element of the array is accessed and then compared. Memory access can be quite expensive, depending on how far up the memory hierarchy (registers, cache levels, RAM, local storage, external storage) you need to traverse before getting the value.
The number of exchanges tells you how many times elements are swapped. This can be expensive any time writing to memory is. For instance, writing to shared memory in a multi-process program.

Taking the above into account, ignoring the error checking case, and also ignoring any potential optimisations from the compiler, I would argue the second algorithm is better. For the first implementation I count 2 comparisons per element, and 1 exchange per element. For the second, I count 1 comparison per element, and then 1 assignment per element (an assignment of a constant is cheaper than exchanging two elements).
You can replace the exchange with assignments in the first implementation. Since the array only contains the values 0 and 1, and you've checked that the two elements are different.
if (j > i && value[i] > value[j] )
{
    value[j] = 1;
    value[i] = 0;
    j--;
    i++;
}

Without profiling the code, I don't think it is possible to tell if the extra number of comparisons in the first implementation are more costly than the overhead of the second pass through the array in the second implementation. I would not be surprised if the numerous bounds checks in the first piece of code (i < j) are the actually largest performance hit.

Third implementation
Based on the comment that the first implementation is still faster, here is another solution that tries to improve upon the second. I'm hoping that by writing it with well known operations, the compiler can work some magic.
public static int[] SortOnesZerosAlternate(int[] values)
{
    int countOfZeros = values.Length - values.sum();
    // Maybe replace this with values.Clear?
    for (int i = 0; i < countOfZeros; i++)
    {
        values[i] = 0;
    }

    for (int i = countOfZeros; i < values.Length; i++)
    {
        values[i] = 1;
    }

    return values;
}


Normalizing strings using PHP preg_replace and regex expressions I needed a way to normalize nearly any given input for a configuration parameter on a dashboard.  Server side code is in PHP so I wrote a test script to experiment with preg_replace and regex.  When I say normalize meaning, the resulting value must match the following restrictions:

Take any cased value and return all lower case. For example, given "ValiDVoicE", after normalizing, "validvoice" would be returned.
Strip away all special characters and white space except underscores. For example, given "@@Valid  _voice" return only "valid_voice".
Trim all nonessential white space from front and end of string. For example, given "   (&&   (*Paxus-Demo    " return "paxus_demo".

Solution: (I copied my test script below)

$myvar =
[
    "Paxus-Demo",
    "Paxus Demo",
    "paxus_demo",
    "Paxus  Demo",
    "paxus-Demo",
    "paxus_ Demo",
    "Paxus _Demo",
    "*(&*&   (*Paxus-Demo   ",
    "@@Valid  _voice",
    "Valid-Voice",
    "PortaL-Demo",
    "gui_demo",
    "Gui-Demo   ",
    "VoiceInstance",
    "vAlid Voice",
    "  vaLid_  _voiCe  ",
];

for( $i = 0; $i < sizeof( $myvar ); ++$i )
{
    $modded = trim( strtolower( preg_replace( array( '/[^a-zA-Z0-9 ]/i', '<\W+>'), array( ' ', '_' ), $myvar[$i] ) ), '_' );
    echo "modded2 = " . $modded." [ ". $i ." ] = " .$myvar[$i]. "<br />";
}

My question is not necessarily for elegance, but could I have done this all in one regex expression using lookarounds?  Bear in mind, I never took the time to truly understand using regex before today so my knowledge of using lookarounds is still a bit wonky.  That said, if someone can simplify the use of lookarounds it'll be greatly appreciated.

At first glance I would first trim without a mask and then stringtolower inside before the preg_replace.

$modded = preg_replace(array( '/[^a-z0-9 ]/','<\W+>'),array( ' ', '_' ), strtolower(trim( $myvar[$i])));

trim first, less characters to process when there is white space to
trim.
Then stringtolower() which allows '/[^a-zA-Z0-9 ]/i' to be
simplified to '/[^a-z0-9 ]/'

Case insensitive tasks require extra CPU cycles.
RegEx is the usually least efficient string function especially compared to stringtolower() and trim()
Using these two function before the RegEx gives the RegEx less work to do.
Consider giving the trim additional mask characters: e.g.
space, new line, carriage return, asterisk, ampersand, at mark, parenthesisPlus any other characters that could need to be trimmed (quote marks?).

This worked well for you test $myvar:

trim($str," \t\n\r\0\x0B\x28\x29\x26\x2a\x40\x5f")

RegEx is not my strong suit.  While it works well this does not make sense to me. '/[^a-z0-9 ]/','<\W+>' I'm not saying it is wrong, it's not broke,  I just do not fully understand.

some minor stuff that bugs me:
You do not need to concatenate simple variables in double quotes

echo "modded2 = " . $modded." [ ". $i ." ] = " .$myvar[$i]. "\n";

This is the same but cleaner:

echo "modded2 = $modded [$i] = $myvar[$i]\n";

This may be more efficient, (or may not, did not benchmark):

for( $i = 0; $i < sizeof( $myvar ); ++$i )

But this is cleaner, easier to see what is going on.

foreach($myvar as $key => $value){

My preference is count over sizeof, although they are the same.

In a test environment like this, instead of <br> I use "\n" with a plain text header:

header('Content-Type: text/plain; charset=utf-8');

It works will with print_r() and var_export(), var_dump() etc.

My Tested Code:

header('Content-Type: text/plain; charset=utf-8');
$myvar = array("Paxus-Demo","Paxus Demo","paxus_demo","Paxus  Demo",
    "paxus-Demo","paxus_ Demo","Paxus _Demo","*(&*&   (*Paxus-Demo   ",
    "@@Valid  _voice","Valid-Voice","PortaL-Demo","gui_demo",
    "Gui-Demo   ",    "VoiceInstance","vAlid Voice","  vaLid_  _voiCe  ");

foreach($myvar as $k => $before){
  $after = preg_replace( array( '/[^a-z0-9 ]/','<\W+>'), array( ' ', '_' ), strtolower(trim($before," \t\n\r\0\x0B\x28\x29\x26\x2a\x40\x5f" )));
//  echo "$k. $before => $after\n";
  echo "$k. $after <= \"$before\"\n";
}

The Result:

0. paxus_demo <= "Paxus-Demo"
1. paxus_demo <= "Paxus Demo"
2. paxus_demo <= "paxus_demo"
3. paxus_demo <= "Paxus  Demo"
4. paxus_demo <= "paxus-Demo"
5. paxus_demo <= "paxus_ Demo"
6. paxus_demo <= "Paxus _Demo"
7. paxus_demo <= "*(&*&   (*Paxus-Demo   "
8. valid_voice <= "@@Valid  _voice"
9. valid_voice <= "Valid-Voice"
10. portal_demo <= "PortaL-Demo"
11. gui_demo <= "gui_demo"
12. gui_demo <= "Gui-Demo   "
13. voiceinstance <= "VoiceInstance"
14. valid_voice <= "vAlid Voice"
15. valid_voice <= "  vaLid_  _voiCe  "


Returning the sum of all multiples of a number up to 1000 I've been learning Python and in order to put my skills to the test, I created a simple program that returns the sum of all of the multiples of number num up to 1000.

def get_multiple_sum(num):
    sum = 0
    i = 0
    for i in range(0, 1001):
        if i % num == 0:
            sum += i
        else:
            continue
    return sum

Is this the fastest way to do this? I'm always looking for ways to make my code more efficient.

range takes an optional third argument, which is the step size for the sequence. For example:

range(0, 10, 3) # Returns [0, 3, 6, 9]

Python also has a built-in sum function that adds up all the numbers in an array, so you could refactor your code to look like this:

def get_multiple_sum(num):
    return sum(range(0, 1000, num))

Performance-wise, this should be much faster, as it doesn't perform 1000 remainder operations and uses a library function which may be optimized by the interpreter.


Cross-Database Query Without Casting to List I am trying to do a cross-database query without having to enumerate the variable t.

public List<Models.MCMessageCenter> GetMsgCtrsWithReloInfoByUserId(Guid userId)
{
    var t = GetMessageCentersByUserId(userId).ToList();

    List<Models.MCMessageCenter> result;

    using (var ctx = new TEDREntities())
    {
        result = (from n in t
                 join m in ctx.UserAccounts.AsNoTracking() on n.UserId equals m.UserAccountId
                 select new Models.MCMessageCenter
                 {
                     Id = n.Id,
                     MessageTitle = n.MessageTitle,
                     CreatedDate = n.CreatedDate,
                     UserId = n.UserId,
                     Name = m.FirstName + " " + m.LastName,
                     RelatedEntityId = n.RelatedEntityId
                 }).ToList();
    }

t could be very large so I don't want to enumerate it by turning it into a list.  Is there a better, more efficient way to perform a cross database query?

Whenever I attempt to keep t as IQueryable, I get an error like "can't query two datasources at once."  I can get the exact message if anyone needs.

You can try to replace the .ToList() by an .AsEnumerable(). AsEnumerable() makes a transition from LINQ-to-EF to LINQ-to-Objects. AsEnumerable() does not enumerate the items (lazy enumeration).


Searching of Word documents I have a website that offers to search documents from local hearings conducted, stored on a network file server.  I need to take in the search term and search a bunch of .docx (roughly 4500) files.  They are not large,  < 150 kb mostly, but it runs very slow downloading the files into the stream. I'm sure there's a better way to write the search, (perhaps multi processing) but I don't know how to tune it up and speed the search up.  The search itself is taking over 3 minutes.

bool found = false;
Hearing h = new Hearing();
Stream str = null;
MemoryStream str2 = new MemoryStream();
HttpWebRequest fileRequest = (HttpWebRequest)WebRequest.Create(url);
HttpWebResponse fileResponse = (HttpWebResponse)fileRequest.GetResponse();
str = fileResponse.GetResponseStream();
str.CopyTo(str2);
str2.Position = 0;
using (WordprocessingDocument wpd = WordprocessingDocument.Open(str2, true))
{
    string docText = null;
    using (StreamReader sr = new StreamReader(wpd.MainDocumentPart.GetStream()))
    {
        docText = sr.ReadToEnd();
        found = docText.ToUpper().Contains(txtBasicSearch.Text.ToUpper());
        if (found)
        {
            hearingArrayList.Add(h);
            foundCount++;
        }
    }
}

This is really the exact use-case for indexed full-text search engines.

Since you're running this code server-side on a website, I'd suggest you seriously consider writing a simple worker that polls your FS for new documents and adds them to a database that has full text searching enabled.

If you're using SQL Server: https://docs.microsoft.com/en-us/sql/relational-databases/search/get-started-with-full-text-search

If you're using MySQL: http://www.w3resource.com/mysql/mysql-full-text-search-functions.php

This way, you'd not only get your results back much more quickly than scanning each document manually, you'd also avoid the onerous network traffic involved in streaming every file from the FS for every request.

To do this, you can pretty easily either write a page in your site or a new console app (preferable) that is called by a cron job (linux) or scheduled task (windows) on the server every so often. That interval would be however often you expect there to be new documents added to the FS or whatever your tolerance is for stale data.

At that point, the page/app would pull the list of documents already cached in the database, query the FS for its contents, and compare the lists of filenames or file dates to see what needs to be added/updated. At that point, you can only stream in the files you actually need to add and you don't really care how long it takes.

The database would then take care of the indexing of the new documents. Your webpage then becomes a dumb pipe for searching those indexed documents.

If storing the texts in a database isn't an option, you might consider mirroring the files on your own server. It'd still remove the slowest part of your algorithm (the network traffic).

You'd still need your cron/scheduled task worker to do that mirroring but it'd be a simple matter of copying the new files from the FS to your local disk.

If you mirror locally or can't do either, your best bet is parallelization. You can do some refactoring but your local operations aren't your real bottleneck.

For instance, if you can mirror locally, you could use this in place of your existing code:

// ToUpper() your search string outside of the loop,
// rather than in each passs.
string txtBasicSearch = "My Search String".ToUpper();

// Use Parallel.ForEach over every docx file in our directory.
Parallel.ForEach(Directory.EnumerateFiles(directoryPath, "*.docx"), (string file) =>
{
    string docText = string.Empty;

    try
    {
        // Try to dispose of our streams as soon as possible to avoid
        // holding memory unecessarily. Also, avoid copying Streams
        // to different types. A generic Stream works just fine.
        //
        // As well, only open with read perms to avoid unecessary locks and
        // any delays that may cause.
        using (Stream str = File.OpenRead(file))
        {
            using (WordprocessingDocument wpd = WordprocessingDocument.Open(str, false))
            {
                using (StreamReader sr = new StreamReader(wpd.MainDocumentPart.GetStream()))
                {
                    docText = sr.ReadToEnd();
                }
            }
        }

        // Search the haystack for the needle.
        if (docText.ToUpper().Contains(txtBasicSearch))
        {
            // No need for a counter variable. Just user
            // hearingArrayList.Count() at the end.
            hearingArrayList.Add(file);
        }
    }
    catch (Exception ex)
    {
        // Do whatever error handling here.
        return;
    }
});

Timing that parallel version against the same version using a regular foreach loop with a small directory on my local NAS showed that the parallel version was typically 3-6 times faster.

If you can't mirror locally, you can still parallelize the file streaming but will need to be cognizant of the limits the server may place on the number of connections you can open at once.

The HttpClient class will probably serve you better here than the WebRequest class. https://msdn.microsoft.com/en-us/library/hh696703(v=vs.110).aspx

There, you can query the files within the remote directory then iterate through them, making an async call with HttpClient.

So, that might look like:

string txtBasicSearch = "My Search String".ToUpper();

HttpClient client = new HttpClient();

// Use client to populate myFileList with the remote files.

foreach (string file in myFileList)
{
    client.GetStreamAsync(file).ContinueWith((Task<Stream> result) =>
    {
        if (result.Status != TaskStatus.RanToCompletion)
        {
            // Error handling.
            return;
        }

        string docText = string.Empty;

        try
        {
            using (WordprocessingDocument wpd = WordprocessingDocument.Open(result.Result, false))
            {
                using (StreamReader sr = new StreamReader(wpd.MainDocumentPart.GetStream()))
                {
                    docText = sr.ReadToEnd();
                }
            }

            if (docText.ToUpper().Contains(txtBasicSearch))
            {
                hearingArrayList.Add(file);
            }
        }
        catch (Exception ex)
        {
            // Do whatever error handling here.
            return;
        }
    }
}

The HttpClient class will take care of rate limiting you. By default, I believe it allows three connections at any time, but you can easily change that to your liking.

Enumerating the files on the remote server would be a different topic, depending on how that remote is being accessed. I'd suggest searching other SO answers like https://stackoverflow.com/questions/124492/c-sharp-httpwebrequest-command-to-get-directory-listing

(If your file server is just a NAS on your intranet, save yourself the pain an just use the System.IO.Directory and .File classes to query the files)


Simple JS validation for two contact forms I'm not a pro in js so, my code is a mess (but it's working!). I would like to ask your help to reduce some redundancies in two cases. Those two are codes are affecting these two forms:

<form action="contact.php" method="post" name="form1" onsubmit="return validateForm1()">
    <input type="text" name="name" id="inputName1" placeholder="Name">
    <input type="text" name="tel" id="inputTel1" placeholder="Telephone Number">
    <input type="text" name="address" id="inputAdr1" placeholder="Address">
    <input type="text" name="topic" id="inputTop1" placeholder="Topic">
    <input type="email" name="email" id="inputMail1" placeholder="E-mail">
    <input type="email" id="inputConfirmMail1" onblur="mailConfirm1()" placeholder="Confirm E-mail">
    <input type="submit" value="Send">
</form>

<form action="contact.php" method="post" name="form2" onsubmit="return validateForm2()">
    <input type="text" name="name" id="inputName2" placeholder="Name">
    <input type="text" name="tel" id="inputTel2" placeholder="Telephone Number">
    <input type="text" name="address" id="inputAdr2" placeholder="Address">
    <input type="text" name="topic" id="inputTop2" placeholder="Topic">
    <input type="email" name="email" id="inputMail2" placeholder="E-mail">
    <input type="email" id="inputConfirmMail2" onblur="mailConfirm2()" placeholder="Confirm E-mail">
    <input type="submit" value="Send">
</form>

FIRST (a function to prevent my two forms to have empty inputs before submission)

function validateForm1() {
    var c = document.forms["form1"]["name"].value;
    var d = document.forms["form1"]["tel"].value;
    var e = document.forms["form1"]["address"].value;
    var f = document.forms["form1"]["topic"].value;
    var g = document.forms["form1"]["email"].value;
    if (c == null || c == "") {
            alert("insert your name");
            return false;
        }
        else if (d == null || d == "") {
            alert("insert your telephone");
            return false;
        }
        else if (e == null || e == "") {
            alert("insert your address");
            return false;
        }
        else if (f == null || f == "") {
            alert("insert a topic");
            return false;
        }
        else if (g == null || g == "") {
            alert("insert your email");
            return false;
        }
    }
    function validateForm2() {
        var c = document.forms["form2"]["name"].value;
        var d = document.forms["form2"]["tel"].value;
        var e = document.forms["form2"]["address"].value;
        var f = document.forms["form2"]["topic"].value;
        var g = document.forms["form2"]["email"].value;
        if (c == null || c == "") {
            alert("insert your name");
            return false;
        }
        else if (d == null || d == "") {
            alert("insert your telephone");
            return false;
        }
        else if (e == null || e == "") {
            alert("insert your address");
            return false;
        }
        else if (f == null || f == "") {
            alert("insert a topic");
            return false;
        }
        else if (g == null || g == "") {
            alert("insert your email");
            return false;
        }
    }

SECOND (I have a "confirm your email" area in my form so I did this function to make sure the user inputs the same value in both areas)

function mailConfirm1() {
    var mail1 = document.getElementById("inputMail1").value;
    var conMail1 = document.getElementById("inputConfirmMail1").value;
    if(mail1 != conMail1) {
        alert('both emails are not the same');
    }
}
function mailConfirm2() {
    var mail2 = document.getElementById("inputMail2").value;
    var conMail2 = document.getElementById("inputConfirmMail2").value;
    if(mail2 != conMail2) {
        alert('both emails are not the same');
    }
}

The simplest way to minimise your code is to change the functions you've written to accept parameters.

HTML:

<form action="contact.php" method="post" name="form1"
    onsubmit="return validateForm(this)">
...
<input type="email" name="conEmail" id="inputConfirmMail1"
    onblur="mailConfirm(this, 'inputMail1')" placeholder="Confirm E-mail">

JS:

function validateForm(form) {
  var c = form.name.value;
  var d = form.tel.value;
  var e = form.address.value;
  var f = form.topic.value;
  var g = form.email.value;

  if (!c) {
     alert("insert your name");
     return false;
  }
  else if (!d) {
     alert("insert your telephone");
     return false;
  }
  else if (!e) {
     alert("insert your address");
     return false;
  }
  else if (!f) {
     alert("insert a topic");
     return false;
  }
  else if (!g) {
     alert("insert your email");
     return false;
  }
}

function mailConfirm(input, target) {
  var mail = document.getElementById(target).value;
  var conMail = this.value;
  if(mail != conMail) {
     alert('both emails are not the same');
  }
}

However this doesn't stop you from submitting the form with both email addresses being different or having never entered the confirmation email.

Also, hand writing out every single validation rule for each individual input would be very time consuming and prone to business logic errors.

I would highly recommend you use an open source validator to assist like Parsley or Bootstrap Validator


Lazy-loading iframes as they scroll into view I'm working in a Tableau driven system, with large numbers of charts, I needed a way to reduce the strain of the initial load of our app. It's fairly simple, but wanted to ensure I accounted for every scenario, I don't generally utilize lazy loading.

Print isn't a priority.

jsfiddle: Demo (Tableau doesn't work within SO's snippets because of the iframes)

Relevant Javascript

const throttle = (fn, delay) => {
    let canCall = true;
    return (...args) => {
      if (canCall) {
        fn.apply(null, args);
        canCall = false;
        setTimeout(() => {
          canCall = true;
        }, delay);
      }
    };
  };

  const setAttributes = (el, options) =>
    Object.keys(options).forEach(attr => el.setAttribute(attr, options[attr]));

  const w = window;
  const d = document;
  const b = d.body;
  const x = w.innerWidth || e.clientWidth || b.clientWidth;
  const y = w.innerHeight || e.clientHeight || b.clientHeight;

  /* unecessary, but to have same object format, e.g. readability */
  const win = Object.create(null);
  win.y = y;

  const frame = d.querySelectorAll("iframe");

  /** may be over doing it here... test results */
  const preloadPath = () =>
    frame.forEach(item => {
      const source = item.getAttribute("data-src");
      const preloadLink = d.createElement("link");
      const head = d.getElementsByTagName("head")[0];

      setAttributes(preloadLink, {
        rel: "preload",
        as: "document",
        type: "text/html",
        crossorigin: "anonymous",
        href: source
      });

      head.insertBefore(preloadLink, head.firstChild);
    });

  const loadFrame = () =>
    frame.forEach(item => {
      /* only run if not src is present */
      if (!item.src) {
        const source = item.getAttribute("data-src"); // path
        const parent = item.parentElement; // wrapper
        const frameRect = parent.getBoundingClientRect(); // wrapper dimensions

        if (
          w.pageYOffset + win.y >= frameRect.y && // if frame top exceeds page bottom
          frameRect.y + frameRect.height >= w.pageYOffset // if frame bottom exceeds page top
        ) {
          item.src = source; // set path
          item.removeAttribute("data-src"); // remove placeholder attr
        }
      }
    });

  /** load on docReady */
  d.addEventListener("DOMContentLoaded", [preloadPath, loadFrame]);

  /** throttle on scroll */
  const t = throttle(loadFrame, 64);
  w.addEventListener("scroll", t);

General Feedback

The code in the jsFiddle Demo appears to function well (although it isn't the same as the code above- see the next section for an explanation). There is good usage of const for values that are not re-assigned and functional programming with arrow functions.

Flaw with event listener setup

It appears that the fiddle has different code than appears here, but nonetheless, the code above contains the following line:

d.addEventListener("DOMContentLoaded", [preloadPath, loadFrame]);

The second argument appears to be an array. I haven't seen an array there used before and it doesn't appear to work. The Parameters section of the MDN documentation for addEventListener reads:

  listener
  The object which receives a notification (an object that implements the Event interface) when an event of the specified type occurs. This must be an object implementing the EventListener interface, or a JavaScript function. See The event listener callback for details on the callback itself.1

If you did need to have both functions run when that event occurs, you would either need to have a single callback function that calls both or call addEventListener once for each function. It appears that in the fiddle preloadPath has been removed...

Suggestions

Variable naming

The variable name frame sounds singular, yet it returns a NodeList which would typically contain multiple DOM elements.

const frame = d.querySelectorAll("iframe");

Thus a more appropriate name would be frames. That way when statements like frames.forEach() is read, it implies that a function is invoked for each of the frames.

The setAttributes() function

Correct me if I am wrong but this function appears to do the same thing that Object.assign(). I was able to replace calls to that function with calls to Object.assign() and still saw the attributes set as expected in Opera and Chrome.

Arrow functions with empty argument lists

You don't have to do this, but _ could be used instead of empty parentheses for arrow functions with no named arguments. Refer to this SO post and its answers for more context.

timeout in function returned by throttle()

While it would only save a couple lines, the arrow function passed to setTimeout() in the function returned by throttle() could be simplified to remove the curly braces. While this would mean that true would be returned, it doesn't affect anything.

const throttle = (fn, delay) => {
  let canCall = true;
  return (...args) => {
    if (canCall) {
      fn.apply(null, args);
      canCall = false;
      setTimeout(_ => canCall = true, delay);
    }
  };
};

1https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Parameters)


PHP class that behaves like enums I'm relatively new to PHP and one of the things I miss from other languages are enums. After looking around for some samples on how can I have somewhat similar to them, I came up with this implementation. I want to know how good is it, how can it be improved or any other flaws you can find in it. It's writen in PHP 7.
Base class
abstract class BaseEnum
{
    private $value;

    protected function __construct(string $value) {
        $this->value = $value;
    }

    public function getValue(): string {
        return $this->value;
    }

    public abstract static function parse(?string $value);

    public abstract static function InitializeEnums();
}

This is the base class from where all my "enums" inherit. In short this is an immutable class that hold a single string value and a getter for it (internally all enums will be string, opposed to other languages that are often ints). There is an abstract method that takes a raw string and returns the matching enum instance that needs to be implemented in a case-by-case basis, as well as an initialization method (see below for this).
Concrete enum class
class VoucherType extends BaseEnum {
    private static $invoice;
    private static $creditNote;
    private static $advance;

    public static function InitializeEnums() {
        VoucherType::$invoice = new VoucherType("I");
        VoucherType::$creditNote = new VoucherType("C");
        VoucherType::$advance = new VoucherType("A");
    }

    public static function parse(?string $value): ?self {
        if($value === VoucherType::$invoice->getValue()) return VoucherType::$invoice;
        if($value === VoucherType::$creditNote->getValue()) return VoucherType::$creditNote;
        if($value === VoucherType::$advance->getValue()) return VoucherType::$advance;
        return null;
    }

    public static function invoice(): self {
        return VoucherType::$invoice;
    }
    public static function creditNote(): self {
        return VoucherType::$creditNote;
    }
    public static function advance(): self {
        return VoucherType::$advance;
    }
}
VoucherType::InitializeEnums();

This is one of my real enum classes. All that it does is to provide the fixed valid values and appropriate getters for each one. Also implements the parse that looks each value in turn. It stores singletons of each possible value and those are returned as needed. The initialization of the singletons is done via the InitializeEnums method, which is called outside the class (a dirty trick to simulate a static constructor).
This enables to make simple comparisons with "constant" values like $var == VoucherType::invoice(), or to convert to/from string with appropriate invalid values handling (and replaced with nulls). Variables will always be from this class type (and not simple strings) and assignment/comparision works as expected.
How can I improve it and what's good, bad or should change about it?

I would simulate the enum using class constants.

class VoucherType {
  const INVOICE = 0;
  const CREDIT_NOTE = 1;
  const ADVANCE = 2;
}

$myType = VoucherType::INVOICE;

They are accesible from everywhere you can access the class, virtually everywhere if you use (and you should) an autoloader.

This is a readable and well-known pattern but there are more sophisticated options, starting with SplEnum and ending with php7-enum, but I like the simplicity of the constants.

Also I you are new to PHP I recommend you to read other's code to learn and get used to the language.


Program that reads 4 files and outputs contents of it into another file after formatting it The below Python 3 program scans 4 files a.txt, b.txt, c.txt, d.txt and then outputs the read data into a file output.txt in a formatted way. The first line of each file is guaranteed to have a header and the second line of each file will be blank. I'm required to scan those four files.

Program:

def main():
    with open('a.txt', 'r') as file_a, open('b.txt', 'r') as file_b, open('c.txt', 'r') as file_c, open('d.txt', 'r') as file_d:
        lines1, lines2, lines3, lines4 = file_a.readlines(), file_b.readlines(), file_c.readlines(), file_d.readlines()
        lines = [lines1, lines2, lines3, lines4]
        number_of_spaces = 5

        assert len(lines1) == len(lines2) == len(lines3) == len(lines4), "Error. Different number of lines found in the files"

        row, column = 0, 1
        with open('output.txt', 'w') as output:

            while row < len(lines):
                output.write(lines[row][0].strip() + ' ' * number_of_spaces)
                row += 1
            output.write('\n')
            row = 0

            while column < len(lines1):
                while row < len(lines):
                    output.write(lines[row][column].strip() + ' ' * (number_of_spaces + len(lines[row][0].strip()) - len(lines[row][column].strip())))
                    row += 1
                output.write('\n')
                column += 1
                row = 0

if __name__ == "__main__":
    main()

When executed gives output.txt:

Sl.No     Elements     Abbreviation     Mass

1         Hydrogen     H                1
2         Helium       He               4
3         Lithium      Li               7
4         Beryllium    Be               9
...
98        Californium  Cf               251
99        Einsteinium  Es               252
100       Fermium      Fm               257

Is there any room for improvement?

Additional information (if necessary):

Note that ... in the files mean that there are a lot of similar data and not that the file contains those dots.

a.txt:

Sl.No

1
2
3
...
99
100

b.txt:

Elements

Hydrogen
Helium
Lithium
Beryllium
...
Californium
Einsteinium
Fermium

c.txt:

Abbreviation

H
He
Li
Be
...
Cf
Es
Fm

d.txt:

Mass

1
4
7
9
...
251
252
257

I think that zip can save you some work:

with open('a.txt', 'r') as file_a, open('b.txt', 'r') as file_b, \
     open('c.txt', 'r') as file_c, open('d.txt', 'r') as file_d:
         for content in zip(file_a.readlines(), file_b.readlines(),
                            file_c.readlines(), file_d.readlines()):
             print(content)

The above is just a basic example, you will want to:

Add error checking (asserting for example)
Add an output file
Add necessary spaces


Validating the model outside of the controller in ASP.NET I wasn't quite sure whether I should do it this way or create a validation service and inject it, but this works:
Controller:
[HttpPost]
public async Task<IActionResult> NewUser(NewUsertVm viewModel)
{
    if (UserValidator.NewUserIsValid(viewModel, ModelState) == false)
    {
        return View(nameof(NewUser), viewModel);
    }

// Code omitted for brevity
}

UserValidatorClass
public static bool NewUserIsValid(NewUserVm viewModel, ModelStateDictionary modelState)
{
    if (string.IsNullOrEmpty(viewModel.RefOne) && string.IsNullOrEmpty(viewModel.RefTwo) && string.IsNullOrEmpty(viewModel.RefThree))
    {
        var validationMessage =
            "At least one of either the first, second or third ref numbers must be entered to add a new user. Please enter one of these numbers and try again.";
        modelState.AddModelError(nameof(viewModel.RefOne), validationMessage);
    }

    var regEx = new Regex("(^\\d{10}$)");

    if (viewModel.RefOne != null)
    {
        if (!regEx.IsMatch(viewModel.RefOne))
        {
            var validationMessage =
                "Ref One must be 10 digits and contain only numbers.";
                modelState.AddModelError(nameof(viewModel.RefOne), validationMessage);
        }
    }

    return modelState.IsValid;
}

The idea behind the validator is that it needs to ensure at least one of the three properties are not null (RefOne, RefTwo & Ref Three. Property names have been changed for post). If RefOne is used, it needs to match the RegEx pattern.
This appears to work as intended but is there a better way of achieving the same result?

Validating the model outside of the controller in ASP.NET:
Model validation with Data Annotations, the MVC framework will usually automated to handle the performance of actual validation and results to the end-user.
To validate the model outside of controller,we can create a sample of console application into the same application or in other application also.
Here,We are using class named User model:

using System.ComponentModel.DataAnnotations;
namespace ValidateExample
{
    public class User
    {
        [Required]
        [StringLength(20)]
        public string FirstName { get; set; }

        [Required]
        [StringLength(25, MinimumLength = 3)]
        public string LastName { get; set; }

        [Required]
        [EmailAddress]
        public string EmailAddress { get; set; }
    }
}

I have created, within the same Solution, a Console App project.Here's our Console App:

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using ValidateExample.Models;

namespace ValidateExample
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Starting validation:");

            User user = new User();
            user.FirstName = null;
            user.LastName = "aaa";
            user.Emailaddress = "eg@exam@example";

            var context = new ValidationContext(user, null, null);
            var results = new List<ValidationResult>();

            var isValid = Validator.TryValidateObject(user, context, results, true);

            if (!isValid)
            {
                Console.WriteLine("User is not valid! Errors are:");
                foreach (var validationResult in results)
                {
                    Console.WriteLine(validationResult.ErrorMessage);
                }
            }
            Console.WriteLine("Validation Complete...");
            Console.ReadKey();
        }
    }
}

We simply created a model named User and made some data-annotations in it.Then,In console class We putted some invalid values to test the application.
Here,We called the Validator.TryValidateObject() this function which validate the model according to the data-annotations.
Run the apllication :
The Console Output :-

Starting validation:
User is not valid! Errors are:
The FirstName field is required.    //(as We assigned null to FirstName field)
The EmailAddress field is not a valid email address.


Recursive Binary Tree Inorder Traversal in Python This is a very simple problem to see if my understanding of basic Python is correct
Problem:
Given a binary tree, return the inorder traversal of its nodes' values.
Example:

Input: [1,null,2,3]
Output: [1,3,2]

Solution:
# Definition for a binary tree node.
# class TreeNode:
#     def __init__(self, x):
#         self.val = x
#         self.left = None
#         self.right = None

class Solution:
    def inorderTraversal(self, root):
        """
        :type root: TreeNode
        :rtype: List[int]
        """
        self.inorder = []
        self.inorderHelper(root, self.inorder)
        return self.inorder

    def inorderHelper(self, node, inorder):
        if (node is None):
            return
        else:
            self.inorderHelper(node.left, inorder)
            inorder.append(node.val)
            self.inorderHelper(node.right, inorder)

Naming

Solution is a bit generic for a class name. It could basically mean anything, because almost every algorithm has some kind of solution. A better fitting name would be TreeTraversal. It tells the reader (you, but also others) what it means.

inorderTraversal has two problems. Firstly, it doesn't sound like a method name, because methods are usually verbs. Secondly, the convention for methods and functions in Python is lower_case_with_underscores, also called snake case. My suggestion is to name it traverse_inorder, or even just inorder considering you rename the class to TreeTraversal.

inorderHelper has the same two problems as the previous one, but additionally it does not really tell you what it does. If it is a helper, maybe it should be a class, but it does not do enough for that. If it is a method or function, what does it help you with, i. e. what does it do? A better name might be traverse, and you could consider making it an inner method of the other method, since it will not be used outside of it. That way the user of the class does not need to think about which one of the two methods to use.

OOP

You might consider making all that code (after the suggested changes above, only one short method) part of the tree class. After all, it depends on the tree anyways. Of course this does not apply if you did not write the Tree class yourself, or if you want to make a more generic traversal class for different types of trees.

In def inorderHelper(self, node, inorder) you do not need to specify a parameter for inorder. Since it is a member variable, you can just access it via self.inorder.

def inorderHelper(self, node, inorder):
    if (node is None):
        return
    else:
        self.inorderHelper(node.left, inorder)
        inorder.append(node.val)
        self.inorderHelper(node.right, inorder)

You can invert the if-condition and thus make the code shorter. The else is implicit. The parantheses around the condition are not needed and usually not used, since without them it reads more naturally.

def inorderHelper(self, node, inorder):
    if node is not None:
        self.inorderHelper(node.left, inorder)
        inorder.append(node.val)
        self.inorderHelper(node.right, inorder)


Deep Neural Network in Python I have written a neural network in Python and focused on adaptability and performance. I want to use it to dive deeper into that field. I am far from being an expert in neural networks and the same goes for Python. I do not want to use Tensorflow since I really want to understand how a neural network works.

My questions are:

How can I increase the performance? At the moment it takes days to train the network

The code runs on a single core. But since every loop over the batches run independently it can be parallelized.

How can I parallelize the loop over the batches?

I found some tutorials on parallel loops in Python but I could not apply it to my problem.

Here is my tested code with some pseudo training data:

from numpy import random, zeros, array, dot
from scipy.special import expit
import time

def sigma(x):
    return expit(x)

def sigma_prime(x):
    u = expit(x)
    return  u-u*u

def SGD(I, L, batch_size, eta):

    images = len(L)

    # Pre-activation
    z = [zeros((layer_size[l],1)) for l in range(1,nn_size)]

    # Activations
    a = [zeros((layer_size[l],1)) for l in range(nn_size)]

    # Ground truth
    y = zeros((images, layer_size[-1]))
    for i in range(images):
        y[i,L[i]] = 1.0

    while (1):

        t0 = time.time()

        # Create random batch
        batch = random.randint(0,images,batch_size)

        dW = [zeros((layer_size[l+1], layer_size[l])) for l in range(nn_size-1)]
        db = [zeros((layer_size[l],1)) for l in range(1, nn_size)]

        for i in batch:
            # Feedforward
            a[0] = array([I[i]]).T
            for l in range(nn_size-1):
                z[l] = dot(W[l], a[l]) + b[l]
                a[l+1] = sigma(z[l])

            # Backpropagation
            delta = (a[nn_size-1]-array([y[i]]).T) * sigma_prime(z[nn_size-2])
            dW[nn_size-2] += dot(delta, a[nn_size-2].T)
            dW[nn_size-2] += delta.dot(a[nn_size-2].T)
            db[nn_size-2] += delta
            for l in reversed(range(nn_size-2)):
                delta = dot(W[l+1].T, delta) * sigma_prime(z[l])
                dW[l] += dot(delta, a[l].T)
                db[l] += delta

        # Update Weights and Biases
        for l in range(nn_size-1):
            W[l] += - eta * dW[l] / batch_size
            b[l] += - eta * db[l] / batch_size

        print(time.time() - t0)

input_size = 1000
output_size = 10

layer_size = [input_size, 30**2, 30**2, 30**2, output_size]

nn_size = len(layer_size)
layer_size = layer_size

# Weights
W = [random.randn(layer_size[l+1],layer_size[l]) for l in range(nn_size-1)]

# Bias
b = [random.randn(layer_size[l],1) for l in range(1,nn_size)]

# Some random training data with label
size_training_data = 1000
# Random data I of size "size_training_data" x "input_size"
I = random.rand(size_training_data, input_size)
# Label for all training data
L = random.randint(0,10, input_size)

batch_size = 100
eta = 0.1
SGD(I, L, batch_size, eta)

The output shows the time needed for one batch of size batch_size.

The reason why your epochs are so slow is because you are iterating over each example in the batch and calculating the gradients in a for loop. The key to speeding this up is realizing that you are performing the same operations over every example in the batch, and so you can stack your examples into a matrix and calculate the gradients for all examples in a single matrix operation.

Let's break that down. For a single example, you start with a feature vector of size (1000,) and that is linearly transformed by multiply it by a weight matrix of size (1000,900) resulting in a (900,1) vector that is then added with a bias vector of size (900,1). This is then non-linearly transformed, which does not affect the dimensions, to result in the first hidden layer of size (900,1).

This is 900 hidden nodes for the first example.

However, since we are performing the same operations to every example in the batch, we can stack the 100 examples to form a matrix of size (100,1000) instead of (1,1000) then take the dot product of this input matrix with the transpose of the weight matrix, (1000,900) for a resulting matrix of (100,900). Add the bias (1,900), which is broadcasted automatically in numpy to a matrix of size (100,900) (it's the same bias vector stacked 100 times) and apply the non-linear transform for a final matrix of size (100,900). This is 900 hidden nodes each for 100 examples.

This can be applied to each hidden layer in the network.

Factoring the original code:

for i in batch:
    # Feedforward
    a[0] = array([I[i]]).T
    for l in range(nn_size-1):
        z[l] = dot(W[l], a[l]) + b[l]
        a[l+1] = sigma(z[l])

    # Backpropagation
    delta = (a[nn_size-1]-array([y[i]]).T) * sigma_prime(z[nn_size-2])
    dW[nn_size-2] += dot(delta, a[nn_size-2].T)
    dW[nn_size-2] += delta.dot(a[nn_size-2].T)
    db[nn_size-2] += delta
    for l in reversed(range(nn_size-2)):
        delta = dot(W[l+1].T, delta) * sigma_prime(z[l])
        dW[l] += dot(delta, a[l].T)
        db[l] += delta

into matrix math form:

a[0] = I[batch]
for l in range(nn_size-1):
    z[l] = a[l].dot(W[l].T) + b[l].T
    a[l+1] = sigma(z[l])

delta = (a[nn_size-1] - y[batch]) * sigma_prime(z[nn_size-2])
dW[nn_size-2] += 2*delta.T.dot(a[nn_size-2])
db[nn_size-2] += np.sum(delta.T, axis=1, keepdims=True)

for l in reversed(range(nn_size-2)):
    delta = delta.dot(W[l+1]) * sigma_prime(z[l])
    dW[l] += a[l].T.dot(delta).T
    db[l] += np.sum(delta.T, axis=1, keepdims=True)

When calculating gradients over batches, taking the sum or average of the gradients over the examples both work, but Andrew Ng suggests using the average over the batch as explained in his course and here: https://stats.stackexchange.com/questions/183840/sum-or-average-of-gradients-in-mini-batch-gradient-decent

In this case, since you divide the gradients by batch size, you can just sum the gradients over the batch.

With the original for loop implementation over 10 epochs, each epoch takes anywhere between 1.75 - 2.25s with an average of 1.91s per epoch.

With the matrix implementation over 100 epochs, each epoch takes between 0.06 - 0.25s with an average of 0.08s per epoch.

Parallelizing sgd is also somewhat non-trivial as SGD is a sequential algorithm. For example, imagine that you are standing on the top of a hill and are trying to find the quickest way down (without being able to see it). The only way to make your way down the hill is by actually taking steps downwards and re-evaluating your options at each step. Even if there were multiple clones of you, all your clones would be standing at the same spot on the hill and so the information gain would be limited. Nevertheless, there are techniques for parallel sgd and so if you are determined on pursuing that, I would suggest you read some papers on the topic:

Cheng 2017: Weighted parallel SGD for distributed unbalanced-workload training system https://arxiv.org/abs/1708.04801

Zinkevich 2010: Parallelized Stochastic Gradient Descent http://martin.zinkevich.org/publications/nips2010.pdf

Or for a higher level overview:
http://blog.smola.org/post/977927287/parallel-stochastic-gradient-descent


Launching an interpreted script I was just wondering if this is a good example, following good practices, of a C program for launching an interpreted script from a native binary executable. Here the interpreter is Perl and the script an implementation of something analogous to du --apparent-size, but the idea is general.

Also, is this code secure against potentially hostile users, at least if Perl is the interpreter (for instance, in a suid or sgid program)?

#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <errno.h>
int main(int argc, char** argv)
{
  char* command[argc+3];
  command[0] = "/usr/bin/perl";
  command[1] = "-W";
  command[2] = "-T";
  command[3] = "/home/demetri/bin/awklikeperl.pl";
  for (int i = 0; i < argc; i++) {
    command[i+4] = argv[i+1];
  }
  execv(command[0], command);
  int error = errno;
  fprintf(stderr, "Failed to exec: %s\n", strerror(error));
  if (error == ENOENT) {
    return(127);
  } else {
    return(126);
  }
}

Consider starting the child process in a sandbox.  /usr/bin/perl is likely safe from a malicious user since /usr/bin is typically locked down by the root user(s).  But /home/demetri/bin/awklikeperl.pl could be replaced if the owner is not careful with permissions.  Putting the process in a sandbox will not only protect the rest of your system from attack, but help you think about what files will be made available.

Obviously, the content of the script should be reviewed for vulnerabilities as well.  Take a look at perlsec for help on that front.

3 is a bit of a magic number in the C program.  As you've already discovered, it's easy to forget to change that number if you add or subtract parameters from the command array.  If you define that in one place (say #DEFINE STARTING_ARGS 3) you'll only need to change the value in one place.

Speaking of which, I considered suggesting adding a -- parameter to prevent any shenanigans with adding extra switches.  But once you pass the script name to perl, it passes all other arguments onto the script.  So I think it's not possible to use -e to execute an arbitrary command.


Query for finding possible "Not An Answer" posts I have this query in SEDE to try to find possible not an answers to flag. (I already went through a lot of them, for Stack Overflow, so I don't really recommend anyone else using this one since isn't that helpful anymore.)

As you can see, it looks to be a lot longer and redundant than it probably needs to be. Is there any way to shorten this?

SELECT
    p.Id as [Post Link],
    len(p.Body),
    p.CreationDate as Date
FROM
    Posts as p
JOIN
    Users as u
ON
    p.OwnerUserId = u.Id
JOIN
    Posts as q
ON
    p.ParentId = q.Id
WHERE
    u.Reputation < 126
  and
    len(p.Body) < 300
  and
    p.Body LIKE '%a href=%'
  and
    p.Score < 3
  and
    p.PostTypeId = 2
  and
    p.CommentCount < 3
  and
    q.ClosedDate IS NULL
  and
    q.FavoriteCount < 3
  and
    q.CommunityOwnedDate IS NULL
  and
    q.Body NOT LIKE '%plugin%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%plugin%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%plug-in%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%plug-in%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%tool%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%tool%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%framework%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%framework%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%API%'
  and
    q.Body NOT LIKE '%API%'
  and
    q.Title NOT LIKE '%IDE%'
  and
    q.Body NOT LIKE '%IDE%'
  and
    q.Title NOT LIKE '%library%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%library%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%cms%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%cms%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%open source%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%open source%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%application%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%application%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%software%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%software%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%tutorial%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%tutorial%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%book%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%book%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%module%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%module%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%resource%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%resource%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%engine%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%engine%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%lightweight%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%lightweight%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%opensource%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%opensource%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%technologies%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%technologies%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%simulator%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%simulator%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%compiler%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%compiler%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Body NOT LIKE '%editor%' COLLATE SQL_Latin1_General_CP1_CI_AS
  and
    q.Title NOT LIKE '%editor%' COLLATE SQL_Latin1_General_CP1_CI_AS
ORDER BY
    len(p.Body) ASC

Firstly, I would avoid using JOIN on it's own. INNER JOIN should be used instead, it's much easier to understand what is happening at a glance if you see INNER JOIN over JOIN

I would capitalise the AS and AND's, it just makes it more consistent.

Otherwise, the general formatting is pretty nice.

I would maybe argue that the table aliases are a little unnecessary, Posts and Users are already pretty short and are more meaningful than the single letter aliases you gave them. I would consider aliasing the joined Posts table Answers or something similar.

Lastly, the largest point. You are checking if both the body and title do not contain the same string. You can concatenate body and title together, with a space to separate them, and check for the word/phrase you are searching for in that instead, effectively halving the size of your where statement.

SELECT
Posts.Id AS [Post Link],
LEN(Posts.Body),
Posts.CreationDate AS Date

FROM Posts

INNER JOIN Users
ON Posts.OwnerUserId = Users.Id

INNER JOIN Posts AS Answers
ON Posts.ParentId = Answers.Id

WHERE Users.Reputation < 126
AND LEN(Posts.Body) < 300
AND Posts.Body LIKE '%a href=%'
AND Posts.Score < 3
AND Posts.PostTypeId = 2
AND Posts.CommentCount < 3
AND Answers.ClosedDate IS NULL
AND Answers.FavoriteCount < 3
AND Answers.CommunityOwnedDate IS NULL
AND Answers.Body+' '+Answers.Title NOT LIKE '%plugin%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%plug-in%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%tool%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%framework%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%API%'
AND Answers.Body+' '+Answers.Title NOT LIKE '%IDE%'
AND Answers.Body+' '+Answers.Title NOT LIKE '%library%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%cms%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%open source%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%application%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%software%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%tutorial%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%book%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%module%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%resource%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%engine%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%lightweight%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%opensource%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%technologies%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%simulator%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%compiler%' COLLATE SQL_Latin1_General_CP1_CI_AS
AND Answers.Body+' '+Answers.Title NOT LIKE '%editor%' COLLATE SQL_Latin1_General_CP1_CI_AS

ORDER BY LEN(Posts.Body) ASC

Here is the SEDE Query


REST API stored in a MongoDB Atlas Database I am building my first REST API using data stored in my MongoDB Atlas Database. I tried to read the REST API conventions with the versioning, using plural nouns for resources and sending the right error codes. I wonder if the following code correctly follows Good Practices for REST APIs and if there is anything that can be written better or more neatly.

connection_str = config.connection_str
db_name = config.db_name

app = Flask(__name__)
cors = CORS(app)
client = MongoClient(connection_str)
db = client.get_database(db_name)

@app.route("/api/v1/players", methods = ['GET'])
def get_all_players():
    # TODO: Implement pagination
    """
    returns a list of players
    """
    try:
        players = db.players.find()
        output = []
        for player in players:
            output.append({
                'name': player['name'],
                'position': player['position'],
                'nationality': player['nation'],
                'position': player['position'],
                'foot': player['foot'],
                'age': utils.date_to_age(player['birth_date']),
                'profile_img': player['profile_img'],
                'abilities': player['abilities']
            })
        return jsonify({'result': output})

    except:
        return jsonify({
            'result': 'failure',
            "error": 400,
            "message": 'Bad Request'}), 400

@app.route("/api/v1/players/<name>", methods = ['GET'])
def get_player(name):
    """
    returns an object of a player given a name
    """
    try:
        player = db.players.find_one({'name': name})
        output = {
            'name': player['name'],
            'position': player['position'],
            'nationality': player['nation'],
            'position': player['position'],
            'foot': player['foot'],
            'age': utils.date_to_age(player['birth_date']),
            'profile_img': player['profile_img'],
            'abilities': player['abilities']
        }
        return jsonify({'result': output})

    except:
        return jsonify({
            'result': 'failure',
            "error": 400,
            "message": "Bad Request (Double check player's name)"}), 400

if __name__ == "__main__":
    # app.run(ssl_context='adhoc')
    app.run()

The API doesn't do that much right now. It just reads data in the DB, but I want to see if any style can be fixed at this early stage.

In general, I think the code looks pretty good.  I would suggest two changes:

Dry

In an effort to keep the code DRY, I suggest constructing the player dict in a function for that purpose like:

def player_to_dict(player):
    return {
        'name': player['name'],
        'position': player['position'],
        'nationality': player['nation'],
        'position': player['position'],
        'foot': player['foot'],
        'age': utils.date_to_age(player['birth_date']),
        'profile_img': player['profile_img'],
        'abilities': player['abilities']
    }

This takes out quite a bit of code from:

def get_player(name):
    try:
        player = db.players.find_one({'name': name})
        return jsonify({'result': player_to_dict(player)})
    ....

I like comprehensions:

Using a list comprehension and the new player_to_dict(), you can remove the explicit for loop and instead make get_all_players much smaller and looking like quite similar to get_player.

def get_all_players():
    try:
        players = db.players.find()
        return jsonify({'result': [player_to_dict(player) for player in players]})
    ....