Standard deviation from iterators I want this to be similar to the STL algorithms but I don't find it elegant nor concise at all:

#include <iostream>
#include <iterator>
#include <vector>
#include <algorithm>

using E = double;
template <typename IT>
E std_dev(IT begin, IT end){
    auto N = std::distance(begin, end);
    E average = std::accumulate(begin, end, E()) / N;
    auto sum_term = [average](E init, E value)-> E{
        return init + (value - average)*(value - average);
    };
    E variance = std::accumulate(begin,  end, E(), sum_term);
    return std::sqrt(variance * 1.0 / (N - 1));
}

int main(){
    std::vector<double> stuff {3.5, 3.4, 3.6, 3.9, 3.5, 3.5, 3.5, 3.5, 3.5};
    std::cout << std_dev(stuff.begin(), stuff.end()) << "\n";
}

Firstly, make it correct.

N is integral, you could make it E so you don't accidentally do integer arithmetic.

N-1 is wrong.

Rename average to mean.

Don't hardcode E.

You get:

template <typename It, typename E = typename std::iterator_traits<It>::value_type>
E std_dev(It begin, It end){
    E N = std::distance(begin, end);
    E const mean = std::accumulate(begin, end, E()) / N;
    auto sum_term = [mean](E init, E value)-> E { return init + (value - mean)*(value - mean); };
    E variance = std::accumulate(begin, end, E(), sum_term);
    return std::sqrt(variance / N);
}

Slightly stylized, with comparison to Boost Accumulator:

Live On Coliru

#include <iostream>
#include <iterator>
#include <vector>
#include <algorithm>

#include <boost/accumulators/accumulators.hpp>
#include <boost/accumulators/statistics.hpp>

template <typename It, typename E = typename std::iterator_traits<It>::value_type, typename R = typename std::common_type<double, E>::type>
R std_dev_boost(It begin, It end){
    namespace ba = boost::accumulators;

    ba::accumulator_set<R, ba::stats<ba::tag::variance> > accu;
    std::for_each(begin, end, std::ref(accu));
    return std::sqrt(ba::variance(accu));
}

template <typename It,
    typename E = typename std::iterator_traits<It>::value_type,
    typename R = typename std::common_type<double, E>::type>
R std_dev(It b, It e)
{
    R N          = std::distance(b, e);
    R const mean = std::accumulate(b, e, R{}) / N;
    R variance   = std::accumulate(b, e, R{}, [mean](R a, E v)-> R { return a + (v-mean)*(v-mean); });
    return std::sqrt(variance / N);
}

int main(){
    std::vector<int> stuff {35, 34, 36, 39, 35, 35, 35, 35, 35};
    std::cout << std_dev_boost(stuff.begin(), stuff.end()) << "\n";
    std::cout << std_dev      (stuff.begin(), stuff.end()) << "\n";
}

Prints

1.34256
1.34256

A simple conciseness improvement would be to inline non-informative variables:

template <typename IT>
E std_dev(IT begin, IT end) {
    auto N = std::distance(begin, end);
    E variance = std::accumulate(begin,  end, E{},
        [average=std::accumulate(begin, end, E{}) / N](E init, E value) -> E
    {
        return init + (value - average)*(value - average);
    });
    return std::sqrt(variance * 1.0 / (N - 1));
}

average was moved into the lambda capture (C++14(?)) and the lambda itself was passed directly to std::accumulate.


Python web-scraper to download table of transistor counts from Wikipedia I have been looking for answers for how to easily scrape data from Wikipedia into a CSV file with Beautiful Soup. This is the code so far. Is there an easier way to do it?

import urllib.request
from bs4 import BeautifulSoup
import csv

#the websites
urls = ['https://en.wikipedia.org/wiki/Transistor_count']
data =[]

#getting the websites and the data
for url in urls:
##    my_url = requests.get(url)
    my_url = urllib.request.urlopen(url)

    html = my_url.read()
    soup = BeautifulSoup(html,'html.parser')

    My_table = soup.find('table',{'class':'wikitable sortable'})
    My_second_table = My_table.find_next_sibling('table')

    with open('data.csv', 'w',encoding='UTF-8', newline='') as f:
        fields = ['Title', 'Year']
        writer = csv.writer(f, delimiter=',')
        writer.writerow(fields)

    with open('data.csv', "a", encoding='UTF-8') as csv_file:
        writer = csv.writer(csv_file, delimiter=',')
        for tr in My_table.find_all('tr')[2:]: # [2:] is to skip empty and header
            tds = tr.find_all('td')
            try:
                title = tds[0].text.replace('\n','')
            except:
                title = ""
            try:
                year = tds[2].text.replace('\n','')
            except:
                year = ""

            writer.writerow([title, year])

    with open('data.csv', "a", encoding='UTF-8') as csv_file:
        writer = csv.writer(csv_file, delimiter=',')
        for tr in My_second_table.find_all('tr')[2:]: # [2:] is to skip empty and header
            tds = tr.find_all('td')
            row = "{}, {}".format(tds[0].text.replace('\n',''), tds[2].text.replace('\n',''))
            writer.writerow(row.split(','))

Is there a way to simplify this code?

Yes. Don't scrape Wikipedia. Your first thought before "should I need to scrape this thing?" should be "Is there an API that can give me the data I want?" In this case, there super is.

There are many informative links such as this StackOverflow question, but in the end reading the API documentation really is the right thing to do. This should get you started:

from pprint import pprint
import requests, wikitextparser

r = requests.get(
    'https://en.wikipedia.org/w/api.php',
    params={
        'action': 'query',
        'titles': 'Transistor_count',
        'prop': 'revisions',
        'rvprop': 'content',
        'format': 'json',
    }
)
r.raise_for_status()
pages = r.json()['query']['pages']
body = next(iter(pages.values()))['revisions'][0]['*']
doc = wikitextparser.parse(body)
print(f'{len(doc.tables)} tables retrieved')

pprint(doc.tables[0].data())

This may seem more roundabout than scraping the page, but API access gets you structured data, which bypasses an HTML rendering step that you shouldn't have to deal with. This structured data is the actual source of the article and is more reliable.

When you asked for an easier way, do you mean an easier way that uses “real code” as in the cartoon?

That looks pretty easy, but I think slightly easier is possible.

In general, when I want a table from a web page, I select it with my mouse and paste special (unformatted) into a spreadsheet.  Bonus is that I don’t have to deal with NSA’s biggest competitor (Google).


Password hashing/handling on a form Does my code look secure right now? I know that I still have the password hashing and so forth, but what about SQL injections? Do I have any? Are there any other security issues?

<?php
$con=mysqli_connect("localhost","root","xxx","s");
// Check connection
if (mysqli_connect_errno())
  {
  echo "Failed to connect to MySQL: " . mysqli_connect_error();
  }

$name = $con->real_escape_string($_POST['name']);
$username = $con->real_escape_string($_POST['username']);
$email = $con->real_escape_string($_POST['email']);
$password1 = $con->real_escape_string($_POST['pass1']);
$password2 = $con->real_escape_string($_POST['pass2']);

if (empty($name) || empty($username) || empty($email) || empty($password1) || empty($password2))
{
    echo "Complete all fields";
    // you can stop it here instead of putting the curly brace ALL the way at the bottom :)
    return;
    }

if (!filter_var($email, FILTER_VALIDATE_EMAIL))
{
    echo $emailvalid = "Enter a  valid email";
    }

if (strlen($password1) <= 6)
{
echo $passlength = "Password must be at least 6 characters long";
}

// Password numbers
if (!preg_match("#[0-9]+#", $password1))
{
echo $passnum = "Password must include at least one number!";
}
if (!preg_match("#[a-zA-Z]+#", $password1))
{
echo $passletter = "Password must include at least one letter!";
}
if ($password1 <> $password2)
{
echo $passmatch = "Passwords don't match";
}

if(!(isset($emailvalid) || isset($passlength) || isset($passnum) || isset($passletter) || isset($passmatch))) {
mysqli_query($con,"INSERT INTO pass (Name,Username,Email,Password) VALUES     ('$name','$username','$email','$password1')");
 }

mysqli_close($con);
?>

Not only is there code that does this type of stuff already out there, freely available. This question has been asked and answered a couple of times before on this site already. This is one such example.

However, given that the answers currently posted here are either incomplete or, IMHO, wrong (suggesting bad practices, or providing incomplete info), I'll post a short review of mine here, too.

You have a simple, procedural script. There's nothing wrong with that, per se. Although, nowadays, people will tell you to pour your code into objects and let each class do its own, specific task.
Why should you use objects here? Well, that's really down to what kind of project you're putting together: is the site going to be a fully dynamic LAMP driven site? Is the code likely to grow, or is it probable that, as you go along, new features will be added to your site? Then yes, you should look into writing your own classes, or adopt a framework (ZendFW, Symfony2,... there are tons to choose from).

If the amount of PHP you'll be using is relatively small, and its role won't exceed simple user input checks and some DB connectivity, then OO code is just going to add clutter, and bloat.

On the code
Connecting to the DB, and executing queries alongside code that actually generates output for the user/client is one of the main reasons why PHP is said to be a badly designed language. Yes, PHP allows you to write that kind of code, but you should know that it's actually seen as bad practice. A saw doesn't prevent you from sawing off the branch you're sitting on, but that doesn't mean it's a good idea to do so... Same thing applies to programming.

If you don't require object-oriented code (yet), at least create functions to handle the DB stuff, put them in a separate file, and call the functions when you need them:

require 'db_functions.php';//or require_once
$db = getDbConnection(
    array(
        'user'  => 'username',
        'pass'  => 'yourpass'
    )
);

Perhaps look into ways to keep the actual login credentials oustide of your code.

Escaping and assigning
I cannot, for the life of me understand why you would do anything like:

$name = $con->real_escape_string($_POST['name']);

with all of your post values. All those values belong together: they represent the data the user posted to your script, to validate and process. Why would you assign them to individual variables? keep that data as a whole.
If you want to check if an array has all values/keys set, then please use isset and then compare the values to an empty string. empty has a couple of oddities you should be aware of.

In this case, validating the post data can be easily done in a single loop:

$keys = array('name', 'username', 'email', 'pass1', 'pass2');
foreach ($keys as $k)
{
    if (!isset($_POST[$k]) || $_POST[$k] == '')
        exit();//<-- more on this later
}

Code like this can be easily poured into a function, to which you can pass both the array to check ($_POST) and the keys to check ($keys). Just 2 simple arguments, and you never need to write code assigning and checking variables again, just a single function call will do.

Now, the real_escape_string calls really are a bad idea. Quote chars, slashes... they're all allowed in email addresses. calling real_escape_string means that there is a chance that you're screwing up perfectly valid user input, and rendering it invalid. don't.

