Brute Force generator I just wrote a little Python 3 program to spit out every possible combination of a set of 99 characters.
It gets the job done, but I would be very interested in what you think of it.
I am just a few days into Python, so I would be grateful for even seemingly obvious advice.

import sys

# List of 99 characters and a blank string:

lib=["","0","1","2","3","4","5","6","7","8","9","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","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","°","!","\"","§","$","%","&","/","(",")","=","ß","´","`","+","#","-",".",",",">","<","@","€","|","^","~","–","{","[","]","}","Ä","Ö","Ü","ä","ö","ü"]

# 10 counters for up to 10-digit-combinations:

counter0=-1
counter1=0
counter2=0
counter3=0
counter4=0
counter5=0
counter6=0
counter7=0
counter8=0
counter9=0

# Repetitive if-statements adding to the counters:

for i in range(sys.maxsize**99999):
    counter0+=1

    if counter0>99:
        counter0=counter0*0
        counter1+=1

    elif counter1>99:
        counter1=counter1*0
        counter2+=1

    elif counter2>99:
        counter2=counter2*0
        counter3+=1

    elif counter3>99:
        counter3=counter3*0
        counter4+=1

    elif counter4>99:
        counter4=counter4*0
        counter5+=1

    elif counter5>99:
        counter5=counter5*0
        counter6+=1

    elif counter6>99:
        counter6=counter6*0
        counter7+=1

    elif counter7>99:
        counter7=counter7*0
        counter8+=1

    elif counter8>99:
        counter8=counter8*0
        counter9+=1

    elif counter9>99:
        print("DONE.")

# Printing the translation from counters to character - and deleting the former output so it stays in one line:

    else:
        print(lib[counter0]+lib[counter1]+lib[counter2]+lib[counter3]+lib[counter4]+lib[counter5]+lib[counter6]+lib[counter7]+lib[counter8]+lib[counter9], end="\r")
        sys.stdout.write("\b"*10+" "*10+"\b"*10)

We can convert a plain string to a list rather than maintain a list for the characters.
It is far easier to modify and read the following then a list.
lib = [''] + list('0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz°!"§$%&/()=ß´`+#-.,><@€|^~–{[]}ÄÖÜäöü')

When we have counter0, counter1, ..., countern it's a hint that we should use a list.
counters = [-1, 0, 0, 0, 0, 0, 0, 0, 0, 0]

We can then use counters[0] as a drop in replacement for counter0.

Now that we have counters which is a list we can simplify your print from the following.

print(lib[counters[0]] + lib[counters[1]] + lib[counters[2]] + lib[counters[3]] + > lib[counters[4]] + lib[counters[5]] + lib[counters[6]] + lib[counters[7]] + lib[counters[8]] + lib[counters[9]], end="\r")

We can use a for loop to loop through counters, index lib and print the character. We'll use end="" to get the same format that you have. Since we've changed from "\r" to "" we'll need to print that afterwards.
for counter in counters:
    print(lib[counter], end="")
print(end="\r")

It would be better to use len(lib) rather than hard coding 99 in your ifs. If we change the content of lib it's much easier to edit just lib rather than lib and 10 99's.

Rather than counter0=counter0*0 it would make more sense to remove the multiplication and just set the value to 0.
counter0 = 0

It's convention in Python to put a space either side of operators. This means a+b should instead be a + b. This is as it makes it easier to see what is and is not an operator and either side of an operator.

It's convention in Python to use _ as a 'throw away' variable. This means it's normal to use _ rather than i in your for loop.

Together this gets:
import sys

lib = [''] + list('0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz°!"§$%&/()=ß´`+#-.,><@€|^~–{[]}ÄÖÜäöü')
counters = [-1, 0, 0, 0, 0, 0, 0, 0, 0, 0]

for _ in range(sys.maxsize**99999):
    counters[0] += 1
    if counters[0] >= len(lib):
        counters[0] = 0
        counters[1] += 1
    elif counters[1] >= len(lib):
        counters[1] = 0
        counters[2] += 1
    elif counters[2] >= len(lib):
        counters[2] = 0
        counters[3] += 1
    elif counters[3] >= len(lib):
        counters[3] = 0
        counters[4] += 1
    elif counters[4] >= len(lib):
        counters[4] = 0
        counters[5] += 1
    elif counters[5] >= len(lib):
        counters[5] = 0
        counters[6] += 1
    elif counters[6] >= len(lib):
        counters[6] = 0
        counters[7] += 1
    elif counters[7] >= len(lib):
        counters[7] = 0
        counters[8] += 1
    elif counters[8] >= len(lib):
        counters[8] = 0
        counters[9] += 1
    elif counters[9] >= len(lib):
        print("DONE.")
    else:
        for counter in counters:
            print(lib[counter], end="")
        print(end="\r")
        sys.stdout.write("\b"*10 + " "*10 + "\b"*10)

There are still some changes that we can make to your code to make it easier to work with.
These are a bit more advanced so don't worry if you don't get them straight away.

We can change your big if elif block into a single for loop.
Lets see what we have so far:

if counters[0] > len(lib):
    counters[0] = 0
    counters[1] += 1

We know that this section is repeated each time for each index.
So we can make this generic by changing 0 to index and 1 to index + 1.
if counters[index] >= len(lib):
    counters[index] = 0
    counters[index + 1] += 1

Now we just need to loop over range(len(counters) - 1) to duplicate the block 9 times.
for index in range(len(counters) - 1):
    if counters[index] >= len(lib):
        counters[index] = 0
        counters[index + 1] += 1

We can use some Python sugar to make your print for loop 'cleaner'.
Firstly we can remove all the prints by building a list.
combination = []
for counter in counters:
    combination.append(lib[counter])

From here we can join all the strings together with "".join and pass it to print like you did before. This will join the list by empty strings so it converts it's like manually doing combination[0] + combination[1] + ....
print("".join(combination), end="\r")

We can then use a list comprehension to build combination in one line.
This is just syntaxtic sugar and is the same as the for loop we used before.
It's just different, cleaner, syntax.
combination = [lib[counter] for counter in counters]

We can use either a while True loop or itertools.count rather than range(sys.maxsize**99999) to loop infinitely.
while True:
    counters[0] += 1

import itertools

for _ in range(itertools.count()):
    counters[0] += 1

We can probably just use print rather than sys.stdout.write.
To make it so there is no newline we can pass end="".
This however doesn't flush the stream straight away, and so we need to pass flush=True.

lib = [''] + list('0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz°!"§$%&/()=ß´`+#-.,><@€|^~–{[]}ÄÖÜäöü')
counters = [-1, 0, 0, 0, 0, 0, 0, 0, 0, 0]

while True:
    counters[0] += 1
    for index in range(len(counters) - 1):
        if counters[index] >= len(lib):
            counters[index] = 0
            counters[index + 1] += 1

    if counters[9] >= len(lib):
        print("DONE.")
    else:
        print("".join([lib[counter] for counter in counters]), end="\r")
        print("\b"*10 + " "*10 + "\b"*10, end="", flush=True)


How is my SFML sprite Move() function? I'm making a pong game with SFML and in the process made a function that takes a Time, Speed, Angle of movement, buffer for the movement on the X axis, and buffer for the movement on the Y axis.

I then implemented this function into my custom mySprite class, more specifically into its update() function which updates its Position using the above function to calculate its new position.

However, I'm worried that giving the function the FrameTime() of the Window (the time since the frame was last updated) is not the best choice as the sprites might start jumping around if there's lag between frame updates or other problems.

Finally I would like to know about my organization, planning, etc.

#include<SFML/Graphics.hpp>
#include<SFML/System.hpp>
#include<cmath>
#include<vector>
#    define M_PI 3.14159265358979323846

sf::RenderWindow Window;

template<typename T>
void CalculateMove(T Time, T Speed, T Angle, T& buffX, T& buffY)
{   //determine what quadrant of circle we're in
    unsigned int  Quadrant= 1;
    if(Angle>90)  Quadrant= 2;
    if(Angle>180) Quadrant= 3;
    if(Angle>270) Quadrant= 4;

    //anything above 90 would be impossible triangle
    Angle= (float)(Angle-(int)Angle)+(float)((int)Angle%90);

    // calculates x and y based on angle and Hypotenuse.02433
    if((int)Angle!=0){
        buffX= sin(Angle / 180 * M_PI)/ (1.f/(Speed*Time));
        buffY= sin((180-Angle-90)/ 180 * M_PI)/ (1.f/(Speed*Time));}

    else{// Movement is a straight line on X or Y axis
        if(Quadrant==0 || Quadrant==2) buffX= Speed*Time;
        if(Quadrant==1 || Quadrant==4) buffY= Speed*Time;}

    //Quadrant Factor (positive or negative movement on the axis)
    switch(Quadrant){
    case 1: break;
    case 2: buffX=-buffX; break;
    case 3: buffX=-buffX; buffY=-buffY; break;
    case 4: buffY=-buffY; break;}
};

/////////////////////////////////////////   Mysprite    ////////////////////////////////
class mySprite : public sf::Sprite
{
private:
    float velocity;
    float angle;

public:
    // all the values needed by the base class sprite();
    mySprite(
        const sf::Image& Img,
        const sf::Vector2f& Position = sf::Vector2f(0, 0),
        const sf::Vector2f& Scale = sf::Vector2f(1, 1),
        float Rotation = 0.f,
        const float Angle= 0.f,
        const float Velocity= 0.f,
        const sf::Color& Col = sf::Color(255, 255, 255, 255)):
      Sprite(Img, Position, Scale, Rotation, Col){
        angle= Angle;
        velocity= Velocity;};

    float Velocity(){return velocity;};
    void SetVelocity(float newVelocity){velocity=newVelocity;};
    float Angle(){return angle;};
    void SetAngle(float newAngle){angle=newAngle;};

    void Update(){
        float frameTime= Window.GetFrameTime();
        float X=0,Y=0;
        CalculateMove(frameTime,velocity,angle,X,Y);
        Move(X,-Y);
    };

    void Reflect(float CollAngle){
        SetRotation(-GetRotation());
        angle=-angle;
        //TODO: factor in the collision angle
    };
};

It's often simpler to have a fixed frame rate in a game. Then you don't have to multiply everything by a time factor. In this case, you do not have to lower the frame rate to the lowest value that all computers can handle. You can for example let the position updates run at 200Hz and update the graphics at the highest speed the computer can handle (which may be lower than 200Hz).

If the updates can easily run at a fixed speed on all hardware you want it to run on, this can work well. It is a good way to get consistent behaviour in physics simulations, and for simple games, updating at a much higher speed than needed for the graphics is not a problem.

On the other hand, if the position updates are complicated, you may not be able to run them at top speed, and you'll have to handle varying time deltas (as in your code). This is often more difficult. The "jumping around in case of lag" problem has a simple (but not perfect) solution: just put a cap on the time delta, so you'll never do calculations on a time delta bigger than say 50 ms.

I guess this part of the question would fit better at gamedev.stackexchange.com, where there probably already are discussions about it.

EDIT: Sure is, have a look at https://gamedev.stackexchange.com/questions/1589/fixed-time-step-vs-variable-time-step