Using prepared statements here would be the only proper way to go: don't process the input a user gave you. If the input is anything other than perfect, assume the input is invalid or possibly malicious.

Avoid too many regex's
Checking for a password that contains both digits and characters can be done through regex, but that might not be the best tool for the job. The actual benchmarking is your job, but a regex often is slower than a simple loop, checking if a char is numeric or not.

Either way: you are using 2 regular expressions for something that can be easily done with just one expression. I've posted an answer here which goes into more details on the matter.

re-assigning again?
After validating the input (that, as I explained above, you may have rendered invalid yourself!), you then check variables like $emailValid again. That really ought to set off alarm bells.

Just think about it: you check some things. If the very first thing you checked showed you the user sent invalid input, you still procede to check, validate, reassign all other values, only to then again check if all input is valid.
That's doing the same thing twice, isn't it? DRY (Do not Repeat Yourself) is a common expression/piece of advice in programming. Your code basically repeats the most basic input validation task. That can't be good, now can it?

That's why throwing an exception is preferable, and that is also why separating output code, and logic/db related code into (at least) functions. These functions (functional units, as their name suggests) could be written to throw exceptions, which will then determine the output message the client will get to see.
Here's a basic example to clear this up. Remeber how I mentioned a function to check if all POST params are set? Here's how that would work in your case:

function checkKeys(array $data, array $keys)
{//note the type-hints!
    foreach ($keys as $k)
    {
        if (!isset($data[$k]) || $data[$k] == '')
            throw new InvalidArgumentException($k.' parameter not set');
    }
    return true;
}
//your code:
try
{
    checkKeys($_POST, array('name', 'username', 'email', 'pass1', 'pass2'));
}
catch( InvalidArgumentException $e)
{//catch Exception works, too
    echo 'Error: ', $e->getMessage();
}

That should be enough to get you started for now ;-P

Consider parameterized queries over building a query string.
If you really want to use encoding, do the encoding directly before passing it to mysql
Factor out the password strength rules into a separate function. This function could output an array of violations.
Don't mix logic and IO.


Prompting for grades, with confirmation if a score higher than 100 is entered This is an exercise from my computer programming class. This program reads in student scores from the user (input) and displays the total points earned along with a grade. I am tasked with modifying it to ask the user for confirmation when a score higher than 100 is entered. Plainly, it should not add a score greater than 100 to the accumulator without permission.

Here is the original source:

 //Lab7-2.cpp - displays the total points earned and grade
 //Created/revised by <your name> on <current date>

 #include <iostream>

 using namespace std;

 int main()
 {
 //declare variables
    int score       = 0;
    int totalPoints = 0;  //accumulator
    char grade      = ' ';

    //get first score
    cout << "First score (-1 to stop): ";
    cin >> score;

    while (score != -1)
    {
        //update accumulator, then get another score
        totalPoints += score;
    cout << "Next score (-1 to stop): ";
    cin >> score;
    }   //end while

    //determine grade
    if (totalPoints >= 315)
    grade = 'A';
    else if (totalPoints >= 280)
    grade = 'B';
    else if (totalPoints >= 245)
    grade = 'C';
    else if (totalPoints >= 210)
    grade = 'D';
    else
    grade = 'F';
    //end if

    //display the total points and grade
    cout << "Total points earned: " << totalPoints << endl;
    cout << "Grade: " << grade << endl;

    system("pause");
 return 0;
 }   //end of main function

Here is the modified source:

//Lab7-2.cpp - displays the total points earned and grade
//Created/revised by Willy Njundong on 2/14/2016

#include <iostream>

using namespace std;

int main()
{
//declare variables
int score = 0;
int totalPoints = 0;  //accumulator
char grade = ' ';
char decision = ' ';

//get first score
cout << "First score (-1 to stop): ";
cin >> score;

while (score != -1)
{
    if (score > 100)
    {                                                                //CONFIRM SCORE VALIDITY
        cout << endl << "Is score correct? (Enter y or n): ";        //IF GREATER THAN 100
        cin >> decision; cout << endl;
        if (decision != 'y' && decision != 'Y')                 //ANY CHARACTER ENTERED OTHER THAN 'Y' FOR YES
            score = 0;                                          //RESULTS IN SCORE OVER 100 BEING IGNORED (SET TO ZERO)
        else
        {                                                       //AND THEN THE
            //update accumulator, then get another score        //PROGRAM RESUMES NORMAL EVALUATION
            totalPoints += score;
            cout << "Next score (-1 to stop): ";
            cin >> score;
        }   //end if

    }
    else                                                        //THESE ARE PROCESSED WHEN SCORES
    {                                                           //ENTERED ARE 100 OR LESS
        //update accumulator, then get another score
        totalPoints += score;
        cout << "Next score (-1 to stop): ";
        cin >> score;
    }   //end if

}   //end while

    //determine grade
if (totalPoints >= 315)
    grade = 'A';
else if (totalPoints >= 280)
    grade = 'B';
else if (totalPoints >= 245)
    grade = 'C';
else if (totalPoints >= 210)
    grade = 'D';
else
    grade = 'F';
//end if

//display the total points and grade
cout << "Total points earned: " << totalPoints << endl;
cout << "Grade: " << grade << endl;

system("pause");
return 0;
}   //end of main function

The only option is this one:

// ...
while (score != -1) {
    if (score > 100) {
        cout << endl << "Is the score correct? (Enter 'y' or 'n'): ";
        cin >> decision; cout << endl;
        if (decision != 'y' && decision != 'Y') score = 0;
    }

    totalPoints += score;
    cout << "Next score ('-1' to stop): ";
    cin >> score;
}   //end while
// ...

It can't be more efficient using given circumstances.


Python username and password program Can the following code be simplified? I think that the program that I have created is very excessive and quite slow. The program runs fine, but I would like to understand if this could actually be programmed better.

I have commented on every line of the code. Any kind of feedback on the program will be great, as I have only been programming for a couple of weeks now.

# Python 3.4.3. Using MacOS (Version 10.12.5)
# Username and Password...
# The programs purpose: A user must enter the correct username and password for a site called FaceSnap...
# The correct username is elmo and the correct password is blue.

userName = input("Hello! Welcome to FaceSnap! \n\nUsername: ") #Ask's the User for Username input
password = input("Password: ") # Ask's the user for their password

count = 0 # Create a variable, to ensure the user has limited attempts at entering their correct username and password
count += 1 # The user has already had one attempt above, therefore count has been incremented by 1 already.

while userName == userName and password == password: # The Input will always lead to this while loop, so we can see if their username and password is wrong or correct.

    if count == 3: # Counter, to make sure the user only gets a limited number (3)of attempts
        print("\nThree Username and Password Attempts used. Goodbye") # Lets the user know they have reached their limit
        break # Leave the Loop and the whole program

    elif userName == 'elmo' and password == 'blue': # The userName and password is equal to 'elmo' and 'blue', which is correct, they can enter FaceSnap!
        print("Welcome! ") # Welcomes the User, the username and password is correct
        break # Leave the loop and the whole program as the username and passowrd is correct

    elif userName != 'elmo' and password != 'blue': # The userName and password is NOT equal to 'elmo' and 'blue', the user cannot enter FaceSnap
        print("Your Username and Password is wrong!") # Lets the user know that the Username and password entered is wrong.
        userName = input("\n\nUsername: ") # Requests the user to have another attempt at entering their correct username
        password = input("Password: ") # Requests the user to have another attempt at entering their correct password
        count += 1 # Increments the count by 1
        continue # Continue, as the user hasn't managed to get their username and password correct yet

    elif userName == 'elmo' and password != 'blue': # The userName is equal to 'elmo', but password is NOT equal to 'blue', the user cannot enter FaceSnap
        print("Your Password is wrong!") # Lets the user know that their password is wrong
        userName = input("\n\nUsername: ") # Requests the user to have another attempt at entering their correct username
        password = input("Password: ") # Requests the user to have another attempt at entering their correct password
        count += 1 # increments the count by 1
        continue # Continue, as the user hasn't managed to get their username and password correct yet

    elif userName != 'elmo' and password == 'blue': # The userName is NOT equal to 'elmo', however password is equal to 'blue', the user cannot enter FaceSnap
        print("Your Username is wrong!") # Lets the user know that their username is wrong
        userName = input("\n\nUsername: ") # Requests the user to have another attempt at entering their correct username
        password = input("Password: ") # Requests the user to have another attempt at entering their correct password
        count += 1 # Increments the count by 1
        continue # Continue, as the user hasn't managed to get their username and password correct yet

A few points:

You can simplify the if loop by using the else condition.
Getting input should be at the beginning of the while loop, because it makes more logical sense to put it there. It's what happens every time you restart.
You can use the simple condition of True for your while loop.
You don't need the continue keyword, as it is at the end of your loop.

count = 0
while True:
    userName = input("Hello! Welcome to FaceSnap! \n\nUsername: ")
    password = input("Password: ")
    count += 1
    if count == 3:
        #tells user bye
        break #exit
    else:
        if userName == 'elmo' and password == 'blue':
            #let them in
            break #they are in, exit loop
        else:
            #tell them it is wrong and have them retry, stay in loop

In terms of design, there are two main things you should take in consideration:

It is not good to have lot of if statements, especially in your case. You need to refactor your code in the name of the Anti-If Compaign.
You should accomplish the tasks through small units of code called functions.

In terms of security, there are also two main things to consider:

When an attacker targets your applications and types the right username, you inform him that the user exists, but only the password is wrong. This is bad in the security perspective because it increases the attack surface of your application. You can overcome this by displaying a more generic and less informative error message instead.
I do not know if your application is running on the Internet or offline. It is recommended that you hide the password which is being typed for the sake of reducing the attack surface too.

Given the above 4 observations, I want to provide you an improved version of your code through an MCVE where:

I use getpass module to hide the password when it is typed.
I use a predefined user credentials to simulate a more realistic situation (such as may be in your case you get the credentials from a database)
I use a separate function to read the user credentials
I use an other function which only purpose is to validate the user input.

These steps will lead us to using one if statement only:

#!/usr/bin/env python
# -*- coding: utf-8 -*-
import getpass

user_credentials = ['begueradj', 'password']

def get_user_credentials():
   username = input('Username: ')
   password = getpass.getpass('Password: ')
   return username, password

def check_user_credentials(user_credentials):
   username, password = get_user_credentials()
   return user_credentials[0] == username and user_credentials[1] == password

if __name__ == "__main__":
    login_attempts = 1
    correct_credentials = check_user_credentials(user_credentials)
    while not correct_credentials and login_attempts < 3:
       print('\nWrong credentials, try again:')
       correct_credentials = check_user_credentials(user_credentials)
       login_attempts += 1
    if correct_credentials:
       print('Welcome! Do whatever work you need here ...')
    else:
       print('Game over!')


Project Euler #5 (LCM 1 - 20) Follow up I'm actually spending time on it, unlike the first time and going through, understanding and implementing the several suggestions I've landed on two different approaches.

Approach 1: Employing Java 8 Lambdas and stream functions made to be the equivalent of David Zheng's answer in Python.

import java.util.ArrayList;

public class Euler5 {
    public static void main(String[] args) {
        long startTime = System.nanoTime();
        final long MAX_NUM = 20L;

        ArrayList<Long> numbers = new ArrayList<Long>() {
            { for (long i = 1L; i <= MAX_NUM; i++) { add(i); } }
        };

        long result = numbers
            .stream().reduce(Euler5::lcm).get();

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

    /*Greatest Common Divisor
    Euclidean algorithm: http://en.wikipedia.org/wiki/Euclidean_algorithm */
    public static long gcd(long a, long b) {
        return b == 0 ? a : gcd(b, a % b);
    }
    // Least Common Multiple
    public static long lcm(long a, long b) {
        return (a * b) / gcd(a, b);
    }
}

Sample output:

  Result: 232792560.
  Time used for calculation in nanoseconds: 111370744.

Approach 2: More similar to my original method, using several suggestions, including one in the OP's comment section which I'm unsure about.

public class Euler5 {
        public static void main(String[] args) {
            long startTime = System.nanoTime();
            // Multiplying by primes for a starting point * 2 for evenness
            int givenTimesPrimes = 2520 * 2 * 11 * 13 * 17 * 19,
                result = givenTimesPrimes;

            while (!isSmallestMultiple(result)) {
                result += givenTimesPrimes;
            }

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

        public static boolean isSmallestMultiple(int n) {
            for (int i = 11; i <= 20; i++) {
                if (n % i != 0) {
                    return false;
                }
            }
            return true;
        }
    }

Example output:

  Result: 232792560.
  Time used for calculation in nanoseconds: 141635.

Approach 2 is clearly faster, but I'm wondering if givenPrimes qualify as "magic numbers?" Or is the explanation enough (feels tenuous/not 100% certain it would qualify as an effective strategy in a different context).
The first method seems the more understandable/applicable. Am I wrong? Which would you fellow CR folks prefer and why?

This Project Euler question is an excellent one to think about computational efficiency of algorithms.  It's a horrible place to make decisions based on benchmarks, though.

Sure, your second method is faster.  But you're basically cheating with the magic number 2520.  It's not a particularly insightful implementation for mathematical learning, either, as it's guessing rather than computing.

To illustrate what I mean, let's consider two more implementations.

"Fast and Crappy"

This algorithm computes the LCM of \$\mathrm{numbers}_i\$ based on the principle that

$$ \mathrm{LCM}(a, b) = \prod p_j^{\max(e_{j,a}, e_{j,b})} $$

when numbers are factorized as \$a = \prod p_j^{e_{j,a}}\$ and \$b = \prod p_j^{e_{j,b}}\$.

  Why do I call it "crappy"? Because it is using the same strategy as this nice-looking Ruby code, but it would take a lot of work to make it look as good in Java.

   require 'prime'

lcm_factors = Hash.new(0)       # Default entry is 0
(1..20).each do |n|
  Prime.prime_division(n).each do |factor|
    prime, exponent = *factor
    lcm_factors[prime] = [lcm_factors[prime], exponent].max
  end
end
puts lcm_factors.map { |prime, max_exponent| prime ** max_exponent }
                .inject(:*)

public class FastCrappyEuler5 {
    private static final int[] PSEUDOPRIMES_BELOW_20 = new int[] {
        2, 3, 5, 7, 11, 13, 17, 19
        // 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19
    };

    public static long lcmOfNumbersUpTo(int rangeMax) {
        if (rangeMax > 20) {
            throw new IllegalArgumentException("TODO: FIXME");
        }

        int[] numbers = new int[rangeMax];
        for (int n = 1; n <= rangeMax; n++) {
            numbers[n - 1] = n;
        }

        long lcm = 1;
        for (int p : PSEUDOPRIMES_BELOW_20) {
            int maxExponent = 0;
            for (int i = 0; i < numbers.length; i++) {
                int exponent;
                for (exponent = 0; numbers[i] % p == 0; exponent++) {
                    numbers[i] /= p;
                }
                maxExponent = Math.max(maxExponent, exponent);
            }

            while (maxExponent --> 0) {
                lcm *= p;
            }
        }

        return lcm;
    }