How can I make this better? Python Web scraping I am 12 days old into Python and web scraping and managed to write my first ever automation script. Please review my code and point out blunders If any.

What do I want to achieve?

I want to scrape all chapters of each Novel in each category and post it on a WordPress blog to test. Please point out anything that I missed, and is mandatory to run this script on the WordPress blog.

from requests import get
from bs4 import BeautifulSoup
import re

r = get(site,
        headers={"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko)"})
soup = BeautifulSoup(r.text, "lxml")
category = soup.findAll(class_="search-by-genre")

# Getting all categories
categories = []
for link in soup.findAll(href=re.compile(r'/category/\w+$')):
    print("Category:", link.text)
    category_link = link['href']
    categories.append(category_link)

# Getting all Novel Headers
for category in categories:
    r = get(category_link,
            headers={"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko)"})
    soup = BeautifulSoup(r.text, "lxml")
    Novels_header = soup.findAll(class_="top-novel-header")

    # Getting Novels' Title and Link
    for Novel_names in Novels_header:
        print("Novel:", Novel_names.text.strip())

        Novel_link = Novel_names.find('a')['href']

        # Getting Novel's Info
        r = get(Novel_link, headers={"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko)"})
        soup = BeautifulSoup(r.text, "lxml")
        Novel_divs = soup.findAll(class_="chapter-chs")

        # Novel Chapters
        for articles in Novel_divs:
            article_ch = articles.findAll("a")
            for chapters in article_ch:
                ch = chapters["href"]

                # Getting article
                r = get(ch, headers={"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko)"})
                soup = BeautifulSoup(r.content, "lxml")
                title = soup.find(class_="block-title")
                print(title.text.strip())
                full_article = soup.find("div", {"class": "desc"})

                # remove ads inside the text:
                for ads in full_article.select('center, small, a'):
                    ads.extract()

                print(full_article.get_text(strip=True, separator='\n'))

Naming

Variable names should be snake_case, and should represent what they are containing. I would also use req instead of r. The extra two characters aren't going to cause a heartache.

Constants

You have the same headers dict in four different places. I would instead define it once at the top of the file in UPPER_CASE, then just use that wherever you need headers. I would do the same for site.

List Comprehension

I would go about collecting categories in this way:

categories = [link['href'] for link in soup.findAll(href=re.compile(r'/category/\w+$'))]

It's shorter and utilizes a quirk in the python language. Of course, if you want to print out each one, then add this just after:

for category in categories:
    print(category)

Also, it seems like you assign category_link to the last element in the list, so that can go just outside the list comprehension.

Save your assignments

Instead of assigning the result of soup.find to a variable, then using it in a loop, simply put that soup.find in the loop. Take a look:

for articles in soup.findAll(class_="chapter-chs"):
    for chapters in articles.findAll("a"):
        ....

As a result of the above changes, you code would look something like this:

from requests import get
from bs4 import BeautifulSoup
import re

HEADERS = {"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko)"}
SITE = "https://readlightnovel.org/"

req = get(SITE, headers=HEADERS)
soup = BeautifulSoup(req.text, "lxml")
category = soup.findAll(class_="search-by-genre")

categories = [link['href'] for link in soup.findAll(href=re.compile(r'/category/\w+$'))]
category_link = categories[-1]

# Getting all Novel Headers
for category in categories:
    req = get(category_link, headers=HEADERS)
    soup = BeautifulSoup(req.text, "lxml")
    novels_header = soup.findAll(class_="top-novel-header")

    # Getting Novels' Title and Link
    for novel_names in novels_header:
        print("Novel:", novel_names.text.strip())

        novel_link = novel_names.find('a')['href']

        # Getting Novel's Info
        req = get(novel_link, headers=HEADERS)
        soup = BeautifulSoup(req.text, "lxml")

        # Novel Chapters
        for articles in soup.findAll(class_="chapter-chs"):
            for chapters in articles.findAll("a"):
                ch = chapters["href"]

                # Getting article
                req = get(ch, headers=HEADERS)
                soup = BeautifulSoup(req.content, "lxml")
                title = soup.find(class_="block-title")
                print(title.text.strip())
                full_article = soup.find("div", {"class": "desc"})

                # remove ads inside the text:
                for ads in full_article.select('center, small, a'):
                    ads.extract()

                print(full_article.get_text(strip=True, separator='\n'))


Convenience function to save and load pickle objects I want to come up with a simple convenience function to save and load pickle objects. A secondary objective is to simply execute the function without the above function. I have come up with two versions so far. Just want some opinions, which of the following will be better from a software engineering point of view. Or more pythonic. If possible please suggest a still better version, with changed parameter names or something already out there.

def test(filename, func, exec_only=False, ignore_file=False, **kwargs):
    if exec_only:
        obj = func(**kwargs)
    else:
        fname = Path(filename)
        if fname.exists() and not ignore_file:
            obj = jl.load(filename)
        else:
            obj = func(**kwargs)
            jl.dump(obj, fname, compress=True)
    return obj

Another version:

def test2(filename, func, exec_only=False, ignore_file=False, **kwargs):
    if Path(filename).exists() and not ignore_file and not exec_only:
        obj = jl.load(filename)
    else:
        obj = func(**kwargs)
        if not exec_only:
            jl.dump(obj, fname, compress=True)
    return obj

Thanks a lot in advance.

EDIT: I created this function as a machine learning utility, since execution is typically very costly and the decision to execute is dependent on the presence of the file. I will be incorporating some features from Sam's answer. Thanks.

A few general tips:

Have a function name that indicates what the function does.  If it's hard to come up with a name that describes the function's purpose (e.g. because it does totally different things depending on what argument you pass it), it's a clue that the function needs to be broken up into multiple functions with clearer purposes.  It looks like your function will either save OR load depending what arguments you pass it; this is pretty bad from a software design point of view because it makes it less obvious what it's doing from the caller's perspective, and harder to verify correctness from the implementation's perspective.
Use type annotations.  These also help to make it clear what your function does, and they let you use mypy to automatically catch bugs.  Note that **kwargs are difficult to declare useful types for; any function call that takes **kwargs is impossible to check statically.  Given that your kwargs are only used to invoke func (I'm guessing func is a constructor?), I would just put the responsibility on the caller to construct the object before passing it to you; that way if the function is typed, the checking will happen in the caller's frame, rather than having your function "hide" the types and permit bugs to happen.
Don't have flags whose only purpose is to make the function a no-op.  This is needlessly confusing; if the caller wants nothing to happen, they can just not call your function.

Given those suggestions I think you'd end up with two different (and very simple) functions:

from typing import Optional

def load_pickle(file_name: str) -> Optional[object]:
   """Load a pickled object from file_name IFF the file exists."""
   return jl.load(file_name) if Path(file_name).exists() else None

def save_pickle(obj: object, file_name: str) -> None:
    """Pickle and save the object to file_name."""
    jl.dump(obj, file_name, compress=True)


Preforming some async operations with Rx.js I tried to come up with a good challenge to learn some Rx.js programming and I came up with this chain of operations below. As well as a bit of code that works and does these things.

Take github username
Create folder based on username (/repos-${user})
Get all repos for username (https://api.github.com/users/${user}/repos)
Get the hash of the repo data
Create file where name is hash and content is repo

I'm looking for insights and optimizations, shortcuts, etc, to make this better.

Here's the code.

import { join } from 'path'
import axios from 'axios'
import Rx from 'rx'
import Promise from 'bluebird'
import crypto from 'crypto'
import fs from 'fs-extra'
import Debug from 'debug'

let debug = Debug('github-repos')

Promise.promisifyAll(fs)

let {
  ensureDirAsync,
  outputJsonAsync } = fs

function getRepos (user) {
  let theUrl = `https://api.github.com/users/${user}/repos`
  return axios.get(theUrl)
  .then(result => {
    debug(`got results back for ${user}`)
    return result.data
  })
}

function getHash (json) {
  let text = JSON.stringify(json)
  return crypto.createHash('md5').update(text).digest('hex')
}

function createDir (user) {
  return ensureDirAsync(join(__dirname, `/repos-${user}`))
}

function createFile (user, hash, data) {
  let file = join(__dirname, `/repos-${user}/${hash}.json`)
  return outputJsonAsync(file, data).then(x => file)
}

let ghUser$ = Rx.Observable.from(['reggi', 'jackofseattle', 'nolanlawson'])

let ensureUser$ = ghUser$
  .do((u) => debug(`Creating directory for: ${u}`))
  .flatMap(u => createDir(u))

let getRepos$ = ghUser$
  .do((u) => debug(`fetching user repos for: ${u}`))
  .flatMap(u => getRepos(u))

let repos$ = getRepos$
  .flatMap(repos => repos.map(repo => ({hash: getHash(repo), repo})))  // create hash for each

let fileWriter$ = repos$
  .map(repo => ({user: repo.repo.owner.login, hash: repo.hash, repo: repo.repo}))
  .flatMap(({user, hash, repo}) => {
    return createFile(user, hash, repo)
  })
  .do((file) => debug(`wrote a new repo to json ${file}`))
  .toArray()

let reposSubscription = fileWriter$.subscribe(
  function (x) {
    debug('subscription')
  },
  function (err) {
     console.log('Error: %s', err)
  },
  function (e) {
     debug('Completed')
  });

The code looks really easy to read to me, so I think you did well, really!

Consider using const instead of let.
You use #debug() in some places and console#log() in other places. If you can use another parallel instance of #debug() to remove the #log(), you will have some more control over logging.

Maybe you could extract the side-effects calls in #do() to not be part of the main stream, and create a separate stream that you can activate and deactivate. This will be overkill for your code, but in a bigger app, it would probably be nice.

Well done anyhow!


Skiena's Programming Challenges: The Trip problem I have the following solution for the Trip problem from Skiena's and Revilla's Programming Challenges (full problem description here).

#include <stdio.h>

double truncate_to_2_decimals(double n)
{
    return (double)((int)(n*100.0)) / 100.0;
}

double mean(int n, double numbers[])
{
    double r;
    int i;
    for(i = 0; i < n; i++) {
        r += numbers[i];
    }
    return r / (double)n;
}

double min_exchange(int n, double expenses[])
{
    double r;
    double mu = mean(n, expenses);
    int i;
    for(i = 0; i < n; i++) {
        double e = expenses[i];
        if(e < mu) {
            r += mu - e;
        }
    }
    return truncate_to_2_decimals(r);
}

int main(int argc, char *argv[])
{
    int n_students;

    while(scanf("%d", &n_students) != EOF) {
        if(n_students == 0) {
            break;
        }
        double expenses[n_students];
        int i;
        for(i = 0; i < n_students; i++) {
            scanf("%lf", &expenses[i]);
        }
        printf("$%.2lf\n", min_exchange(n_students, expenses));
    }
    return 0;
}

The solution handles both the sample input and the following "tricky" input correctly:

4
9999.10
9999.10
9999.00
9999.10
3
1.01
0.99
0.99
0

Output:

$0.07
$0.01

Yet for some reason the online judge returns a "Wrong Answer" error. What could I be missing?

return (double)((int)(n*100.0)) / 100.0; fails for values of n greater than 2.147483648E7 as product n*100.0 would be larger than the maximum value of a 32-bit signed int. Use floor() instead. But that's probably not the reason the online judge fails it. **EDIT**problem size is restricted so this isn't an issue.
your mean() function is theoretically numerically instable, but that's probably not it either. EDIT definitely not it due to the problem description.
double expenses[n_students]; is not supported in C89 as it is a variable-length array. This feature was required in C99, but in the newer C11 it is an optional feature that compilers are not required to support. I suspect that your compiler supports variable-length arrays a la C99 but the online judge's compiler doesn't. Try using double * expenses = malloc(sizeof(double)*n_students); and changing other types as appropriate. Don't forget to free() your memory too. EDIT Checked the website and they say it supports "ANSI C". My guess is they mean C89.


Image Compression Using the Singular Value Decomposition (SVD) with MATLAB I'm new to both linear algebra and MATLAB.
I need help with this code with the objective of compressing image using the Singular Value Decomposition (SVD).
The code is currently working, if I run with e.g sigma_threshold = 10, I get a blurry picture, if I set it to 100 it's even more blurry etc.

I have searched all over the web and found a lot of similiar examples that I have used to improve this code with, but still I know I can do it a lot more elegant.

One problem is that I dont fully understand the math behind what I'm doing, and thus I don't know really know what to do, particularly with the matrices U and V in [U S V] = svd(A).

Wondered if someone could give me a hint how to improve the sections under the comments
"%Find the first index smaller than sigma_threshold" and "%Compose the red, green and blue channel again until the sigma value"

function svd_exercise(imagename,sigma_threshold)

image = imread(imagename);
image = im2double(image);

%Decompose the image in rgb values
R = image(:, :, 1);
G = image(:, :, 2);
B = image(:, :, 3);

%Compute the Single Value decomposition for each channel
[U_R, S_R, V_R] = svd(R);
[U_G, S_G, V_G] = svd(G);
[U_B, S_B, V_B] = svd(B);

%Find the first index smaller than sigma_threshold
[red_j, ~] = find(S_R > 0 & S_R < sigma_threshold, 1);
[green_j, ~] = find(S_G > 0 & S_G < sigma_threshold, 1);
[blue_j, ~] = find(S_B > 0 & S_B < sigma_threshold, 1);

%Compose the red, green and blue channel again until the sigma value
R_K = U_R(:, 1:red_j) * S_R(1:red_j, 1:red_j) * V_R(:, 1:red_j)';
G_K = U_G(:, 1:green_j) * S_G(1:green_j, 1:green_j) * V_G(:, 1:green_j)';
B_K = U_B(:, 1:blue_j) * S_B(1:blue_j, 1:blue_j) * V_B(:, 1:blue_j)';

AK = zeros(size(image));
AK(:,:,1) = R_K;
AK(:,:,2) = G_K;
AK(:,:,3) = B_K;

If you look at the structure of your matrices of singular values S_R, S_G and S_B, you'll see that the non-zero entries lie on the diagonal. You can check only these entries by using diag, removing the need for your S_R>0 condition.

This also means that for find you don't need to supply the second output argument.

To get the first non-zero entries less than sigma_threshold you can hence use

red_j = find(diag(S_R) < sigma_threshold, 1);
green_j = find(diag(S_G) < sigma_threshold, 1);
blue_j = find(diag(S_B) < sigma_threshold, 1);

Bear in mind that this is the first value which should be cut off, not the last value to include, hence you might want to do

red_j = find(diag(S_R) < sigma_threshold, 1) - 1;

or

red_j = find(diag(S_R) >= sigma_threshold, 1, 'last');


Python number guessing game I'm a PHP Developer. I'm trying to learn Python. So I tried to create a simple game, a Number Guesser.

I tried to comment out as much as possible without making super obvious comments.

I also made the attempts message color to be red.

Here's my code.

# 1. Print a message about the game
# 2. Tell the user that the random number is between some numbers
# 3. Run a while loop to continue the input, break the loop when the guess is right.
# 4`. When the user guessed the number, tell them that they're right.

import random

# Simple message to the user.
print "Welcome to the Number Guesser game."
print "I have a number in my mind that's between 1-20."

number = random.randint(0, 20)

# Guess the number
# >
prompt = "> "

# Start out as 5 and then decrement it on wrong guesses.
attempts = 5

while attempts > 0:
    print "What's your guess? "

    # Simple validation
    try:
        guess = int(raw_input(prompt))
    except ValueError:
        print "Only numbers are allowed. \n"
        break

    if guess > number:
        print "Go lower."
    elif guess < number:
        print "Go higher."
    else:
        print "Nice! You've guessed it right. The number was %d." % number
        print "And you got it with %d attempts!" % attempts
        # Exit.
        break

    # Decrement the attempts each time the guess isn't right.
    attempts -= 1

    if attempts == 0:
        print "You didn't guessed it, the number was %d. You lose." % number
        # Exit the script.
        break
    # Print with the color red.
    print("\033[91m You have {} guesses left. \033[00m").format(attempts)

number = random.randint(0, 20)

You just told the user that the number is between one and twenty, but you use random.randint(0, 20).  What if the result was 0?  One disgruntled player.  You should be using number = random.randint(1, 20).

prompt = "> "

You aren't hard-coding the value.  That's very good.  Since it is a constant, however, you should use ALLCAPS.

attempts = 5
while attempts > 0:

You are starting out at the high point and decrementing until you get to zero.  That isn't really the number of attempts.  That is how many attempts are left.  I would define a constant for the number of attempts allowed, define attempts as 0, and have the while make sure that attempts is lower than that number.  This becomes especially useful when printing how many attempts it took.  As it is, your program displays the wrong number: the number of attempts that were left.

try:
    guess = int(raw_input(prompt))
except ValueError:
    print "Only numbers are allowed. \n"
    break

It's good that you try to validate the input, but you should use continue instead of break.  One invalid input shouldn't end the program.

print "Nice! You've guessed it right. The number was %d." % number

Using % for a format string isn't officialy deprecated, but it is recommended to use .format().

What if the user wants to play again?  You make him re-run the script.  I would use a while loop and a function:

import random

PROMPT = "> "
GUESSES = 5

def guess_number(start=1, end=20):
    print "I have a number in my mind that's between {}-{}.".format(start, end)
    number = random.randint(start, end)
    attempts = 0
    while attempts < GUESSES:
        print "What's your guess? "

        # Simple validation
        try:
            guess = int(raw_input(prompt))
        except ValueError:
            print "Only numbers are allowed. \n"
            continue

        if guess > number:
            print "Go lower."
        elif guess < number:
            print "Go higher."
        else:
            print "Nice! You've guessed it right. The number was {:d}.".format(number)
            print "And you got it with {:d} attempts!".format(attempts + 1)
            break

        # Increment the attempts each time the guess isn't right.
        attempts += 1

        if attempts == GUESSES:
            print "You didn't guess it; the number was {:d}. You lose.".format(number)
            break
        # Print with the color red.
        print("\033[91m You have {} guesses left. \033[00m").format(attempts)

if __name__ == "__main__":
    print "Welcome to the Number Guesser game."

    again = True
    while again:
        print "I have a number in my mind that's between 1-20."
        guess_number()
        play_again = raw_input("Would you like to play again? ")
        while play_again.lower() not in "yn":
            play_again = raw_input("Invalid input. [y/n] ")
        again = (play_again.lower() == "y")


Implementing the Repository Pattern I'm new to this pattern and I see arguments online about different implementations. I have the following code and it seems fine to me, but I'm wondering if having to pass the DBContext into the constructor would cause any issues down the line? I'm using this with EntityFramework (RECORD_TYPE is a table brought in from EF).

I'm a little newer to unit testing as well, so I'm thinking I would have to make a mock context in my unit tests that basically return a list of RECORD_TYPE objects that I create in the mock DBContext I make.

Am I missing any ideas with this?

public interface IRepository<T> where T : class
{
    IQueryable<T> All { get; }
    T Find(int id);
    void Save();
}

public class Repository<T> : IRepository<T> where T : class
{
    private DbContext context;

    public Repository(DbContext c)
    {
        context = c;
    }

    public IQueryable<T> All { get { return context.Set<T>(); } }

    public T Find(int id)
    {
        return context.Set<T>().Find(id);
    }

    public void Save()
    {
        context.SaveChanges();
    }
}

public class RecordTypeRepository : Repository<RECORD_TYPE>
{
    public RecordTypeRepository(DbContext context)
        : base(context)
    {
    }
}

In my opinion using the repository pattern is absolutely fine, for a number of reasons, not limited to:

It enables unit testing
Allows you to change EF for a another DB technology

The detail that you have to be careful of using the base class is that you make sure that you declare properties/variables via the interface and not via the base class. So consider this:

Good

public class Test{
    private IRepository<MyClass> _repo;
}

Bad

public class Test{
    private BaseRepository<MyClass> _repo;
}

With the actual code, I would suggest a number of changes:

The interface should have full CRUD operations
Try and use the same name for fetching a record (Consider Get() and Get(id) vs All() and Get(1). It makes it more natural to consumers)
The class Repository should be marked as abstract to stop you using it directly
The class Repository should probably be named EFBaseRepository

Typically, when I implement the pattern I create the interface followed by a class called BaseRepository that implements all the methods. The methods no nothing more than throw a NotSupportedException/NotImplementedException. This is because I might only want consumers to have read/update but not create. It does depend on the context of the application.


Displaying contacts in PHP, with options to omit some information I am currently changing some PHP code. The old code looked like:

<div class="contact<?php echo $this->pageclass_sfx; ?>" itemscope itemtype="https://schema.org/Person">
 <?php if ($tparams->get('show_page_heading')) : ?>

   <?php echo $this->escape($tparams->get('page_heading')); ?>

 <?php endif; ?>

 <?php if ($this->contact->name && $tparams->get('show_name')) : ?>

    <?php if ($this->item->published == 0) : ?>
     <?php echo JText::_('JUNPUBLISHED'); ?>
    <?php endif; ?>
    <?php echo $this->contact->name; ?>

 <?php endif; ?>

 <?php $show_contact_category = $tparams->get('show_contact_category'); ?>

 <?php if ($show_contact_category === 'show_no_link') : ?>

   <?php echo $this->contact->category_title; ?>

 <?php elseif ($show_contact_category === 'show_with_link') : ?>
  <?php $contactLink = ContactHelperRoute::getCategoryRoute($this->contact->catid); ?>

   <a href="<?php echo $contactLink; ?>">
    <?php echo $this->escape($this->contact->category_title); ?>

 <?php endif; ?>

I found this quite hard to follow and decided to try and rewrite the same piece of code. It ended up looking like this:

<?php
echo '
pageclass_sfx . '" itemscope itemtype="https://schema.org/Person">';

// show/hide heading
if ($tparams->get('show_page_heading')){
 echo '
'. $this->escape($tparams->get('page_heading')) . '
';
}

// show/hide contact name
if ($this->contact->name && $tparams->get('show_name')){
 echo '
';
 echo '
';
 if ($this->item->published == 0){
  echo '' . JText::_('JUNPUBLISHED') . '';
 }
 echo ''. $this->contact->name . '';
 echo '
';
 echo '
';
} ?>

I think the logic is much easier to follow when written like this.

I was wondering if anyone had some opinions / remarks / feedback on improvements for the second version of the code.
If anyone sees some advantages of the first version over the second I would love to hear what they are.

The answers you'll get will be highly opinion-based, I guess. So, here a few points in favor of the first type of templating as the approach of inserting short PHP-elements into the template has some advantages.

Easier to read and follow

For a developer, the latter approach might be easier to follow. But often it's important to understand the structure and hierarchy of the markup. Your approach makes it quite hard:

echo '
'. $this->escape($tparams->get('page_heading')) . '
';
echo '
';
echo '
';
if ($this->item->published == 0){
    echo '' . JText::_('JUNPUBLISHED') . '';
}
echo ''. $this->contact->name . '';
echo '
';
echo '
';

It looks like everything is on one level and only the span-element is nested. This might be ok for a short passage like we see here. But consider a complex component with lots of nested elements.

Somebody who understands HTML can maintain the template

The first point brings us straight to this: A fronted-developer, who is not familiar with PHP can maintain and update your template. Taking the example from above, the person knows easily where to adjust the code, without breaking anything, if it would be written like this:

<?=$this->escape($tparams->get('page_heading'))?>

    <?=($this->item->published ? JText::_('JUNPUBLISHED') : '')?>
    <?=$this->contact->name?>

Also removing parent wrappers or inserting new elements is much easier.

Too much logic in the templates

I would say, that with the latter approach one might tend to include too much logic into the template. "Oh, I forgot to load the all products and categories from the database. Well, I'll just include it right here." And yes, this could happen with the first approach as well:

<?php elseif ($show_contact_category === 'show_with_link') : ?>
    <?php $contactLink = ContactHelperRoute::getCategoryRoute($this->contact->catid); ?>

This should happen somewhere in the controller or any other part before the template. However, I find it easier, to keep logic out of the presentation layer, if I'm filling and not creating a template.

Help from the IDE

Your IDE will find problems in the template, like unclosed elements, if you're actually writing HTML instead of creating strings containing HTML.

Switching to a different template engine or even language

At some point you might want to switch from PHP templating to a template engine  like Twig or Moustache. It's way easier when you have everything ready for this:

<?=$this->escape($tparams->get('page_heading'))?>

Can easily become this in Twig – as it automatically escapse strings:

{{ page_heading }}

Or if you switch to Moustache you aren't even coupled to PHP anymore. You could use your template with a lot of other languages. Or you might even want to include the template in a native APP in the future.


Setting controller, action, and values based on the number of URL chunks Is there any way to improve this code? It looks a bit ugly for me.

if($url_chunks_num == 1) {
    $this->controller = $this->pageData[0];
} elseif($url_chunks_num == 2) {
    $this->controller = $this->pageData[0];
    $this->action = $this->pageData[1];
} elseif($url_chunks_num > 2) {
    $this->controller = $this->pageData[0];
    $this->action = $this->pageData[1];
    $this->values = array_slice($this->pageData, 2);
}

I thought about nested conditions, but this option is ugly as well.

There seems to be some code repetition. Perhaps this could be improved like this:

if ($url_chunks_num >= 1)
   $this->controller = $this->pageData[0];
if ($url_chunks_num >= 2)
   $this->action = $this->pageData[1];
if ($url_chunks_num > 2)
   $this->values = array_slice($his->pageData, 2);

Or

if ($url_chunks_num >= 1) {
   $this->controller = $this->pageData[0];
   if ($url_chunks_num >= 2)
      $this->action = $this->pageData[1];
   if ($url_chunks_num > 2)
      $this->values = array_slice($his->pageData, 2);
}


2 Player Rock Paper Scissors (Python) I created a "2-player" rock-paper-scissors game in Python mainly using if statements and a single while loop. The program is really simple, but I was wondering if using switch statements or perhaps another format would make it less cluttered/quicker/etc.
again = 'y'

while (again == 'y'):

    p1 = input("Player 1 --> Rock, Paper, or Scissors? ")
    p1 = p1.lower()

    print()

    p2 = input("Player 2 --> Rock, Paper, or Scissors? ")
    p2 = p2.lower()

    print()

    if (p1 == "rock"):
        if (p2 == "rock"):
            print("The game is a draw")
        elif (p2 == "paper"):
            print("Player 2 wins!")
        elif (p2 == "scissors"):
            print("Player 1 wins!")
    elif (p1 == "paper"):
        if (p2 == "rock"):
            print("Player 1 wins!")
        elif (p2 == "paper"):
            print("The game is a draw")
        elif (p2 == "scissors"):
            print("Player 2 wins!")
    elif (p1 == "scissors"):
        if (p2 == "rock"):
            print("Player 2 wins!")
        elif (p2 == "paper"):
            print("Player 1 wins!")
        elif (p2 == "scissors"):
            print("The game is a draw")
    else:
        print("Invalid input, try again")

    again = input("Type y to play again, anything else to stop: ")

    print()

Remove Redundant Parenthesis
In this line:
while (again == 'y'):

The parenthesis are unneeded and can (should) be removed.
Use a dict to drive the stacked if/else
Whenever I see a stacked if/else I look to restructure the code using a dict (mapping) instead.
The net results of the if/else structure in your code can be put into a dict like so:
results = {
    "rock": {
        "rock": "The game is a draw",
        "paper": "Player 2 wins!",
        "scissors": "Player 1 wins!",
    },
    "paper": {
        "rock": "Player 1 wins!",
        "paper": "The game is a draw",
        "scissors": "Player 2 wins!",
    },
    "scissors": {
        "rock": "Player 2 wins!",
        "paper": "Player 1 wins!",
        "scissors": "The game is a draw",
    },
}

If the results are mapped this way, then the interactions with the user becomes more clear and
your loop can simply become:
while again == 'y':

    p1 = input("Player 1 --> Rock, Paper, or Scissors? ").lower()
    print()

    p2 = input("Player 2 --> Rock, Paper, or Scissors? ").lower()
    print()

    result = results.get(p1, {}).get(p2, "Invalid input, try again")
    print(result)

    again = input("Type y to play again, anything else to stop: ")
    print()

This is an example of separating the UI (User Interface) from the Business Logic.


Filter on HTML table using jQuery I am adding some check box filters on a HTML table, based on whether a check box is checked or not, and I am toggling rows.  Is there anyway I could improve this code?  There is no issue with it at the moment I could think of e.g. performance wise, but maybe someone could help me with decreasing number of lines.

HTML

                <asp:CheckBox ID="cbIncludeOpen" runat="server" ClientIDMode="Static" Checked="true" onclick="RMToggle()" />
                <asp:CheckBox ID="cbIncludeNew" runat="server" ClientIDMode="Static" onclick="RMToggle()" />
                <asp:CheckBox ID="cbIncludeClosed" runat="server" ClientIDMode="Static" onclick="RMToggle()" />
                <asp:CheckBox ID="cbIncludeRejected" runat="server" ClientIDMode="Static" onclick="RMToggle()" />

JS

function RMToggle() {
    var o = $("#cbIncludeOpen").is(':checked');
    var n = $("#cbIncludeNew").is(':checked');
    var c = $("#cbIncludeClosed").is(':checked');
    var r = $("#cbIncludeRejected").is(':checked');

    if (o) { $('#table1 tr[data-id="Open"]').show(); }
    else { $('#table1 tr[data-id="Open"]').hide(); }

    if (n) { $('#table1 tr[data-id="New"]').show(); }
    else { $('#table1 tr[data-id="New"]').hide(); }

    if (c) { $('#table1 tr[data-id="Closed"]').show(); }
    else { $('#table1 tr[data-id="Closed"]').hide(); }

    if (r) { $('#table1 tr[data-id="Rejected"]').show(); }
    else { $('#table1 tr[data-id="Rejected"]').hide(); }
}

Since you are executing the RMToggle function whenever a checkbox is selected, extend it a little by passing in the IDs too. (I've changed the IDs as well):

<div class="someRow" style="width: 100%">
    <asp:CheckBox ID="Open" runat="server" ClientIDMode="Static" Checked="true" onclick="RMToggle(this)" />
    <asp:CheckBox ID="New" runat="server" ClientIDMode="Static" onclick="RMToggle(this)" />
    <asp:CheckBox ID="Closed" runat="server" ClientIDMode="Static" onclick="RMToggle(this)" />
    <asp:CheckBox ID="Rejected" runat="server" ClientIDMode="Static" onclick="RMToggle(this)" />
</div>

Now, moving to the toggle function:

function RMToggle( elm ) { // elm = element
    var ischecked = elm.checked,
        $target = $('#table1 tr[data-id="' + elm.id + '"]');
    $target.toggle( ischecked );
}


Finding the sum of all the segment counts of a 7-segment display in a range I'm working on a programming challenge and the question is as follows:
Once Max found an electronic calculator from his grandfather Dovlet's chest. He noticed that the numbers were written with seven-segment indicators (https://en.wikipedia.org/wiki/Seven-segment_display).

Max starts to type all the values from a to b. After typing each number Max resets the calculator. Find the total number of segments printed on the calculator.
For example if a = 1 and b = 3 then at first the calculator will print 2 segments, then — 5 segments and at last it will print 5 segments. So the total number of printed segments is 12.
Input
The only line contains two integers a, b (1 ≤ a ≤ b ≤ 106) — the first and the last number typed by Max.
Output
Print the only integer a — the total number of printed segments.
Examples
input
1 3
output
12
input
10 15
output
39
Here's my attempt:
#include <iostream>
#include <unordered_map>

static std::unordered_map<char, int>
         segment_count = {
                            {'1', 2},
                            {'2', 5},
                            {'3', 5},
                            {'4', 4},
                            {'5', 5},
                            {'6', 6},
                            {'7', 3},
                            {'8', 7},
                            {'9', 6},
                            {'0', 6}
                         };

int get_number_of_segments(char num) {
  return segment_count[num];
}

int main() {
  int start, end, sum = 0;

  std::cin >> start >> end;

  for(int i = start; i <= end; ++i) {
    std::string current_num = std::to_string(i);
    for(auto c: current_num) {
      sum += get_number_of_segments(c);
    }
  }
  std::cout << sum << '\n';
}

When I submit the code, it fails on a timeout for a test case (unknown test case). How can I make it more efficient? All tests should run in under 1 second.
One idea I've had is to store the numbers not in the "one's place" - these won't change.
For instance: in the sequence 10, 11, 12, 13, 14, ..., I don't need to lookup 1 each time. I only need to look at the last digit until I see a 9.
Perhaps there are better ways to do this, though.
Any help would be appreciated!

Here are some things that may help you improve your program.  First, I'll mention a few things about the code you've already written, and then I'll describe an idea for a better algorithm.

Be careful with signed versus unsigned

By the problem description, neither start nor end can be less than 1, so they should probably be declared as unsigned rather than int.

Isolate calculation from I/O

The main routine does both the input and output and also is materially involved in the main calculation.  I'd advocate that that function should be isolated like this:

int segcount(unsigned start, unsigned end)
{
  unsigned sum = 0;
  for(auto i = start; i <= end; ++i) {
    std::string current_num = std::to_string(i);
    for(auto c: current_num) {
      sum += get_number_of_segments(c);
    }
  }
  return sum;
}

int main() {
  unsigned start, end;

  std::cin >> start >> end;
  auto sum = segcount(start, end);
  std::cout << sum << '\n';
}

Use const where practical

The segment_count map shouldn't be changed once constructed, and so it should be declared const.

Avoid global variables

The segment_count variable is only used within get_number_of_segments(), so it should go there instead of being a global.  I would also avoid the overhead of the std::unordered_map by using a simple array:

unsigned get_number_of_segments(char num) {
    static constexpr std::array<unsigned, 10> segment_count{
        6, 2, 5, 5, 4, 5, 6, 3, 7, 6
    };
    return segment_count[num-'0'];
}

A better algorithm

Your existing code, while not the fastest possible, does have a significant advantage in that it is obviously correct.  We can use that to verify any alternative approach as well as using it for timing comparisons.   From here to the end of this review, I'll be showing stepwise improvements in the code.

Write a test harness

We might write several versions of the code and want to compare them.  One nice way to do that is using a templated structure and a macro like this:

template <typename F>
struct testfunc {
    F *fn;
    const char *name;
};

#define TEST(x) { x, #x }

Now we can easily make an array of tests and run through all alternative algorithms:

#include "stopwatch.h"
int main()
{
    const testfunc<decltype(segcount)> test[]{
        TEST(segcount),
        TEST(segcount2),
    };

    unsigned start, end, sum;

    std::cin >> start >> end;
    for (const auto t : test) {
        Stopwatch<std::chrono::milliseconds> sw;
        sum = t.fn(start, end);
        std::cout << t.name << ": " << sum << ", " << sw.stop() << "ms\n";
    }
}

Note that this uses my Stopwatch template, but one could use an alternative if desired.

Think carefully about the problem

First, let's consider a range of single digit numbers.  If we go from 1 to 5, for example, we simply add up the segment counts for each of those digits \$= 2+5+5+4+5 = 21\$.  For a range that doesn't start from 1, such as for 3 to 5, we can calculate it one of two ways.  We could calculate just for those numbers \$= 5+4+5 = 14\$ or we could calculate 1 to 5 and subtract 1 to 2.  The motivation for doing it by the latter method is that we can precalculate all possible ranges for a digit and then simply do two lookups and a subtraction.

We can now extend this to look at two-digit numbers.  If we have a range in which the first digit doesn't change, such as 71 to 75 it's simply the single digit range 1 to 5 (which we already calculated above is 21) plus the size of the range (5) times the segments for the first digit (3), so the answer is \$21 + 5(3) = 36\$.

If it's a two-digit number for which the first digit does change, we can break it down into a sum of two-digit ranges for which the leading digit is constant.  For example, if we go from 71 to 88, we can consider it as two subranges: 71 to 79 and 80 to 88 where the algorithm above could be applied.

I think you can probably take it from there and see how to extend the algorithm generically.


Solution to a simple Hackerrank problem: Compare the Triplets Given two equally sized arrays of integers A and B, compute the number of times A[i] > B[i] and the number of times A[i] < B[i] for each index i.

New to scala, I put together the following solution:

import scala.io.Source

object Solution {

    def main(args: Array[String]) {
        val lines = Source.stdin.getLines
        val alice = lines.next.split(" ").map(_.toInt)
        val bob = lines.next.split(" ").map(_.toInt)

        val competitions = alice zip bob
        val scoreA = competitions.count(c => c._1 > c._2)
        val scoreB = competitions.count(c => c._1 < c._2)

        printf("%d %d", scoreA, scoreB)
    }
}

I normally solve these problems using python, so I simply searched online for my usual programming constructs. I have a few questions.

Is there a more general way to print a list of space separated integers? Currently my use of printf is limited to exactly two.
Are there any pitfalls I'm unaware of?
Ideas of more concise solutions without sacrificing readability?

Any other insights would be greatly appreciated!

You don't have to traverse the competitions array twice. The results can be collected in a tuple result after a single traversal.

val results = competitions.foldLeft((0,0)){
                             case ((aw,bw),(a,b)) =>
                               if (a>b)      (aw+1,bw) //a won
                               else if (b>a) (aw,bw+1) //b won
                               else          (aw,bw)   //tie
                           }

The tuple's elements can be accessed via indexing, results._1 and results._2, but it can be more convenient to unpack it via pattern matching.

val (scoreA, scoreB) = results

So, putting it all together, and removing the competitions step (just because you can).

val (aWins
    ,bWins) = alice.zip(bob)
                   .foldLeft((0,0)){
                      case ((aw,bw),(a,b)) =>
                        if (a>b)      (aw+1,bw)
                        else if (b>a) (aw,bw+1)
                        else          (aw,bw)
                   }

Sending the results to STDOUT is usually done via println(),  either with string catenation...

println(aWins + " " + bWins)

...or with string interpolation.

println(s"$aWins $bWins")


Hash and check passwords in node.js using the native pbkdf2 I use the native crypto . I use the pbkdf2 and the randomBytes for salting, and the timingSafeEqual to check for the password validity when logging in.

I wrote the following functions, based on various examples and the aforementioned APIs and functions. Here is my code

(stack : node 8.11.1 + express 4.16.3 + PostgreSQL 10)

const crypto = require('crypto');

var config = {
  hashBytes: 32,
  saltBytes: 16,
  iterations: 10000,
  digest : 'SHA512'
};

const  hashPassword = (password) =>{
  return new Promise((resolved, rejeted)=>{
    crypto.randomBytes(config.saltBytes, (err, salt) =>{
      if (err){return rejeted('Error while sign in');}
      salt = salt.toString('hex');
      crypto.pbkdf2(password, salt, config.iterations, config.hashBytes, config.digest,(err, hash) =>{
        if (err) {return rejeted('Error while sign in');}
        hash = hash.toString('hex');
        const combined = [salt, hash, config.iterations, config.hashBytes, config.digest].join('$');
        resolved(combined);
      });//closes pbkdf2
    });//closes randomBytes
  })//closes Promise
}//closes hashPassword

const  verifyPassword = (password,original) =>{
  return new Promise((resolved, rejeted)=>{
    let hash = original.split('$')[1];
    let salt = original.split('$')[0];
    let iterations = Number(original.split('$')[2]);
    let hashBytes = Number(original.split('$')[3]);
    let digest = original.split('$')[4];
    hash = Buffer.from(hash, 'hex');

    crypto.pbkdf2(password, salt, iterations, hashBytes, digest, (err, verify) =>{
      if (err){return rejeted('Error while logging in');}
      let equals = crypto.timingSafeEqual(hash, verify);
      resolved(equals);
    });
  })//closes Promise
}//closes verifyPassword

exports.hashPassword = hashPassword;
exports.verifyPassword = verifyPassword;

This works, but, here is what bothers me :

Is it ok if I save the combined from the hashPassword as text in
the DB? (column type text)
I have to convert from text back in Buffer in the verifyPassword
-- hash = Buffer.from(hash, 'hex'); part (that's because timingSafeEqual only accepts buffer). Is this ok? Does it take a lot of time?
Is using text ok, or should I use and save buffer for this?

Any suggestions? Thank you

Is it ok if I save the combined from the hashPassword as text in
  the DB? (column type text)

Yes, that's OK, if you use this to store password hashes. If you use it as a (encryption) key then you should avoid text, as it can be hard to destroy the result.

  I have to convert from text back in Buffer in the verifyPassword
  -- hash = Buffer.from(hash, 'hex'); part (timingSafeEqual only accepts buffer). Is this ok? Does it take a lot of time?

That's OK. Compared to PBKDF2 almost nothing takes a lot of time.

  Is using text ok, or should I use and save buffer for this?

No text is OK, see question #1.

With regards to the code, some notes:

Calling split multiple times is not a good idea, call it once and store the intermediate result.
As there are no checks on the results after the split, the hash string representation could be altered without notice (impact depends on how the code is used).
You could use just a counter to retrieve the various parts after split, and at least create the variables in order (e.g. salt before hash) - storing the hash last makes most sense to me.

I was afraid that you were implementing PBKDF2 yourself, but you seem to be correctly using the proper crypto calls.

A different idea of handling this (for you to ponder over).

Define your protocol somewhere and store a protocol version in your hash string. You could even use that to replace the salt size, iterations, hash type etc.. Then if you choose a higher iteration count you could just update your protocol version.


Kernel Convolution in Python 2.7 The code I wrote performs a mean blur on an image (I hardcoded it as zebra.jpg for testing purposes).
My problem is that for an image of 39KB image it take minutes to perform, is there any way of making this code more efficient?
Preferably using built in python modules.
Also is there any way of improving my code in other ways?

from PIL import Image
img = Image.open('zebra.jpg')

img_w = img.size[0]
img_h = img.size[1]

# The bigger the kernel size, the more intense the blur
kernel = [[1]*10]*10

outputIm = Image.new("RGB", (img_w, img_h))
d = []
for y in range(0, int(img_h)):
    for x in range(0, int(img_w)):
        r, g, b, count = 0, 0, 0, 0
        index_y = int((len(kernel[0]) - 1) / 2.0) * -1

        for kernel_offset_y in kernel:
            index_x = int((len(kernel_offset_y) - 1) / 2.0) * -1

            for kernel_val in kernel_offset_y:

                if img_w > x + index_x+1 > 0 and img_h > y + index_y+1 > 0:
                    temp_r, temp_g, temp_b = img.getpixel((int(x + index_x), int(y + index_y)))
                    r += temp_r * kernel_val
                    g += temp_g * kernel_val
                    b += temp_b * kernel_val
                    count += 1
                    index_x += 1
            index_y += 1

        if (r > 0):
            r = r / count
        if (g > 0):
            g = g / count
        if (b > 0):
            b = b / count

        d.append((r,g,b))

outputIm.putdata([tuple(pixel) for pixel in d])
outputIm.save('blurred.jpg')

Unnecessary type conversions

There are a couple of unnecessary type conversions, for example:

index_y = int((len(kernel[0]) - 1) / 2.0) * -1

Casting to int truncates, but so does integer division.
So you can simplify the above with:

index_y = -(len(kernel_offset_y) - 1) // 2

In Python 2 integer division truncates by default,
so ... / 2 would work too.
But not in Python 3, where the behavior was changed.
Using // works in Python 2.7 and Python 3 consistently.

I also replaced the * -1 with a simpler - prefix.

On this line, x, index_x, y and index_y are all integers,
so no need to convert them to int:

temp_r, temp_g, temp_b = img.getpixel((int(x + index_x), int(y + index_y)))

This is also unnecessary:

outputIm.putdata([tuple(pixel) for pixel in d])

Since d is already a list of tuples, you could write simply:

outputIm.putdata(d)

Simplify

The default start value of range is 0, so instead of range(0, k) you could write range(k).

Instead of x = x / y you could write x /= y.

The parentheses are unnecessary in if (expr):.

Formatting

Instead of this:

kernel = [[1]*10]*10

PEP8 recommends this writing style:

kernel = [[1] * 10] * 10


Word Search in LeetCode I solved this programming challenge:

  Given a 2D board and a word, find if the word exists in the grid.

  The word can be constructed from letters of sequentially adjacent
  cell, where "adjacent" cells are those horizontally or vertically
  neighboring. The same letter cell may not be used more than once.

  For example,

  Given board =

[
  ['A','B','C','E'],
  ['S','F','C','S'],
  ['A','D','E','E']
]

  word = "ABCCED", -> returns true

  word = "SEE", -> returns true

  word = "ABCB", -> returns false

class Solution {
public:
  bool DFS(vector<vector<char>> &board, string word,
           vector<vector<bool>> visited, int i, int j, int curr) {
    if (i < 0 || j < 0 || i >= board.size() || j >= board[0].size()) {
      return false;
    }
    if (visited[i][j] || board[i][j] != word[curr]) {
      return false;
    }
    visited[i][j] = true;
    ++curr;
    if (curr == word.size()) {
      return true;
    }

    return DFS(board, word, visited, i + 1, j, curr) || // Down
           DFS(board, word, visited, i, j + 1, curr) || // Right
           DFS(board, word, visited, i - 1, j, curr) || // Up
           DFS(board, word, visited, i, j - 1, curr);   // Left
  }

  bool exist(vector<vector<char>> &board, string word) {
    for (int i = 0; i < board.size(); ++i) {
      for (int j = 0; j < board[i].size(); ++j) {
        if (word[0] == board[i][j]) {
          vector<vector<bool>> visited(board.size(),
                                       vector<bool>(board[0].size(), false));
          if (DFS(board, word, visited, i, j, 0)) {
            return true;
          }
        }
      }
    }
    return false;
  }
};

My solution ranks at around 2% faster when compared to other solutions with the same language. My main desire from reviews are performance improvements.

In 'exist' function, instead of looping through the entire board every time for the first character match, you can build a lookup map with char vs it's indices in  the board. I modified your exist method as below and I could see better performance.

#include <iostream>
#include <vector>
#include <map>

using namespace std;

class Solution {
  public:
   bool DFS(vector<vector<char>> &board, string word, vector<vector<bool>> visited, int i, int j, int curr)
   {
          if (i < 0 || j < 0 || i >= board.size() || j >= board[0].size()) {
              return false;
          }
          if (visited[i][j] || board[i][j] != word[curr]) {
             return false;
          }
          visited[i][j] = true;
          ++curr;
          if (curr == word.size()) {
             return true;
          }

          return DFS(board, word, visited, i + 1, j, curr) || // Down
                 DFS(board, word, visited, i, j + 1, curr) || // Right
                 DFS(board, word, visited, i - 1, j, curr) || // Up
                 DFS(board, word, visited, i, j - 1, curr);   // Left
    }

    void buildFirstCharIndexMap(const vector<vector<char>> &board)
    {
         firstCharIndex.clear();
         for (int i=0; i<board.size(); ++i){
             for (int j=0; j<board[i].size(); ++j){
                  firstCharIndex.emplace(make_pair(board[i][j], make_pair(i,j)));
             }
         }
    }

    bool exist (vector<vector<char>> &board, string word) {
        // get the indices for the first character.
        auto range_itr = firstCharIndex.equal_range(word[0]);

        for (auto it = range_itr.first; it != range_itr.second; ++it){
             auto cur_index_pair = it->second;

              vector<vector<bool>> visited(board.size(), vector<bool>(board[0].size(), false));
              if (DFS(board, word, visited, cur_index_pair.first, cur_index_pair.second, 0)) {
                    return true;
              }
         }
         return false;
     }
  private:
     // lookup table for each char and it's location in the board
     multimap<char, pair<int, int>> firstCharIndex;
  };

  int main() {
       vector<vector <char>> board = {
                                  {'A','B','C','E'},
                                  {'S','F','C','S'},
                                  {'A','D','E','E'}
                                };
       Solution s;
       // build the look up table
       s.buildFirstCharIndexMap(board);
       cout << s.exist(board, "SEE") << endl;
       cout << s.exist(board, "ABCCED") << endl;
       cout << s.exist(board, "ADCB") << endl;
       return 0;
    }


Dataset with optional addition to where statement I am trying to generate a dataset from a query that has a where statement. If the user passes a value additional things need to be added to the where statement. Is this the best way to go about this?

class TableAdapters
{
    public class FindScriptByTargetVDN
    {
        public static DataSet Fill(SqlConnection connection, string parameter)
        {
            string conditional = string.Empty;
            if (parameter.Length > 0)
            {
                conditional = "and svce.PeripheralNumber= " + parameter + " ";
            }
            string query = "select svce.PeripheralNumber AS Service, "
                + "ms.EnterpriseName AS [Script Name], "
                + "p.EnterpriseName AS Peripheral, "
                + "sg.PeripheralNumber AS [Skill Mapping], "
                + "s.Version AS [Latest Version], "
                + "s.DateTime AS Created, "
                + "s.Author "
                + "from Script_Cross_Reference scr "
                + "LEFT OUTER JOIN Script s on s.ScriptID = scr.ScriptID "
                + "LEFT OUTER JOIN Master_Script ms on s.MasterScriptID = ms.MasterScriptID "
                + "LEFT OUTER JOIN Service svce on svce.SkillTargetID = scr.ForeignKey "
                + "LEFT OUTER JOIN Peripheral p on svce.PeripheralID = p.PeripheralID "
                + "LEFT OUTER JOIN Service_Member sm on svce.SkillTargetID = sm.ServiceSkillTargetID "
                + "LEFT OUTER JOIN Skill_Group sg on sg.SkillTargetID = sm.SkillGroupSkillTargetID "
                + "where s.Version = ms.CurrentVersion and scr.TargetType = 1 "
                + conditional
                + "order by svce.PeripheralNumber, ms.EnterpriseName, p.EnterpriseName";
            SqlDataAdapter adapter = new SqlDataAdapter(query, connection);
            DataSet dataSet = new DataSet();
            dataSet.Tables.Add(new DataTable());
            adapter.Fill(dataSet.Tables[0]);
            return dataSet;
        }
    }
}

Here is an example of the call:

this.FindScriptsDataGrid.DataSource = TableAdapters.FindScriptByTargetVDN.Fill(connection, this.ValueTextBox.Text).Tables[0];

The connection is defined before the call. The reason for the some what round about method is the user has the option of defining what server they want to run this query on before they run it.

This:

+ "where s.Version = ms.CurrentVersion and scr.TargetType = 1 "
+ conditional
+ "order by svce.PeripheralNumber, ms.EnterpriseName, p.EnterpriseName";

Is poison.

conditional came from a string concatenation that involves a parameter string that the method cannot trust:

conditional = "and svce.PeripheralNumber= " + parameter.ToString() + " ";

..Uh, the ToString() call is superfluous here, parameter is already a string! ;)

Looking at the code in your other question, this is where the parameter came from:

string value = this.ValueTextBox.Text;

That is what you shouldn't be doing: concatenating user input into an SQL statement is asking for trouble, whether it's a web or a desktop application doesn't matter, SQL Injection is SQL injection. Not to mention that you just put the calling code in charge of knowing the types for all columns that might be used, whether or not the values should be enclosed in single quotes, etc.

You need to use a parameterized query instead.


Notify observers when object attributes update I have a Singleton manager class for managing request model and its attributes.

My main aim is update object observers when some of the object attributes changes.

class FlightSearchManager {

    private val _flightSearchRequestModelLiveData : MutableLiveData<FlightSearchRequestModel> = MutableLiveData()
    val flightSearchRequestModelLiveData : LiveData<FlightSearchRequestModel> get() = _flightSearchRequestModelLiveData

    var flightSearchRequestModel : FlightSearchRequestModel

    init {
        flightSearchRequestModel =
            FlightSearchRequestModel(
                null,
                null,
                null,
                null,
                null,
                null,
                null,
                null,
                null)
    }

    fun updateFlightSearchRequestModel(_arriveAirport : String? = null,
                                       _arriveAirportId: Int? = null,
                                       _departureAirport : String? = null,
                                       _departureAirportId: Int? = null,
                                       _departureDate : String? = null,
                                       _isRoundTrip : Boolean? = false,
                                       _paxTypeCount : List<PaxTypeCount>? = null,
                                       _returnDate : String? = null,
                                       _seatClass : String? = null) {
        flightSearchRequestModel.apply {
            arriveAirport = _arriveAirport ?: arriveAirport
            arriveAirportId = _arriveAirportId ?: arriveAirportId
            departureAirport = _departureAirport ?: departureAirport
            departureAirportId = _departureAirportId ?: departureAirportId
            departureDate = _departureDate ?: departureDate
            isRoundTrip = _isRoundTrip ?: isRoundTrip
            paxTypeCount = _paxTypeCount ?: paxTypeCount
            returnDate = _returnDate ?: returnDate
            seatClass = _seatClass ?: seatClass
            // Notify all listeners of live data
            _flightSearchRequestModelLiveData.value = this
        }
    }
}

When I want to notify observers I call:

flightSearchManager.updateFlightSearchRequestModel(_seatClass = "1")

This solution does not seem good solution. How can I write it more intelligently?

To give you valuable feedback on your usecase, I would need more information, like: How do listeners listen? How does the caller look like?

How exactly does _flightSearchRequestModelLiveData.value = this notify all listeners of live data? Its not a function call, so how is an event created and pushed?!

Instead I will give some pattern advices you may need.

Use constructor initialisation

Kotlin enables you to set fields over constructors. Even if your class is a singleton and all values are default - creating a constructor will enable to test your class easily (and other benefits).

So instead of:

class FlightSearchManager {

    private val _flightSearchRequestModelLiveData : MutableLiveData<FlightSearchRequestModel> = MutableLiveData()
    val flightSearchRequestModelLiveData : LiveData<FlightSearchRequestModel> get() = _flightSearchRequestModelLiveData

    var flightSearchRequestModel : FlightSearchRequestModel

try to go for:

class FlightSearchManager(
    val flightSearchRequestModelLiveData : MutableLiveData<FlightSearchRequestModel> = MutableLiveData(),
    var flightSearchRequestModel : FlightSearchRequestModel = FlightSearchRequestModel()
) {

If you already have a constructor for a class and it is valid to have all fields to have default values - don't force the caller to set them!

This code is an anti-pattern:

flightSearchRequestModel =
            FlightSearchRequestModel(
                null,
                null,
                null,
                null,
                null,
                null,
                null,
                null,
                null)

What you should go for would be:

data class FlightSearchRequestModel(
   val propertyOne : String? = null,
   val propertyTwo: Int? = null,
   ...
)

FlightSearchManager {
  val model = FlightSearchRequestModel()
}

Parameters

It is seen as code smell, when your method has too many parameters, so I would recommend to reduce the number of those in fun updateFlightSearchRequestModel.

You could create a single function for every parameter change. Of course it means more lines to write, but is cleaner, straight forward and not as error-prone to side effects since you don't change ~20 things in one method.

Naming convetions

I would guess that you coded with something like C# before, because of the usage of underscores _ in variable names. Please try to follow the appropreate convetion for this language (Kotlin), as well as in Java:

  Local variables, instance variables, and class variables are also written in lowerCamelCase. Variable names should not start with underscore (_) or dollar sign ($) characters, even though both are allowed. This is in contrast to other coding conventions that state that underscores should be used to prefix all instance variables.

Constants:

  Constants should be written in uppercase characters separated by underscores. Constant names may also contain digits if appropriate, but not as the first character.

Source

Fields like this:

private val _flightSearchRequestModelLiveData : MutableLiveData<FlightSearchRequestModel> = MutableLiveData()

would become:

private val flightSearchRequestModelLiveData : MutableLiveData<FlightSearchRequestModel> = MutableLiveData()

Functions like this

fun updateFlightSearchRequestModel(_arriveAirport : String? = null,
                                   _arriveAirportId: Int? = null,

would become:

fun updateFlightSearchRequestModel(arriveAirport : String? = null,
                                   arriveAirportId: Int? = null,

Immutability

Immutable code is easier to debug, to understand and to test. Favor val over var in (almost) every case.

If you really need to change something by reference, at least create a completely new immutable object. Example:

This

flightSearchRequestModel.apply {
            arriveAirport = _arriveAirport ?: arriveAirport
            arriveAirportId = _arriveAirportId ?: arriveAirportId
            ...
}

could become this:

flightSearchRequestModel = copy(arriveAirport = arriveAirport, arriveAirportId  = arriveAirportId, ...)


new Class().Function() pattern I am maintaining an application and have seen something like this

Permission p = new UserModel().GetPermission(userToTestPerrmission, permissionWeCheck);
if(p.CanChange)
   //sometihing
else
   //something else

This approach looks a little "dirty" to me. The part I have problems with is this new UserModel().GetPermission(). Is this a typical way to write something like this or should it be like:

var model = new UserModel();
model.GetPermissions(...);
...

Is this a typical way to write something like

If that’s all the information that is required – yes, why not? I see no problem at all with this approach.

On the other hand, if code lik this is prolific this is a sure sign that the original class design is broken. But from your code it looks like this is outside of your control anyway, since UserModel is a framework class.

In general, if a class is used in such a fashion then it is over-engineered: a static method could have been used just as well (on the other hand, this may make unit testing harder because it prevents mocking).


Part 1: Create or update record via HTTP request Update:
This is an older version of the question/script. The new version can be found here: Part 2: Create or update record via HTTP request

I have an external system that sends an HTTP request to a Jython script (in IBM's Maximo Asset Management platform).
The Jython 2.7.0 script does this:

Accepts an HTTP request: http://server:port/maximo/oslc/script/CREATEWO?_lid=wilson&_lpwd=wilson&f_wonum=LWO0382&f_description=LEGACY WO&f_classstructureid=1666&f_status=APPR&f_wopriority=1&f_assetnum=LA1234&f_worktype=CM
Loops through parameters:

Searches for parameters that are prefixed with f_ ('f' is for field-value)
Puts the parameters in a list
Removes the prefix from the list values (so that the parameter names match the database field names).

Updates or creates records via the parameters in the list:

If there is an existing record in the system with the same work order number, then the script updates the exiting record with the parameter values from the list.
If there isn't an existing record, then a new record is created (again, from the parameter values from the list).

Finishes by returning a message to the external system (message: updated, created, or other (aka an error)).

Can the script be improved?

from psdi.mbo import SqlFormat
from psdi.server import MXServer
from psdi.mbo import MboSet

params = list( param for param in request.getQueryParams() if param.startswith('f_') )
paramdict={}
resp=''
for p in params:
    paramdict[p[2:]]=request.getQueryParam(p)

woset = MXServer.getMXServer().getMboSet("workorder",request.getUserInfo())

#Prevents SQL injection
sqf = SqlFormat("wonum=:1")
sqf.setObject(1,"WORKORDER","WONUM",request.getQueryParam("f_wonum"))

woset.setWhere(sqf.format())

woset.reset()
woMbo = woset.moveFirst()

if woMbo is not None:
    for k,v in paramdict.items():
        woMbo.setValue(k,v,2L)
    resp = 'Updated workorder ' + request.getQueryParam("f_wonum")
    woset.save()
    woset.clear()
    woset.close()
else:
    woMbo=woset.add()
    for k,v in paramdict.items():
        woMbo.setValue(k,v,2L)
    resp = 'Created workorder ' + request.getQueryParam("f_wonum")
    woset.save()
    woset.clear()
    woset.close()
responseBody = resp

Note 1: Previously, there was an SQL injection vulnerability in the code. This issue has been resolved via the SQLFormat java class. The code has been updated.
Note 2: Unfortunately, I'm not able to import Python 2.7.0 libraries into my Jython implementation. In fact, I don't even have access to all of the standard python libraries.
Note 3: The acronym 'MBO' stands for 'Master Business Object' (it's an IBM thing). For the purpose of this question, a Master Business Object can be thought of as a work order record. Additionally, the constant 2L tells the system to override any MBO rules/constraints.

Can the script be improved?

Jython is somewhat of a horror show, so improvements are limited but still possible.
List comprehensions
params = list( param for param in request.getQueryParams() if param.startswith('f_') )

can be
params = [
    param for param in request.getQueryParams()
    if param.startswith('f_')
]

Dict comprehensions
This just squeaked into Python 2.7:
paramdict = {
    p[2:]: request.getQueryParam(p)
    for p in params
}

Factor out common code
Factor out common code from these blocks:
if woMbo is not None:
    for k,v in paramdict.items():
        woMbo.setValue(k,v,2L)
    resp = 'Updated workorder ' + request.getQueryParam("f_wonum")
    woset.save()
    woset.clear()
    woset.close()
else:
    woMbo=woset.add()
    for k,v in paramdict.items():
        woMbo.setValue(k,v,2L)
    resp = 'Created workorder ' + request.getQueryParam("f_wonum")
    woset.save()
    woset.clear()
    woset.close()

For example,
if woMbo is None:
    woMbo=woset.add()
    verb = 'Created'
else:
    verb = 'Updated'

for k,v in paramdict.items():
    woMbo.setValue(k,v,2L)
resp = verb + ' workorder ' + request.getQueryParam("f_wonum")
woset.save()
woset.clear()
woset.close()

Guaranteed closure
Wrap your code in a try/finally:
woset = MXServer.getMXServer().getMboSet("workorder",request.getUserInfo())
try:
    # ...
finally:
    woset.close()

Named constants

the constant 2L tells the system to override any MBO rules/constraints

Fine; so it should get first-class treatment:
IGNORE_RULES = 2L


Displaying current date and time using JavaScript and HTML time tag I am creating  a simple support ticket system. I thought It would be a nice little feature to show the current date and time. Since I want to show the current time I thought JavaScript would be best versus using PHP. I have looked at different questions posted by other user about the topic and created the following snippet. My question is if this is a efficient way to get and display the date and time using JavaScript date object. I also thought is would be appropriate to use the <time> tag. I also ask if I have used it in the correct form.

JavaScript and HTML

var monthName = new Array('January', 'Febuary', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December');
var hourap = new Array(12, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11);
function showTime(){
    var dateObj = new Date();
    var day = dateObj.getDate(), month = dateObj.getMonth(), year = dateObj.getFullYear(), hour = dateObj.getHours(), minutes = (dateObj.getMinutes()<=9?'0'+dateObj.getMinutes():dateObj.getMinutes());
    var string  = monthName[month]+
    ' '+day+
    ', '+year+
    '
'+hourap[hour]+
    ':'+minutes+
    ' '+(hour<=11?'am':'pm');
    var timeDiv = document.getElementById('time');
    if(timeDiv !== null) {
        timeDiv.innerHTML = string;
        timeDiv.setAttribute('datetime',year+'-'+(month+1<=9?'0'+(month+1):month+1)+'-'+day+' '+hour+':'+minutes);

    };
};
setInterval(showTime,1000);
<time id="time"></time>

HTML output

<time id="time" datetime="2015-01-30 22:00">January 30, 2015
10:00 pm</time>

Any suggestions or comments would be appreciated. Please note that I wish to script it using pure JavaScript and not jQuery.

Semantic markup is less important for dynamically generated HTML than it is for server-generated HTML, but it is a laudable goal nonetheless.  If you're going to produce a datetime attribute for machine consumption, then I suggest using T as the delimiter between the date and time, as recommended by ISO 8601.

Since you frequently need to zero-pad a number, you should define a function for it.

For clarity, you should avoid excessively long lines of code.  In addition, I think it would be a good idea to define separate functions for each of the datetime formats.

I would eliminate the hourap array, since it's not that difficult to achieve the same effect programmatically.

The timeDiv variable refers to a <time> element, not a <div>, and should therefore be renamed.

The semicolon after the final if block is unusual.

var MONTH_NAME = ['January', 'Febuary', 'March', 'April', 'May', 'June',
                  'July', 'August', 'September', 'October', 'November', 'December'];
function showTime() {
    function twoDigit(n) {
        return ('0' + n).slice(-2);
    }
    function iso8601(date) {
        return date.getFullYear() +
               '-' + twoDigit(1 + date.getMonth()) +
               '-' + twoDigit(date.getDate()) +
               'T' + twoDigit(date.getHours()) +
               ':' + twoDigit(date.getMinutes());
    }
    function en_US(date) {
        var h = date.getHours() % 12;
        return MONTH_NAME[date.getMonth()] +
               ' '  + date.getDate() +
               ', ' + date.getFullYear() +
               '<br />' + (h == 0 ? 12 : h) +
               ':'  + twoDigit(date.getMinutes()) +
               ' ' + (date.getHours() < 12 ? 'am' : 'pm');
    }

    var timeEl = document.getElementById('time');
    if (timeEl !== null) {
        var now = new Date();
        timeEl.innerHTML = en_US(now);
        timeDiv.setAttribute('datetime', iso8601(now));
    }
};
setInterval(showTime, 1000);
<time id="time"></time>


Pinging the user requested host - is this code insecure? <?php
    $userinput = $_GET['host'];
    $e = escapeshellcmd($userinput);
    $arr = (explode(".",$e));
    $num = count($arr);
    $times = (int)$_GET['times'];
    $time = (range(1,51));
    if (!isset($time[$times])){
        $times = 5;
    }
    function isValidURL($url){
        return preg_match('/(www\.)?(.)*[\.](.)*$/i', $url);
    }
    if($num == "2"){
        if(isValidURL($e)){
            echo"
", passthru("ping -n $times $e"),"
";}
        else {
            echo "The URL or IP $e isn't valid
";
    }
?>

This code is pinging the user requested host. Is this code insecure because I am using the  passthru() function?

I am using explode to know if it's domain.com or just text and a regular expression to validate its not texttext.text. Is there way for the user to access my terminal with this?

Your regular expression for detecting valid urls is very weak and you can slip a lot past it.

Though you have used escapeshellcmd() to protect yourself a bit from people trying to run alternative commands you have not protected yourself from people sending extra parameters to ping.

Two alternative attacks spring to mind:

You can overload your servers by making them send lots of useless packets in the ping requests.
You can use your servers as the host for an attack on somebody else.

An example of sending extra bytes with ping (I put your script in the file test.php on myhost (not real name).

http://myhost.com/test.php?host=plop.com%20-s%20500

Generates:

ping -n 0 plop.com -s 500


`atof` implementation I have implemented the atof function. Here is my implementation

double atof(const char *str)
{
    double a;           /* the a value in a*10^b */
    double decplace;    /* number to divide by if decimal point is seen */
    double b;           /* The b value (exponent) in a*10^b */

    int sign = 1;       /* stores the sign of a */
    int bsign = 1;      /* stores the sign of b */

    while (*str && isspace(*str))
        ++str;

    if (*str == '+')
        ++str;
    if (*str == '-') {
        sign = -1;
        ++str;
    }
    if ((*str == 'n' || *str == 'N') &&
       (str[1] == 'a' || str[1] == 'A')
       && (str[2] == 'n' || str[2] == 'N'))
            return NAN * sign;
    if ((*str == 'i' || *str == 'I') && (str[1] == 'n' || str[1] == 'N') &&
        (str[2] == 'f' || str[2] == 'F'))
              return INFINITY * sign;

    for (a = 0; *str && isdigit(*str); ++str)
        a = a * 10 + (*str - '0');

    if (*str == '.')
        ++str;
    for (decplace = 1.; *str && isdigit(*str); ++str, decplace *= 10.)
        a = a * 10. + (*str - '0');

    if (*str == 'e' || *str == 'E') {
        /* if the user types a string starting from e, make the base be 1 */
        if (a == 0)
            a = 1;
        ++str;
        if (*str == '-') {
            bsign = -1;
            ++str;
        }
        if (*str == '+')
            ++str;
        for (b = 0; *str && isdigit(*str); ++str)
            b = b * 10 + (*str - '0');

        b *= bsign;
    }
    else
        b = 0;

    return (a * sign / decplace) * pow(10, b);
}

FP overflow

A ridiculously long fractional part causes decplace to reach inf, and the perfectly valid number is converted to nan.
Testing for NAN and INF

Using strncmp would be much more clear.
DRY

The integer conversion loop is repeated at least twice (for whole part and exponent). It is a strong indication that you need to factor it out into a function. You may want to have this function to handle fractional part as well.
Best effort

The function fails to convert certain numbers which stdlib version does handle, e.g.

    char * s ="10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001E-200";

(a ridiculously long whole part compensated by E-200 exponent). I suppose that stdlib version anticipates such inputs and normalizes them prior to conversion.


JSON selector class in Python I've created a simple little class called JSONSelector to be able to select relevant data from a JSON object (a dict in Python). Here's what I have so far - it's based off of a few SQL statements, NumPy methods, and JavaScript iterating methods:

class JSONSelector:
    def __init__(self, json):
        self.json = json

    def __str__(self):
        return "{}({})".format(type(self).__name__, str(self.json))
    __repr__ = __str__

    def select(self, selectors):
        if selectors == "*":
            return type(self)(self.json)
        else:
            temp = {}
            for sel in selectors:
                temp[sel] = self.json.get(sel, None)
            return type(self)(temp)

    def where(self, cond):
        temp = {}
        for key, value in self.json.items():
            if cond(key, value):
                temp[key] = value
        return type(self)(temp)

    def astype(self, type_):
        temp = {}
        for key, value in self.json.items():
            temp[key] = type_(value)
        return JSONSelector(temp)

    def sort(self, **kwargs):
        return type(self)(dict(sorted(self.json.items(), **kwargs)))

    def foreach(self, func):
        for key, value in self.json.items():
            if func(key, value) == False: break

    def print(self, func=None):
        if not func:
            print(self)
            return
        else:
            print(func(self))
            return

Here's a test case with an explanation:

a = JSONSelector({
    "stuff": 5,
    "seeded": 6,
    "number": "7",
    "more_stuff": 8
})

a.select("*") \  # Select all entries from the dict
    .where(lambda key,value: type(value) is int) \ # Where the value is an integer
    .sort(key=lambda x: x[1]) \ # Sort by the value
    .astype(str) \ # Convert the values to type str
    .foreach(lambda key, value: print(key + ": " + value)) # Loop over all entries

I'd like to know if any of my code is redundant, where I can shorten things up, and if anything is incorrect. Thank you in advance!

A few notes:

Firstly, I would add some docstrings to each function to document what the expected behaviour is.

In select when you're not doing any mutations (i.e. under the * case), it seems you could just return self. Is there any reason to make a new copy?

In select, where, astype instead of creating a temporary dict, you could use a dict comprehension instead, example:

def where(self, cond):
    return type(self)({key: value for key, value in self.json.items() if cond(key, value)})

In astype you're using JSONSelector, however everywhere else you're using type(self) this should be consistent whichever one you go for.

print seems like an unnecessary function, but if you keep it the return lines have no effect.

Hope that's helpful.


Working and calculating with geographic positions I was thinking about how to implement a GPS navigation for my quadcopter.  I was writing some functions to calculate the angle between two positions (from home point to the target).

I'd like a review of this code.  I am not very sure whether I was calculating the geographic position right.

/*
 * Geographic calculations
 */
float UAVNav::width_of_merid_m(const float fLat_deg) {
  float fdX_m = _2PI * _RADIUS_EARTH_m * cos(ToRad(fLat_deg) );
  fdX_m /= 360.f;
  return fdX_m;
}

float UAVNav::dist_2_equat_m(const float fLat_deg) {
  return (_PERIMETER_EARTH_m / 360.f) * fLat_deg;
}

float UAVNav::dist_2_greenw_m(const float fLat_deg, const float fLon_deg) {
  return width_of_merid_m(fLat_deg) * fLon_deg;
}

float UAVNav::home_2_target_deg() {
  const float fMod = 10000000.f;
  GPSPosition target = m_pReceiver->m_Waypoint;
  GPSData home = m_pHalBoard->get_gps();

  float fLatHome_deg = home.latitude / fMod;
  float fLonHome_deg = home.longitude / fMod;

  float fLatTarg_deg = target.latitude / fMod;
  float fLonTarg_deg = target.longitude / fMod;

  float fXHome = dist_2_greenw_m(fLatHome_deg, fLonHome_deg);
  float fYHome = dist_2_equat_m(fLatHome_deg);

  float fXTarg = dist_2_greenw_m(fLatTarg_deg, fLonTarg_deg);
  float fYTarg = dist_2_equat_m(fLatTarg_deg);

  float dX = fXTarg - fXHome;
  float dY = fYTarg - fYHome;

  // Calculate the angle to the destination direction
  // NOTE: Here the system is inverted by 90° to align the magnetic north to 0°
  m_fDestin_deg = ToDeg(atan2((float)dX, (float)dY) );
  return m_fDestin_deg;
}

There are some temporary variables that you might not use, such as fLatTarg_deg and fLotTarg_deg: you can commit the operation along with the function call.

Also, the variables (which are actually costants, probably) _RADIUS_EARTH_m and _PERIMETER_EARTH_m are breaking the standard C++ naming conventions.


Duplicate words in a text Here is a simplified implementation to obtain the duplicate words in a text using lambda expressions.

public class FindDuplicateWordsInText {

    public static Set<String> findDuplicateWordsInText(String text) {
        String[] words = text.split(" ");
        Set<String> duplicatesRemovedSet = new HashSet<>();
        Set<String> duplicatesSet = Arrays.stream(words).filter(string -> !duplicatesRemovedSet.add(string))
                .collect(Collectors.toSet());
        return duplicatesSet;
    }
}

Your use of the boolean return value of the Set.add() call is a clever way to check for your duplicates. The concept you have is good, and I can't think of a faster way.

Additionally, I like how you have used Interface-based types on the left-side of assignments Set<String> and the concrete classes on the right new HashSet<>() .... people often put the concrete type on the left too, and it's good to see that you did not.

In terms of the Java streaming API, though, I can't help but feel that you missed out on an opportunity to improve the process by streaming the split.... The Pattern class has a splitAsStream method which would reduce your latency on the first words....

As an aside, a word should probably be on a contiguous whitespace, not just a single space (i.e. "\\s+" instead of " ").

Here's your code done differently:

private static final Pattern SPACE = Pattern.compile("\\s+");

public static Set<String> findDuplicateWordsInText(String text) {
    Set<String> duplicatesRemovedSet = new HashSet<>();
    return SPACE.splitAsStream(text)
            .filter(string -> !duplicatesRemovedSet.add(string))
            .collect(Collectors.toSet());
}


Implementation of Permutation Algorithm B from Topor, 1982 This is Algorithm B from Functional Programs for Generating Permutations (Topor, 1982).

We have implemented Algorithm B in an F# recursive module as follows.

module rec Permutations.Permute2

let removeFirst (list: List<'t>) (item: 't): List<'t> =
    match list with
    | [] -> []
    | head::tail when head = item -> tail
    | head::tail -> head :: (removeFirst tail item)

let mapcons (a: 't) (ps: List<List<'t>>) (qs: List<List<'t>>): List<List<'t>> =
    match ps with
    | [] -> qs
    | head::tail -> (a :: head) :: mapcons a tail qs

let mapperm (x: List<'t>) (y: List<'t>): List<List<'t>> =
    match y with
    | [] -> []
    | head::tail ->
        let permuteNext = permute (removeFirst x head)
        let mappermNext = mapperm x tail
        mapcons head permuteNext mappermNext

let permute (x: List<'t>) : List<List<'t>> =
    match x with
    | [] -> [ [] ]
    | _ -> mapperm x x

We would like a review to make our F# more canonical. That is, we would like to adjust our code to be in keeping with common F# styles and techniques.

Alternatively, we would like a reviewer to show alternative techniques (if not better ones) for implementing Algorithm B in F#.

Overall the code is pretty decent, but if you strive for perfection :-), I can offer a few observations from my side.

1. Generics notation

For generic types with a single generic parameter, F# has an alternative syntax, where you put the parameter in front of the constructor, not in angle brackets - e.g. int list instead of List<int> (note the lower-case list by the way - that is also idiomatic).

For example:

let rec mapcons (a: 't) (ps: 't list list) (qs: 't list list): 't list list =

2. Type signatures

Unlike some other ML-style languages (looking at you, Haskell :-), it is not commonplace for F# functions to have explicit type signatures. As a rule, F# code will only have type signatures either in the most tricky places, as documentation, or else in places where the compiler can't do without it, like with objects or interfaces.

Your functions are trivial enough (or at the very least, they don't really gain anything from type signatures) that I would leave the signatures off. For example:

let rec mapcons a ps qs =

3. Explicit recursion

Recursion is bleh. Don't get me wrong, it's super helpful, and the basis of a lot of stuff, we couldn't really live without it. But writing it explicitly is generally bleh. It's hard to get right, and it turns out that most linearly recursive algorithms can be reduced to just a few common patterns (and ultimately - to just fold).

Take your mapcons function: it first prepends a to every list in ps, and then concatenates qs to that. Well, if you want to transform every element of a list, there's an app for that - map:

let mapcons a ps qs = List.map (fun p -> a :: p) ps @ qs

Alternatively, I would consider a list comprehension:

let mapcons a ps qs = [for p in ps -> a :: p] @ qs

This produces the same effect, but looks differently. I'm on the fence about which one I would prefer, but I'm leaning towards the list comprehension.

4. Some trivial mistakes

As already pointed out in the comments (I wasn't quick enough), your removeFirst function needs to be rec, and mapperm + permute should be a recursive block with let rec ... and ....