    public static void main(String[] args) {
        long start = System.nanoTime();
        System.out.println(lcmOfNumbersUpTo(20));
        System.err.println(System.nanoTime() - start);
    }
}

This takes about 6 times as long as your fast implementation.  (For me, it's on the order of ~105 nanoseconds.)  I've taken a shortcut of hard-coding the primes below 20 — a "fair" move, I think, as most people know them by heart.

If you replace the pseudoprimes list with a list of all integers from 2 to 20, you get the same answer, with no significant difference in running time.

Using streams

This implements exactly the same algorithm as FastCrappyEuler5, but does so using Java 8 streams to be more elegant.

import java.util.stream.IntStream;
import java.util.stream.LongStream;

public class StreamsEuler5 {
    private static LongStream pseudoPrimesBelow(int max) {
        // We should be more selective, but as we saw above,
        // it shouldn't make much difference.
        return LongStream.range(2, max);
    }

    private static long pow(long base, /* unsigned */ int exponent) {
        long power = 1;
        while (exponent --> 0) {
            power *= base;
        }
        return power;
    }

    public static long lcmOfNumbersUpTo(int rangeMax) {
        final int[] numbers = IntStream.rangeClosed(1, rangeMax).toArray();

        // Warning: computation is not parallelizable
        return pseudoPrimesBelow(rangeMax).map((p) -> {
            int maxExponent = 0;
            for (int i = 0; i < numbers.length; i++) {
                int exponent;
                for (exponent = 0; numbers[i] % p == 0; exponent++) {
                    numbers[i] /= p;
                }
                maxExponent = Math.max(maxExponent, exponent);
            }
            return pow(p, maxExponent);
        }).reduce(1, (a, b) -> a * b);
    }

    public static void main(String[] args) {
        long start = System.nanoTime();
        System.out.println(lcmOfNumbersUpTo(20));
        System.err.println(System.nanoTime() - start);
    }
}

Unfortunately, it's two orders of magnitude slower! (For me, it's on the order of 5 ⨉ 107 nanoseconds.)

Conclusion

$$\begin{array}{l|r}
\textrm{Implementation}                & \textrm{Approx. running time} \\
\hline
\textrm{Legato's streams}              & 1 \times 10^8 \mathrm{ns} \\
\textrm{Legato's cheat-and-guess}      & 5 \times 10^4 \mathrm{ns} \\
\textrm{200_success's fast-and-crappy} & 1 \times 10^5 \mathrm{ns} \\
\textrm{200_success's streams}         & 5 \times 10^7 \mathrm{ns} \\
\end{array}$$

Are the streams-based implementations worse because they are slower?  I don't think so.  The overhead of the streams seems to dwarf everything else.  But I don't mind waiting a few million nanoseconds if it means that I like the code better.  My streams-based solution is the same algorithm as "Fast and Crappy", so the complexity must be the same.  Presumably, the startup costs would be inconsequential for larger problems.

Basically, do the "right thing" for readability, maintainability, and educational value.  Time measurements mean nothing when you haven't taken scalability into account.  Worry about speed when it's noticeably slow.

This is my solution, it seems to be a lot faster than the ones above:

    public class LCM {
        public static void main(String[] args) {
        long startTime = System.nanoTime();
        int high = 20;
        long value = 1;
        for (int i = 0; i < high; i++) {
            int j = 1;
            if ((value % (i + 1)) == 0) {
            } else {
                while (value % (i + 1) != 0) {
                    if ((value * j) % (i + 1) == 0) {
                        value *= j;
                    }
                    j++;
                }
            }
        }
        System.out.print("Result: " + value +
                ".\nTime used for calculation in nanoseconds: " +
                (System.nanoTime() - startTime) + ".");
        }
}

It basically will check to see if the value will be divisible by the current number, and if so, will multiply. It's not very clean though, having it all in the main method.


Finding index of first substring I wrote this function to find the index of the first substring. I was wondering if you can help me find some flaws or possibly help increase performance?

Example:

  str = "happy" substr = "app"

  index = 1

My code:

public static int subStringIndex(String str, String substr) {
    int substrlen = substr.length();
    int strlen = str.length();
    int j = 0;
    int index = -1;

    if (substrlen < 1) {
        return index;
    }
    else {
        for (int i = 0; i < strlen; i++) {              // iterate through main string
            if (str.charAt(i) == substr.charAt(j)) {    // check substring
                index = i - j;                              // remember index
                j++;                                    // iterate
                if (j == substrlen) {                   // when to stop
                    return index;
                }
            }
            else {
                j = 0;
                index = -1;
            }
        }
    }
    return index;
}

Just checking that you intend to be reinventing-the-wheel, you can do:

System.out.println("happy".indexOf("app"));

You did know that, right?

Or, if you want to reformat the 'signature' to match yours, it is:

public static int subStringIndex(String str, String substr) {
    return str.indexOf(substr);
}

There are a number of helper methods on String which will help:

String.indexOf(substr) - return the index of the first occurrence of substr
String.indexOf(substr, start) - return the index of the first occurrence of substr on or after the start position.
String.lastIndexOf(substr) - return the index of the last occurrence of substr
String.lastIndexOf(substr, start) - return the index of the last occurrence of substr starting before the start position.
 -

My re-writing: clear and clean, yet efficient. There is no innovation in the algorithm. Just the way of the coding in more structural re-arrangement, trying to make the thought and steps easy to read and understand (comments are welcome):

static int subStringIndex( String str, String substring) {
    if (substring.length() < 1 )
        return -1;

    int L = str.length();
    int l = substring.length();
    int index = -1;

    for (int i = 0, j; i <= L-l; i++) {
    // if the remaining (L-i) is smaller than l,
    // there won't be no enough length to contain the substring.
        for (j = 0; j < l; j++) {
            if (substring.charAt(j) != str.charAt(i+j) ) {
                    break;
            }
        }
        // has it reached the end of the shorter string?
        // if so, it means no non-equals encountered.
        if (j == l) {
            index = i;
            break;
        }
    }

    return index;
}


Building session-variables from a SQL stored procedure Calling code and attempt at building session variables:

DataTable dtServerVars = clCommonFunctions.TryAutoLogin("europe\\MrTest");
Session["CablingUserID"]= dtServerVars.Rows[0]["CablingUserID"].ToString();
Session["CablingUseremail"] = dtServerVars.Rows[0]["CablingUseremail"].ToString();
Session["CablingLogin"] = dtServerVars.Rows[0]["CablingLogin"].ToString();
Session["CablingPassword"] = dtServerVars.Rows[0]["CablingPassword"].ToString();
Session["CablingPersonnel"] = dtServerVars.Rows[0]["CablingPersonnel"].ToString();
Session["CablingSurname"] = dtServerVars.Rows[0]["CablingSurname"].ToString();
Session["CablingFirstName"] = dtServerVars.Rows[0]["CablingFirstName"].ToString();
Session["CablingSuperUser"] = dtServerVars.Rows[0]["CablingSuperUser"].ToString();
Session["CablingDateAdded"] = dtServerVars.Rows[0]["CablingDateAdded"].ToString();
Session["CablingContact"] = dtServerVars.Rows[0]["CablingContact"].ToString();
Session["CablingApprovalAuthority"] = dtServerVars.Rows[0]["CablingApprovalAuthority"].ToString();
Session["CablingAdminUser"] = dtServerVars.Rows[0]["CablingAdminUser"].ToString();
Session["SharedInfoID"] = dtServerVars.Rows[0]["SharedInfoID"].ToString();
Session["SharedInfousername"] = dtServerVars.Rows[0]["SharedInfousername"].ToString();
Session["SharedInfopassword"] = dtServerVars.Rows[0]["SharedInfopassword"].ToString();
Session["SharedInfoname"] = dtServerVars.Rows[0]["SharedInfoname"].ToString();
Session["SharedInfoemail"] = dtServerVars.Rows[0]["SharedInfoemail"].ToString();
Session["SharedInfoICLlocation"] = dtServerVars.Rows[0]["SharedInfoID"].ToString();
Session["SharedInfoPhone"] = dtServerVars.Rows[0]["SharedInfoPhone"].ToString();
Session["SharedInfoSecLevel"] = dtServerVars.Rows[0]["SharedInfoSecLevel"].ToString();
Session["IMSUserID"] = dtServerVars.Rows[0]["IMSUserID"].ToString();
Session["IMSUserName"] = dtServerVars.Rows[0]["IMSUserName"].ToString();
Session["IMSIsAnonymous"] = dtServerVars.Rows[0]["IMSIsAnonymous"].ToString();
Session["IMSLastActivityDate"] = dtServerVars.Rows[0]["IMSLastActivityDate"].ToString();
Session["loggedin"] = "unknown";

Code being called:

public static DataTable TryAutoLogin(string strREMOTE_USER)
{
    SqlConnection siConnection = new SqlConnection();
    siConnection.ConnectionString = Databases.getDbConnectionString("csSharedInfo");
    siConnection.Open();
    SqlCommand seCmd = new SqlCommand("GetSignOnDetails", siConnection);
    seCmd.CommandType = CommandType.StoredProcedure;
    seCmd.Parameters.Add(new SqlParameter("@DomainAccount", SqlDbType.NVarChar, 300));
    seCmd.Parameters["@DomainAccount"].Value = strREMOTE_USER;
    seCmd.CommandType = CommandType.StoredProcedure;
    SqlDataAdapter sda = new SqlDataAdapter();
    seCmd.Connection = siConnection;
    sda.SelectCommand = seCmd;
    DataTable dtServerVars = new DataTable();
    sda.Fill(dtServerVars);
    siConnection.Close();
    if (dtServerVars != null)
    {
        if (dtServerVars.Rows.Count > 0)
    {
        return dtServerVars;
    }
}
    return null;
}

First your TryAutoLogin:

A SqlConnection is IDisposable. Use it in combination with using.
You have some duplicate lines of code, like seCmd.CommandType = CommandType.StoredProcedure;.
dtServerVars can never be null.
You do not have to open the connection, SqlDataAdapter will do that for you.
Since you are using only ONE datarow, just return one datarow.

Resulting in:

 public static DataRow TryAutoLogin(string strREMOTE_USER)
 {
      using(SqlConnection siConnection = new SqlConnection(Databases.getDbConnectionString("csSharedInfo")))
       {
           SqlCommand seCmd = new SqlCommand("GetSignOnDetails", siConnection);
           seCmd.CommandType = CommandType.StoredProcedure;
           seCmd.Parameters.AddWithValue("@DomainAccount", strREMOTE_USER);
           SqlDataAdapter sda = new SqlDataAdapter(seCmd);
           DataTable dtServerVars = new DataTable();
           sda.Fill(dtServerVars);
           if (dtServerVars.Rows.Count > 0)
                return dtServerVars.Rows[0];
           return null;
      }
 }

Having done this, the next piece of code will also be a lot easier:

DataRow drServerVars = clCommonFunctions.TryAutoLogin("europe\\MrTest");
Session["CablingUserID"]= drServerVars["CablingUserID"].ToString();
Session["CablingUseremail"] = drServerVars["CablingUseremail"].ToString();
Session["CablingLogin"] = drServerVars["CablingLogin"].ToString();
...

My only suggestion, in addition to existing answers, would be to use a domain object instead of a datatable and multiple session variables. That way you have a simple, strongly-typed way of accessing all those details at once without dealing with the database fields all over the place.

The idea would be to create a class for containing all the data:

public class SignOnDetails
{
    public int CablingUserID { get; set; }
    public string CablingUseremail { get; set; }
    public string CablingLogin { get; set; }
    public string CablingPassword { get; set; }
    //All other properties ommited for brevity, but it goes like the previous ones
}

Then, the TryAutoLogin method would return an instance of this class instead of a raw table:

public static SignOnDetails TryAutoLogin(string strREMOTE_USER)
{
    SqlConnection siConnection = new SqlConnection();
    siConnection.ConnectionString = Databases.getDbConnectionString("csSharedInfo");
    siConnection.Open();
    SqlCommand seCmd = new SqlCommand("GetSignOnDetails", siConnection);
    seCmd.CommandType = CommandType.StoredProcedure;
    seCmd.Parameters.Add(new SqlParameter("@DomainAccount", SqlDbType.NVarChar, 300));
    seCmd.Parameters["@DomainAccount"].Value = strREMOTE_USER;
    seCmd.CommandType = CommandType.StoredProcedure;
    SqlDataAdapter sda = new SqlDataAdapter();
    seCmd.Connection = siConnection;
    sda.SelectCommand = seCmd;
    DataTable dtServerVars = new DataTable();
    sda.Fill(dtServerVars);
    siConnection.Close();
    if (dtServerVars != null)
    {
        if (dtServerVars.Rows.Count > 0)
        {
            //Here we build the SignOnDetails instance from the datatable
            SignOnDetails details = new SignOnDetails();
            details.CablingUserID = dtServerVars.Rows[0]["CablingUserID"];
            details.CablingUseremail = dtServerVars.Rows[0]["CablingUseremail"];
            details.CablingLogin = dtServerVars.Rows[0]["CablingLogin"];
            details.CablingPassword = dtServerVars.Rows[0]["CablingPassword"];
            //All other properties ommited for brevity, but it goes like the previous ones
            return details ;
        }
    }
    return null;
}

Then, you save all the data at once in one go:

SignOnDetails details= clCommonFunctions.TryAutoLogin("europe\\MrTest");
Session["SignOnDetails"] = details;

And using it becomes strongly typed, requiring accesing a single session variable:

string login = ((SignOnDetails)Session["SignOnDetails"]).CablingLogin;

This has the advantages of being stringly typed, don't rely much on magic strings, and the database is nicely encapsulated in one method, while the rest of the program only knows about this class.


Login function in PHP I've been working on a simple login script. The registration part is still to be coded, but I'm done the login part. Are there any security holes in my code? I'm using SHA-512 right now but I think I'll change it to mcrypt later.

Class User
{
    private $db;

    public function __construct($db)
    {
        $this->db = $db;
    }

    public function Login($user_email, $user_pass, $remember = false)
    {
        $user_pass = sha1($user_pass);
        $query = $this->db->prepare("SELECT id, password FROM users WHERE email = :email");
        $query->bindValue(':email', $user_email);
        $query->execute();

        if($query->rowCount() > 0) {
            $user = $query->Fetch(PDO::FETCH_OBJ);
            if($user->password == $user_pass) {
                $_SESSION['loggd_id'] = $user->id;
                if($remember == true) {
                    setcookie('lggd_sess', hash('sha512', uniqid()), 84600);
                    return true;
                }
                return true;
            } else {
                return 'Incorrect Email/Password Combination.';
            }
        } else {
            return 'Incorrect Email/Password.';
        }

    }
}

I'm using SHA-512 right now but I think I'll change it to mcrypt later.

Neither of those are the proper tool for the job. Mcrypt is meant for symmetric encryption (AES, TwoFish, DES, etc.), not password hashing.

If you're running PHP 5.5+, use password_hash() and password_verify()

Otherwise, https://github.com/ircmaxell/password_compat is the way to go.

EDIT: Example code:

<?php
Class User
{
    private $db;

    public function __construct($db)
    {
        $this->db = $db;
    }

public function Login($user_email, $user_pass, $remember = false)
{
        $query = $this->db->prepare("SELECT id, password FROM users WHERE email = :email");
        $query->bindValue(':email', $user_email);
        $query->execute();

        if($query->rowCount() > 0) {
            $user = $query->Fetch(PDO::FETCH_OBJ);
            if (password_verify($user_pass, $user->password)) {
                $_SESSION['loggd_id'] = $user->id;
                if($remember == true) {
                    setcookie('lggd_sess', hash('sha512', mcrypt_create_iv(64, MCRYPT_DEV_URANDOM)), 84600);
                    return true;
                }
                return true;
            } else {
                return 'Incorrect Email/Password Combination.';
            }
        } else {
            return 'Incorrect Email/Password.';
        }
    }
}

When you insert a row in the DB, you want to use password_hash($user_password)

Only thing I could think of adding right now would be,

if (isset($user_pass)) {
    $user_pass = htmlspecialchars($user_pass);
    $user_pass = preg_replace('/\s+/', '', $user_pass);
}
if (isset($user_email)) {
    $user_email = htmlspecialchars($user_email );
    $user_email = preg_replace('/\s+/', '', $user_email);
}

Even though you prepare your statement and bind your variable, it doesn't hurt to escape any characters that may break your query.

Also, consider removing all unnecessary white space?


Mongo command to add a field to all documents in a collection This code appends a boolean column to all documents in the birds collection.

db.birds
  .find()
  .snapshot()
  .forEach(function(el) {
    el.hi = true
    db.birds.save(el)
  })

Here's how the command is run in production.

mongo db_stuff/db_name -u username -p password theScript.js

This command runs very slowly.  Should I look into the $set operator or something else that will speed this up?

This is a lot faster:

db. birds.updateMany(
   {},
   { $set:
      {
        hi: true
      }
   }
)


Showing activity indicator (loading image) while processing in background I have a login screen on my iOS app. When I click login button I want to make sure that user should be displayed a loading image in center of the screen.

I found lots of different methods online but I am after a simplest method. I found a way to display activity Indicator with minimum code:

First of all, import your UI Kit:

import UIKit;

Then you have to initiate activity indicator:

let activityIndicator:UIActivityIndicatorView = UIActivityIndicatorView();

Then just copy paste this functions below which are self explainatory and call them whereever you need them:

func startLoading(){
    activityIndicator.center = self.view.center;
    activityIndicator.hidesWhenStopped = true;
    activityIndicator.activityIndicatorViewStyle = UIActivityIndicatorViewStyle.gray;
    view.addSubview(activityIndicator);

    activityIndicator.startAnimating();
    UIApplication.shared.beginIgnoringInteractionEvents();

}

func stopLoading(){

    activityIndicator.stopAnimating();
    UIApplication.shared.endIgnoringInteractionEvents();

}

Although, I am still not convinced that it's the best way to do it. Could it still be reduced?

Here is how my screen looks with the code:

There are a few things I would recommend to do.

Usually, instead of disabling the interaction events there is an extra layer on top of all views that contains the activity indicator. So, instead of adding the activity indicator directly into the viewController's view, add it in another view first with clear background color and with 0.4 alpha.
Right now the startLoading() function also sets up the activity indicator, what i would rather separate into a different function, like setupLoading()
If you want to reuse the solution on other viewControllers, create a class or a protocol at least ecapsulating the logic. Also, it is better to present it on the UIApplication's keyWindow property, than just simply adding it to viewContollers view's.
Prepare for the stopLoading() and startLoading() functions to be called from a background thread.

Finally, there are plenty of very good solutions already on the interwebs, that you could integrate into your project, like SwiftSpinner.

If you need further help, code examples, let me know and I will edit my answer.

Tested in Swift 5, you can use the following simple extension:
// Activity Indicator Extension
var vSpinner : [UIView] = []

extension UIViewController {
    func showSpinner(onView : UIView) {
        let spinnerView = UIView.init(frame: onView.bounds)
//        spinnerView.backgroundColor = UIColor.init(red: 0.8, green: 0.8, blue: 0.8, alpha: 0.9)
        let ai = UIActivityIndicatorView.init(style: .medium)
        ai.startAnimating()
        ai.center = spinnerView.center

        DispatchQueue.main.async {
            spinnerView.addSubview(ai)
            onView.addSubview(spinnerView)
        }

        vSpinner.append(spinnerView)
    }

    func removeSpinner() {
        DispatchQueue.main.async {
            vSpinner.popLast()?.removeFromSuperview()
        }
    }
}

Within your async task use these two handy boys:
showSpinner(onView: self.view) Remember to replace self.view with any kind of view you desire
self.removeSpinner() Remove the loading indicator:


Project Euler #1 Sum of multiples of 3 and 5 Given:

  If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3, 5, 6 and 9. The sum of these multiples is 23. Find the sum of all the multiples of 3 or 5 below 1000.

Solution:

class Main {
  public static int sumMupltiple(int num, int limit) {
      int numOfMulitples = (limit - 1) / num;
      return num * (numOfMulitples * (numOfMulitples + 1) / 2);
  }

  public static void main(String[] args) {
    System.out.println(
        sumMupltiple(3, 1000) + sumMupltiple(5, 1000) - sumMupltiple(15, 1000));

  }
}

Is there a clearer approach for Java?

Meta comments

Is your intention to create a separate Java program for each of the hundreds of Project Euler problems? If you plan on completing more than a handful of them, I would recommend finding a way to consolidate your solutions to all problems into a single program. You will find a significant amount of overlap between certain questions, so being able to create "helper" routines will benefit you greatly. Consider adding in some testing, validation, and timing framework to make sure your results are correct and being solved within a reasonable amount of time (i.e., PE's suggested 1-minute duration).

Algorithmic Comments

As this is a code review, and not an algorithm review, and because I don't want to spoil yours or anyone else's Project Euler experience, I'll keep this brief. Your algorithm is good; you've already used the closed-form equation for these triangle numbers and done some math. You can actually take the math a step further, but then the programming is trivialized and entirely problem #1-specific.

Code Comments

Creating the general sumMultiple() routine is good, as it is general enough to probably prove useful later on.

I would highly recommend factoring this line into its own problem001() subroutine:

 sumMupltiple(3, 1000) + sumMupltiple(5, 1000) - sumMupltiple(15, 1000));

Then in your main() function, you need only do:

System.out.println(problem001());

And the self-documenting nature of that naming scheme makes it pretty clear that you are printing the result of problem 001. It might seem a little silly in this case, but not all problems will be one-liners, and consistency is probably better than brevity in this case.

If you do decide to take some of my "meta comments" advice later, it will be even easier to organize your program.

The given solution doesn't work for very large inputs like \$1<limit<10^9\$. Hence here is the solution using BigInteger.

import java.util.Scanner;
import java.math.BigInteger;

public class Solution {
    public static BigInteger sumMupltiple(int num, int limit) {
        BigInteger sum = BigInteger.ZERO;
        limit = limit - 1;
        BigInteger numMultiples = new BigInteger(""+limit).divide(new BigInteger(""+num));
        return sum.add(
            numMultiples.multiply(numMultiples.add(new BigInteger(""+1))).divide(new BigInteger(""+2)).
            multiply(new BigInteger(""+num)));
    }

    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);
        int T = input.nextInt();
        for (int i = 0; i < T; i++) {
            int N = input.nextInt();
            System.out.println(
                sumMupltiple(3, N).
                add(sumMupltiple(5, N)).
                subtract(sumMupltiple(15, N)));
        }
    }
}


Simple Temperature Converter 2 in C I finally got around to redoing my Temperature conversion program. I incorporated many elements of everyone's comments, and am uploading the finished program. Any new, or problems that I missed? Maybe a few very late here where I am working. Main focus of this redo was keeping my functions short and single purposed.

#include <stdio.h>

void inter_face();
void welcome();
void option_prompt();
void input_temp_prompt();
int get_option();
float get_input_temp();
void display_F_to_C(float);
void display_C_to_F(float);
void display_off();
void display_error();
float F_to_C(float);
float C_to_F(float);

enum { F_TO_C = 1, C_TO_F = 2, OFF = 3 } option_type;

int main()
{
    inter_face();
    getchar();
}

void inter_face()
{
    int option = -1;
    float input_temp = 0.0;
    welcome();
    while (option != OFF) {

        option_prompt();
        option = get_option();

        if (option == F_TO_C || option == C_TO_F) {
            input_temp_prompt();
            input_temp = get_input_temp();
        }
        switch (option) {
        case F_TO_C:
            display_F_to_C(F_to_C(input_temp));
            break;
        case C_TO_F:
            display_C_to_F(C_to_F(input_temp));
            break;
        case OFF:
            display_off();
            break;
        default:
            display_error();
        }
     }
 }

 void welcome()
{
    printf("Temperature conversion Calculator!!\nPlease select an option:");
    printf("\n1.) F to C\t2.) C to F\t3.) off\n\n");
}

void option_prompt()
{
    printf("Option: ");
}

void input_temp_prompt()
{
     printf("Temp: ");
}

float get_input_temp()
{
    float input_temp = 0;
    scanf_s("%f", &input_temp);
    return input_temp;
}

int get_option()
{
    int option;
    scanf_s("%d", &option);
    return option;
}

void display_F_to_C(float converted_temp)
{
     printf("Celsius: %f\n", converted_temp);
}

void display_C_to_F(float converted_temp)
{
    printf("Fahrenheit: %f\n", converted_temp);
}

void display_off()
{
    printf("OFF\n");
}

void display_error()
{
    printf("Incorrect input, try again\n");
}

float F_to_C(float input_temp)
{
      return (5.0 / 9.0) * (input_temp - 32);
}

float C_to_F(float input_temp)
{
    return (9.0 / 5.0) * (input_temp)+32;
}

Firstly, I don't quite understand why are you declaring so much functions that consist of a single line dedicated to printing a string. They are only used once and in one file, so there's no need to use a function or constant. I suggest you to get rid of them and replace calls to these functions with their bodies. Also, put break; inside default case (Here you can learn more about the reason why you should do this):

    switch (option) {
        case F_TO_C:
            printf("Celsius: %f\n", F_to_C(input_temp));
            break;
        case C_TO_F:
            printf("Fahrenheit: %f\n", C_to_F(input_temp));
            break;
        case OFF:
            printf("OFF\n");
            break;
        default:
            printf("Incorrect input, try again\n");
            break;
    }

Secondly, I recommend to use puts() instead of printf() when printing simple strings as it is obviously simpler and places a newline automatically (And you use it often throughout your code). Note that this doesn't involve cases when printing formatted strings.

    switch (option) {
        case F_TO_C:
            printf("Celsius: %f\n", F_to_C(input_temp));
            break;
        case C_TO_F:
            printf("Fahrenheit: %f\n", C_to_F(input_temp));
            break;
        case OFF:
            puts("OFF"); // No need for printf()!
            break;
        default:
            puts("Incorrect input, try again"); // No need for printf() as well!
            break;
    }

Thirdly, I'd suggest to move the body of inter_face() function to main() as it is not used anywhere else and basically creates a run loop, so I don't see any reasons why you should place it in a separate function. In addition to this, you probably want to define your functions before they're used, thus moving all functions above main(). It's not necessary as the compiler will find them anyway, yet I reckon the source code looks better that way (Just imagine reading a book from the end to the beginning).

float F_to_C(float input_temp)
{
      return (5.0 / 9.0) * (input_temp - 32);
}

float C_to_F(float input_temp)
{
    return (9.0 / 5.0) * (input_temp)+32;
}

float get_input_temp()
{
    float input_temp = 0;
    scanf_s("%f", &input_temp);
    return input_temp;
}

int get_option()
{
    int option;
    scanf_s("%d", &option);
    return option;
}

int main()
{
    int option = -1;
    float input_temp = 0.0;

    printf("Temperature conversion Calculator!!\nPlease select an option:");
    printf("\n1.) F to C\t2.) C to F\t3.) off\n\n");

    while (option != OFF) {

        printf("Option: ");
        option = get_option();

        if (option == F_TO_C || option == C_TO_F) {
            printf("Temp: ");
            input_temp = get_input_temp();
        }

        switch (option) {
            case F_TO_C:
                printf("Celsius: %f\n", F_to_C(input_temp));
                break;
            case C_TO_F:
                printf("Fahrenheit: %f\n", C_to_F(input_temp));
                break;
            case OFF:
                puts("OFF"); // No need for printf()!
                break;
            default:
                puts("Incorrect input, try again"); // No need for printf() as well!
                break;
        }
    }

    getchar();
}

Fourthly, you should put function prototypes, enumerations and other objects that carry data (Of course, prototypes do not carry data, yet they belong in header files) into a separate header file (For instance, main.h). That way your source file looks a lot cleaner, and if someone wants to use these functions elsewhere, he can easily find this header file with needed prototypes within.

Fifthly, I'd consider either merging or getting rid of get_option() and get_input_temp() functions as they perform the same task, just with different data types. I suggest to create a single function called get_input() that returns a float (Since float can be compared to int without requiring you worry about loosing precision) instead if you don't want to put them inside main() as we did with other redundant functions before. You may then compare these variables normally as usual arithmetic conversions from int to float will occur when comparing float to int (You can learn more about such conversions here, here and here).

#include "main.h" // Prototypes and enum are there!

float F_to_C(float input_temp)
{
      return (5.0 / 9.0) * (input_temp - 32);
}

float C_to_F(float input_temp)
{
    return (9.0 / 5.0) * (input_temp)+32;
}

float get_input(void)
{
    float input = 0;
    scanf_s("%f", &input);
    return input;
}

int main()
{
    int option = -1;
    float input_temp = 0.0;

    printf("Temperature conversion Calculator!!\nPlease select an option:");
    printf("\n1.) F to C\t2.) C to F\t3.) off\n\n");

    while (option != OFF) {

        printf("Option: ");
        option = get_input();

        if (option == F_TO_C || option == C_TO_F) {
            printf("Temp: ");
            input_temp = get_input();
        }

        switch (option) {
            case F_TO_C:
                printf("Celsius: %f\n", F_to_C(input_temp));
                break;
            case C_TO_F:
                printf("Fahrenheit: %f\n", C_to_F(input_temp));
                break;
            case OFF:
                puts("OFF"); // No need for printf()!
                break;
            default:
                puts("Incorrect input, try again"); // No need for printf() as well!
                break;
        }
    }

    getchar();
}

Also, be sure to check chux's answer. It contains even more useful tips and tricks to improve your code. Especially scanf_s() checks, option = -1 change and math simplification.

Similar to ChatterOne, i would suggest to consolidate your functions:

int get_option()
{
    printf("Option: ");
    int option;
    scanf_s("%d", &option);
    return option;
}

float get_input_temp()
{
    printf("Temp: ");
    float input_temp = 0;
    scanf_s("%f", &input_temp);
    return input_temp;
}

Also it is necessary to check whether the provided input is valid:

int get_option()
{
    printf("Option: ");
    int option;
    scanf_s("%d", &option);
    if (option == F_TO_C || option == C_TO_F || option OFF) {
        return option;
    } else {
        printf("Incorrect input, try again\n");
        return get_option();
    }
}

Note, that i have added an recursive call to get_option in the error case. Similar changes should apply for get_input_temp().


How clean is my close_account code? Hello i'm coding a social site and I need to know how clean this code is. I read other answers people told me from my previous questions like this and used it as guidelines. So now I wanna know if anything could be improved. Thank you.
<?php

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
error_reporting(-1);

include 'includes/header.php';

$userLoggedIn = $_SESSION['thesocial_username'];

$post_id = isset($_POST['cancel']) ? header("Location: settings.php") : '';

if(isset($_POST['close_account'])) {

    $close_query = $con->prepare('UPDATE users SET user_closed = "yes" WHERE username = ?');
    $close_query->bind_param("s", $userLoggedIn);
    $close_query->execute();

    session_destroy();
    header("Location: register.php");
}

?>

Close Account

    Are you sure you want to close your account ?

    Closing your account will hide your profile and activity from users.

    You can reopen your account at anytime by logging in.

    <form action="close_account.php" method="POST">

        <input type="submit" name="close_account" id="close_account" value="Yes, Close my account.">
        &nbsp;&nbsp;
        <input type="submit" name="cancel" id="update_details" value="Nevermind.">

    </form>

The code is overall good basic PHP and above the average. However, I would like it more structured.
For example, why you are setting the error reporting manually in this file? Apparently you have sort of a bootstrap file as includes/header.php, why not to put these lines there?
Besides, i don't see any point in the line assigning $post_id variable. What does a post id to do with deleting an account? And yes, such a syntax structure looks embarrassing.
It is also a good rule of thumb to follow Location header with an explicit exit call. Although it is not critical in this particular case, sometimes the lack of the script termination could pose a serious threat, as the header itself doesn't stop the script execution.
Some notes on the database structure

The computer world has its own representation for "yes" and "no", it's 1 and 0 respectively. So consider making the field of tinyint type instead of varchar.
it is considered a good practice to address all database entities by the unique id. It's better to store the user id in the session and use it for all the database operations.

The code structure
One big issue I overlooked at first is the structure of your script. It seems that includes/header.php is not just a bootstrap file but it also starts some HTML output. that's a big NO. An HTTP header such as Location cannot be sent after HTML. Your PHP lets you to do it due to some permissive setting, but it is nevertheless wrong, from the logical and technological points of view. It just makes no sense to start any output when your script is not going to display anything but would only redirect to another page.
Split your includes/header.php file into two: bootstrap.php and header.php. The former should contain all the configuration options such as database connection, error reporting etc, but not a byte should it output. And the latter would contain all the HTML.
Than make your code like this
<?php

include 'includes/bootstrap.php';

$id= $_SESSION['user_id'];

if(isset($_POST['close_account']))
{
    $stmt = $con->prepare('UPDATE users SET user_closed = 1 WHERE id = ?');
    $stmt->bind_param("s", $id);
    $stmt->execute();

    session_destroy();
    header("Location: register.php");
    exit;
}
include 'includes/header.php';
?>
<center>

To add to "Your Common Sense" answer:

I would suggest that the next step would be to separate your HTML and PHP code.

i.e. moving out the logic for account closure into its own class since you might want to use that logic elsewhere. (DRY Principle)

It's best to let JS/HTML handle redirects too since that is something related to the front end facing application.

In a larger application, speed is of the essence and every data byte may count, due to which I would suggest declaring variables only when and where you need them.

i.e.

$userLoggedIn is used within an if statement, yet it was declared outside of an if statement.
$userLoggedIn is used only once. The question to ask is - does having this variable improve the readability of my code? If not, then just use $_SESSION['thesocial_username'].


Programming Challenge from Kattis: Watchdog I've tried to solve the kattis-challenge called Watchdog. My code works just fine, but it was too slow on the last test. I'm wondering if anyone sees any big algorithmic performance issues.

A short description of the task: First line from .in states number of test-cases.
First line in each test-case gives length of a square and number of hatches respectively. For each hatch you'll have one line giving x and y-point of hatch respectively. The mission is to find a point (x,y) where the length from (x,y) to any hatch is not greater than the length from (x,y) to any point outside of the square. If no such point, print poodle. If several such point print point with lowest x, if several points with equal x, print point with lowest y. Hatches and these points cannot overlap.

I think one issue might be extensive usage of lists, but I'm also not sure how to avoid them. Initially I started reading one and one line from stdin using the data. That way things looked way more readable, but it was also slower. And it's the speed that I'm having issues with.

Example input:

3
10 2
6 6
5 4
20 2
1 1
19 19
10 3
1 1
1 2
1 3

Example output:

3 6
poodle
2 2
2 2

My code:

import sys
from math import sqrt
import itertools

def findmaxdist(x1, y1, list):
    '''Finds the distance from x1, y1 to the most
    distant point in the list.'''
    dist = 0
    for x2, y2 in list:
        newdist = (x2-x1)*(x2-x1)+(y2-y1)*(y2-y1)
        if newdist>dist:
            dist=newdist
    return sqrt(dist)

fulltext = sys.stdin.readlines()
text = [w.rstrip('\n') for w in fulltext]

cases = int(text[0])
i = 0
j = 1
while i < cases:
    side, hatches = text[j].split()
    side = int(side)
    hatches = int(hatches)
    j += 1
    hatch_list = list()
    for k in range(hatches):
        x, y = text[j].split()
        hatch_list.append((int(x), int(y)))
        j += 1

    possibles = list()
    for x,y in itertools.product(range(side), range(side)):
        if (x, y) not in hatch_list:
            dist = findmaxdist(x, y, hatch_list)
            if x+dist<=side and x-dist>=0:
                if y+dist<=side and y-dist>=0:
                    possibles.append((x, y))

    if len(possibles)==0:
        print('poodle')
    elif len(possibles)==1:
        print(str(possibles[0][0])+' '+str(possibles[0][1]))
    elif len(possibles)>1:
        smallx = min(possibles, key = lambda t: t[0])[0]
        semifinal = list((tuple for tuple in possibles if tuple[0] == smallx))
        if len(semifinal)==1:
            x,y = semifinal[0]
            print(str(x)+' '+str(y))
        elif len(semifinal)>1:
            smally = min(semifinal, key = lambda t: t[1])[1]
            final = list((tuple for tuple in semifinal if tuple[1] == smally))
            if len(final)==1:
                x,y = final[0]
                print(str(x)+' '+str(y))
        else:
            print('Error: Wrong input-format?')
    i+=1

Your solution is right, but unfortunately the time limit enforces a certain optimization. The key of this is where it says:

  If several such point print point with lowest x, if several points with equal x, print point with lowest y

In your case, you just append all possible candidate and then you have a very complicated way to find the lowest. First of all, this complicated way could have been reduced to sorting and returning first element.

If we think a bit further on how does itertools.product work, it will first iterate over x and then over y. This means that the first candidate we find, is the one with lowest x and lowest y of those x. This means 2 things: you don´t even need that sort, the answer will always be the first candidate.

Now if the answer is always the first candidate, why are we keeping all candidates? And why are we still calculating other possible candidates when we already have the answer? Keeping this in mind, the solution to your time limit is simple adding a break after the possibles.append((x, y)) line. This will make us stop processing when we find the answer, and will save enough time for the time limit.

This means all the elif len(possibles)>1: part is unnecessary, making it shorter and cleaner. Another simple optimization is changing hatch_list to a set instead of a list so that the if (x, y) not in hatch_list: takes less than O(n). Its a small change that only requires changing list() to set() and append to add.

There might be more optimization to make but the one of the break is the key to the problem and the expected optimization to make to be able to not have time limit.

To specifically answer your question, you'd need to use a profiler to look at the code and see how long it takes.

cprofilev 20190401a.py < 20190401a.txt
     1273 function calls in 0.001 seconds

Ordered by: cumulative time

ncalls  tottime  percall  cumtime  percall filename:lineno(function)
    1    0.000    0.000    0.001    0.001 {built-in method builtins.exec}
    1    0.000    0.000    0.001    0.001 20190401a.py:1(<module>)
  593    0.000    0.000    0.000    0.000 20190401a.py:6(findmaxdist)
    1    0.000    0.000    0.000    0.000 {method 'readlines' of '_io._IOBase' objects}
  593    0.000    0.000    0.000    0.000 {built-in method math.sqrt}
    3    0.000    0.000    0.000    0.000 {built-in method builtins.print}
    2    0.000    0.000    0.000    0.000 /usr/lib/python3.5/codecs.py:318(decode)
    2    0.000    0.000    0.000    0.000 {built-in method builtins.min}
    1    0.000    0.000    0.000    0.000 20190401a.py:18(<listcomp>)
    1    0.000    0.000    0.000    0.000 <frozen importlib._bootstrap>:996(_handle_fromlist)
   22    0.000    0.000    0.000    0.000 {method 'append' of 'list' objects}
    4    0.000    0.000    0.000    0.000 20190401a.py:48(<genexpr>)
    1    0.000    0.000    0.000    0.000 {built-in method builtins.hasattr}
   11    0.000    0.000    0.000    0.000 {method 'rstrip' of 'str' objects}
   10    0.000    0.000    0.000    0.000 {method 'split' of 'str' objects}
    2    0.000    0.000    0.000    0.000 {built-in method _codecs.utf_8_decode}
   15    0.000    0.000    0.000    0.000 20190401a.py:47(<lambda>)
    1    0.000    0.000    0.000    0.000 {method 'disable' of '_lsprof.Profiler' objects}
    9    0.000    0.000    0.000    0.000 {built-in method builtins.len}

I ran cprofile a few times and the function call numbers are static. From the look of your code, we're seeing multiple requests to findmaxdist and math.sqrt, there might be an opportunity to clean that up.
There are 22 appends to a list object, which if we look at "Common Data Structure Operations" (see http://bigocheatsheet.com/), which might be an inefficient data structure for what you're trying to achieve.

Looking at your code, regardless of speed, having it all as one giant blob instead of neatly separated into functions (look up the Single Responsibility Principal) can make your code execute faster as functions compile down neatly and very efficiently - especially when written in a functional style of programming (no state maintained in the function after the function completes).

Other than streamlining the calls and changing your data structure, I don't think you'll see much speed improvement with the code as-in. I hope this helps somewhat?
Good luck!


Feedback on a programming practice problem in C This is a programming practice that our teacher gave us, and I would appreciate if someone can look over my program and tell me if I did a good job.

Basically, the context is I joined a company and am supposed to write a program in C that takes three inputs (ft of steel, number of ball bearings, and lbs of bronze), and returns how many bells and whistles can be made the next day.

Making a whistle requires 0.5 ft of steel and 1 ball bearing. Making a bell requires 1.10 lbs of bronze (.75 lbs of bronze and 1 clapper (.35 lbs of bronze)).

This is the program I have written.  I apologize if it seems messy.

#include <stdio.h>
#include <stdbool.h>

void PrintLine();
int CalculateWhistles(int steel, int bearings);
int CalculateBells(int bronze);

int main() {
    bool runProgram = true;
    int runAgain = 0;

    while (runProgram == true) {
        int ftSteelIn = 0;
        int numBallBearingIn = 0;
        int ibsBronzeIn = 0;

        printf("\n\n"); // Formatting...
        PrintLine();

        printf("    How many feet of steel was received today:               "); // Steel received
        scanf("%d", &ftSteelIn);
        printf("    How many ball bearings were received today:              "); // Ball bearings received
        scanf("%d", &numBallBearingIn);
        printf("    How many pounds of bronze was received today:            "); // Bronze received
        scanf("%d", &ibsBronzeIn);

        PrintLine(); // Print line

        PrintLine(); // Print line

        // Print company logo and program title
        printf("                                Acme Corporation\n");
        printf("                                Product Report\n");

        PrintLine();// Print line

        // Print number of bells and whistles
        printf("    Number of Bells                                                      %d\n", CalculateBells(ibsBronzeIn));
        printf("    Number of Whistles                                                   %d\n", CalculateWhistles(ftSteelIn, numBallBearingIn));

        PrintLine(); // Print line
        PrintLine();

        printf("\n\n");
        PrintLine();

        // Ask the user if they would like to run the program again
        printf("    Would you like to run the program again? ('1' for yes, '2' for no): ");
        scanf("%d", &runAgain);
        PrintLine();

        // If user does not say 'yes', end the program and say goodbye
        if (runAgain != 1) {
            printf("\n    Exitting the program. Have a good day.\n\n");
            PrintLine();
            runProgram = false;
        }
    }

    return 0;
}

void PrintLine() {
    int i = 0;
    printf("    ");
    while (i <= 70) {
        printf("-");
        i++;
    }
    printf("\n");
}

int CalculateWhistles(int steel, int bearings) {
    steel = steel / 0.5;
    if (steel < bearings) {
        return steel;
    }
    else if (bearings < steel) {
        return bearings;
    }
}

int CalculateBells(int bronze) {
    int bells = bronze / 1.1;
    return bells;
}

It runs fine with no errors. There is a second part where I calculate the remainder, but I want some feedback before I go on to that part.

Any advice would be appreciated, how to make it more efficient or anything like that (I'm just learning C).

Roughly 50% of all programming is about dealing with errors. You say your code works, but it doesn't handle errors and therefore doesn't work.

Example 1:

How many feet of steel was received today:  four

Program doesn't say "Error, try again". Instead it displays all prompts without waiting for any user input and exits.

Example 2:

How many feet of steel was received today:  3.7

Program doesn't say "Error, try again". Instead it displays all prompts without waiting for any user input and exits.

Example 3:

How many feet of steel was received today:  3 4

Program doesn't say "Error, try again". Instead it assumes 3 feet of steel; displays "How many ball bearings" and doesn't wait for user input and assumes 4 ball bearings.

Example 4:

Would you like to run the program again? ('1' for yes, '2' for no): yes

Program doesn't say "Error, try again". Instead it exits.

Example 5:

How many feet of steel was received today:               -99
How many ball bearings were received today:              -99
How many pounds of bronze was received today:            -99

Program decides you can make -89 bells and -198 whistles.

Example 6:

How many feet of steel was received today:               999999999999999999
How many ball bearings were received today:              999999999999999999
How many pounds of bronze was received today:            999999999999999999

Program decides you can make -1351471477 bells and -2147483648 whistles.

Example 7:

How many feet of steel was received today:               200
How many ball bearings were received today:              100

Program decides you can make 0 whistles. Note: unlike the previous examples, this is not a "failed to check for invalid input" problem.

Do you really want to make these calculations with integers? Shouldn't you be using floating point variables?

I'm impressed on how every other answer focused on esthetics but didn't comment on the most critical problem of your code: it doesn't work even with correct input.

edit: Actually, other answers touched the issue, it was I that failed to notice that this is not exactly a bug, since using ints do achieve the desirable answer.


Binary search of an integer array I'm a beginner. This is my implementation of binary search algorithm in Python. Please review my code and give your valuable inputs for further improvements on overall coding style, rare cases etc.

def binary_search(arr,value):
    left = 0
    right = len(arr) - 1
    while(left <= right):
        mid = (left + right) // 2
        print(mid)
        if arr[mid] is value:
            return mid
        elif value < arr[mid]:
            right = mid - 1
        elif value > arr[mid]:
            left = mid + 1

    return -1

def print_binary_search(arr,value):
    pos = binary_search(arr,value)
    if pos is -1:
        print("Element not found")
    else:
        print("Element found in the positon " +str(pos))

def test1():
    arr = [2,5,8,9,11,15,17,19,22,26,29,33,56]
    value = 56
    print_binary_search(arr,value)

def test2():
    arr = [2,5,8,9,11,15,17,19,22,26,29,33,56]
    value = 59
    print_binary_search(arr,value)

test1()
test2()

if arr[mid] is value:

You don't want to compare two integers with is, which compares identity. Use == instead. is will break your code when using large or small numbers (\$<-5\$, \$>256\$).

For example, using is with this "test" function:

def test1():
    arr = list(range(257, 270))
    value = 260
    print_binary_search(arr, value)

Results with an infinite loop.

Using == works as expected and returns 3.

The reason is that Python uses integer caching for the range \$[-5, 256]\$.

And some PEP8 nitpicking:

while(left <= right):

The parenthesis around the condition are useless in this case, you should prefer a whitespace:

while left <= right:

You are missing some whitespaces after commas in function definition, function calls, list literals and around operators.

For example, your code should look like this (relevant lines only):

def print_binary_search(arr, value):

#######

pos = binary_search(arr, value)

#######

arr = [2, 5, 8, 9, 11, 15, 17, 19, 22, 26, 29, 33, 56]

#######

print("Element found in the position " + str(pos))

and according to a recent hot discussion on SO.

you should use mid = left + (right-left)/2 instead of mid = (left+right)/2 to avoid possible overflows


Changing background color of HTML elements based on user input This is jQuery code which works like this: depending on a user choice it changes the color and background of some elements in the DOM.

The behavior is ok, is doing exactly what I want.

And as you may see, there is some code repeated. What's the best way to keep a good performance by doing a function in order to avoid repeating code?

function ArticleQuiz($el){
    this.$el = $el;
    this.answer = this.$el.data('answer');
    this.answerContainer = this.$el.find('.answer-container');
    this.result = this.$el.find('.result');
    this.correctAnswer = this.$el.find('.correct-answer');
    this.incorrectAnswer = this.$el.find('.incorrect-answer');
    this.quizLogic();
    return this;
}

ArticleQuiz.prototype.quizLogic = function(){
    var THIS = this;
    var answer;

    $('.masthead-article-quiz').parent().css('background', '#eee');
    $('.quiz-cols a', this.$el).click(function(e) {
        THIS.target = $(e.target);
        e.preventDefault();
        // exit if choice already made: users can't change their pick
        if ($(this).parents('.quiz-cols').parent().find('.white-font').length) return;
        // set class according to data-answer:
        if (THIS.target.hasClass('icon-false-shape')) {
            THIS.answerContainer.show();
            $(this).addClass('background-' + ((THIS.answer === false) ? 'green' : 'red')).addClass('white-font');
            THIS.result.addClass('font-' + ((THIS.answer === false) ? 'green' : 'red'));
            answer =  THIS.answer ? THIS.incorrectAnswer.show() : THIS.correctAnswer.show();
        }
        if (THIS.target.hasClass('icon-true-shape')) {
            THIS.answerContainer.show();
            $(this).addClass('background-' + ((THIS.answer === true) ? 'green' : 'red')).addClass('white-font');
            THIS.result.addClass('font-' + ((THIS.answer === true) ? 'green' : 'red'));
            answer =  !THIS.answer ? THIS.incorrectAnswer.show() : THIS.correctAnswer.show();
        }
    });
    return THIS;
};

The first thing that jumps out at me is the tight coupling it has with the current application in which it is being used.  Why hard code all those CSS selectors?  This class really cries out for configuration.

You might see a pattern like this used to achieve decoupling of class from config.

function ArticleQuiz($el, $config){

   this.config = {};
   ...
   init() {
       $.extend(this.config, this.defaultConfig, $config || {});
       ...
   }
   this.init();
}

ArticleQuiz.defaultConfig = {
    answerDataLabel = 'answer',
    answerContainerSelector = '.answer-container',
    ...
}

You should consider taking the approach of using meaningful class names to apply multiple CSS styles at once.  So, rather than using specific semantic color and font class names, define a CSS class that encapsulates the entire state of being a correct/wrong answer.  So taking this approach along with the suggestion to store configuration something might allow you to change this:

        $(this).addClass('background-' + ((THIS.answer === false) ? 'green' : 'red')).addClass('white-font');
        THIS.result.addClass('font-' + ((THIS.answer === false) ? 'green' : 'red'));
        answer =  THIS.answer ? THIS.incorrectAnswer.show() : THIS.correctAnswer.show();

Into something like:

       var classToApply = THIS.config.correctAnswerClass;
       if(THIS.answer === true) {
           classToApply = THIS.config.incorrectAnswerClass;
       }
       $(this).addClass(classToApply).show();

Your QuizLogic function would not seem to have reason to be in the static (prototype) context, as you are using it as a constructor for your class. I would think this function should be within class function scope instead on prototype and should be call something like init() or similar common name for constructors in javascript.  Note how I present the init() function in my first code example above.

Consider first populating your instance properties in the constructor rather than in the main class code.  This gives you the ability to do things such as validate that the selectors do not return empty collections and similar activities needed to put the object in the proper state for use. If these validations fail, you could throw an error to prevent the object from being used in an improper state.

You can use angular directives, for example:

<input type="button" value="set color" ng-click="myStyle={color:'red'}">
<input type="button" value="set background" ng-click="myStyle={'background-color':'blue'}">
<input type="button" value="clear" ng-click="myStyle={}">

Sample Text

myStyle={{myStyle}}

Example


Better way to write this code in functional manner using map and reduce? I have an array of items on which I have to perform 2 tasks:

Applying  a function on the array item
Checking if the value is true or false.

The functional approach to solving this would be

var result = arr.map(function(item){return some_action(item);}).filter(function(item){return other_action(item) == true;});

But here the array arr is traversed twice in comparison to

var result = [];
arr.forEach(function(item){
    var x = other_action(some_action(item));
    if (x)
        result.push(x);
});

Isn't the functional approach bad in this case or am I not using map and filter the right way?

Your “functional” approach is overly complicated. Notice that this is completely equivalent:

var result = arr.map(some_action).filter(other_action);

I.e. If you're only delegating to another function, you can specify that function directly. Also, an == true test is superfluous.

Your “procedural” variant is not equivalent, that would have to be:

var result = [];
arr.forEach(function(item){
    var changedItem = some_action(item);
    if (other_action(changedItem))
        result.push(changedItem);
});

Note that both variants have the same algorithmic complexity, and that the cost of iteration is likely negligible compared with the cost of some_action and other_action.

Functional programming does not mean unreadable code. Even if you're not just delegating to another function, you could improve formatting, e.g. to

var result = arr.map(function (item) {
  return some_action(item);
}).filter(function (item) {
  return other_action(item);
});

Don't do this:

 var result = arr.map(function(item){
          var result=some_action(item);
          if(other_action(result)){
             return result;
          }
  });

if other_action() return false, the item position in array will be undefined.


Project Euler 23: Non Abundant Sums Here is my solution to Project Euler 23: Non Abundant Sums. I know I'm doing the Divisors function brute force, but I can't seem to get it to work any other way. Any improvements are welcome.

#include <iostream>
#include <vector>

//Calculates divisors
void Divisors(unsigned number, std::vector<unsigned> &result){
    for(unsigned a = 1; a < number; a++){
        if(number % a == 0){
            result.push_back(a);
        }
    }
}
//Calls for divisors and adds them
unsigned SumOfDivisors(unsigned number){
    unsigned result = 0;
    std::vector<unsigned> divisors;
    Divisors(number, divisors);
    for(unsigned a = 0; a < divisors.size(); a++){
        if(divisors[a] < number)
            result += divisors[a];
    }
    return result;
}
//Calls calculates the sum of divisors to see if it is Abundant
bool IsAbundant(unsigned number){
    if(SumOfDivisors(number)> number){
        return true;
    }
    else{
        return false;
    }
}
//pushes back abundant numbers
void Abundant(unsigned limit, std::vector<unsigned> &abunNumbers){
    for(unsigned a = 1; a < limit; a++){
        if(IsAbundant(a)){
            abunNumbers.push_back(a);
        }
    }
}

//Find's all number's that are the sum of two abundant numbers
void SumOfAbundant(unsigned limit, std::vector<unsigned> const& abunNumbers, std::vector<bool> &sumAbun){
    for(unsigned a = 0; a < abunNumbers.size(); a++){
        for(unsigned b = a; b < abunNumbers.size(); b++){
            if(abunNumbers[a]+abunNumbers[b] <= limit){
                sumAbun[abunNumbers[a]+abunNumbers[b]] = true;
            }
        else{
            break;
        }
        }
    }
}

int main(){
    const unsigned limit = 28123;
    unsigned result = 0;

    std::vector<unsigned>abunNumbers;
    Abundant(limit, abunNumbers);

    std::vector<bool>sumAbun(limit,false);
    SumOfAbundant(limit, abunNumbers, sumAbun);

    for(unsigned a = 0 ; a < sumAbun.size() ; a++){
        if(!sumAbun[a]){
        result += a;
        }
    }
    std::cout << result;
}

I would recommend the following changes:

Math is your friend. Factorize the number into prime powers (a precalculated table of primes is of immence value for a lot of Project Euler problems), and use a formula from this article.
Memoize. As soon as you identify a single (prime) divisor of a number, and take the quotient, notice that the quotient was already inspected and factorized. A vector of tuples (prime, power) representing a factorization may also help a lot.

There could be other optimizations, but these two seem to be enough.

The problem states that all numbers greater than 28123 are out, so you only need to consider numbers up to that limit.  That's not a lot of numbers.

Having said that, each project Euler problem involves a new math skill.  I'll provide some optimization hints.

In this case, the math you need is factorization.  You are dividing by every number from 1 to n-1 in order to factorize n.  You don't have to do that.

All numbers are divisible by 1, so don't even test it. You can start your count at 1 instead of zero.
You are summing up the factors, so don't build a list, build a running total. That will reduce a lot of inefficiency.  In fact, you overuse lists in several places: not only are you building a list of factors, but you are building a list of the numbers you want.  Nothing in this problem said you had to build a list of anything, just sum the numbers you find.  Granted, the only one that really affects the timing is the list of factors.
You only need to go up to the square root of a number to factor.  Consider 28: divide by 1, get 28 (which by the way doesn't count). Divide by 2, get 14. Divide by 4, get 7.  You don't have to divide by 7, because you already got it by dividing by 4. That cuts the problem to \$O(\sqrt{n})\$ rather than \$O(n)\$.

The suggestion above is still a brute-force approach, but it's a more efficient brute-force solution.


I can't fault your tags at all. The only problem is that due to your table being quite complex, assistive technology may not be able to figure out the structure of your table.

I was going to recommend using scope but according to W3C there are some inconsistency issues.

  The row and col values of the scope attribute are currently supported
  to a large extent by most current versions of JAWS. However, there are
  still some problems and WindowEyes support for scope is inconsistent.
  The same is true for Japanese versions of these screen readers.
  Versions of JAWS prior to version 5 and older versions of Window-Eyes
  have inconsistent support for scope.

Using scope would look like this, with scope="col" in all  elements in the first row and scope="row" for all other  elements.

            th 1
            th 2
            th 3
            th 4

            th 5
            th 6
            th 7
            td 1
            td 2
            td 3

            td 4
            td 5
            td 6

            th 8
            td 7
            td 8
            td 9

            td 10
            td 11
            td 12

            th 9
            th 10
            td 13
            td 14
            td 15

            td 16
            td 17
            td 18

            th 11
            td 19
            td 20
            td 21

            td 22
            td 23
            td 24

JSFiddle for scope method

This is W3C's recommendation:

  At the current time, those who want to ensure consistent support
  across Assistive Technologies for tables where the headers are not in
  the first row/column may want to use the technique for complex tables
  H43: Using id and headers attributes to associate data cells with
  header cells in data tables. For simple tables that have headers in
  the first column or row we recommend the use of the th and td
  elements.

Let me just start off by saying this is a complete pain in order to be completely accessibly and scope will probably be fine in most cases. Here is the table using the id/header method (shudder):

            th 1
            th 2
            th 3
            th 4

            th 5
            th 6
            th 7
            td 1
            td 2
            td 3

            td 4
            td 5
            td 6

            th 8
            td 7
            td 8
            td 9

            td 10
            td 11
            td 12

            th 9
            th 10
            td 13
            td 14
            td 15

            td 16
            td 17
            td 18

            th 11
            td 19
            td 20
            td 21

            td 22
            td 23
            td 24

JSFiddle for id/headers

What is comes down to in the end in my opinion is an accessible solution that is also maintainable in the back end. Realistically JAWS and WindowEyes will eventually fix up their bugs and using scope will be fine even for tables as complex as this. Unless you expect many users to use assistive technologies and require strict adherence to WCAG, go with scope. Markup needs to be beautiful too :)

There is also the <colgroup> element which is relevant but support for it isn't great currently.

Plan it out on paper with percentages, and then take it row by row in an editor, inserting the content as you go. Otherwise you can't do it at all. And the more cells and custom rowspan/colspan you use with coding non-CSS table design, the time taken to code it all increases exponentially.