A tiny command-line program that prints a sum of numbers Here's my first Rust program that I could actually use for something:

use std::io;
use std::process;

fn main() {
    let mut sum = 0;
    loop {
        let mut number_str = String::new();
        match io::stdin().read_line(&mut number_str) {
            Ok(n) => { if n == 0 {break} },
            Err(e) => { println!("ERROR: got '{}' when reading a line", e) }
        }
        match number_str.trim().parse::<i32>() {
            Ok(n) => { sum += n }
            Err(n) => {
                println!("ERROR: Entered something that is not a number: '{}'",
                    number_str.trim_right());
                process::exit(1)
            },
        }
    }
    println!("{}", sum);
}

Did I violate any style rules? Is there anything else I should add/change/improve?

You have a compiler warning. One of the points of using a statically typed language is to have the compiler watching your back. If you ignore what the compiler says, you lose out.

warning: unused variable: `n`, #[warn(unused_variables)] on by default
  --> main.rs:14:17
   |
14 |             Err(n) => {
   |                 ^

If you don't care about the value, use an underscore (_).
Learn and love Rustfmt.

There are no braces on one-line match arms.
Always use trailing commas on multiple line items.
Use spaces inside braces ({ break }).

If the current code encounters an error while reading input, it continues processing. That seems unintuitive.
The current code uses trim when parsing, but only trim_right when showing the error to the user. This seems like a way of confusing the user when the error occurs.
It's potentially inefficient to reallocate the string in each iteration. Moving it outside the loop would mean you are responsible for clearing it each iteration. Failing to clear it would be a bug and potential security hole.
However, the code to process each line is already implemented for you.
There's duplicate error handling code. I'd extract a method to be able to use Result. I love using quick_error to construct error enumerations. This lets you use try! or ?.

#[macro_use]
extern crate quick_error;

use std::io::{self, BufRead};
use std::process;

use quick_error::ResultExt;

quick_error! {
    #[derive(Debug)]
    enum Error {
        Io(err: io::Error) {
            from()
            description("unable to read line")
            display("Unable to read line: {}", err)
            cause(err)
        }
        Parse(err: std::num::ParseIntError, num: String) {
            description("unable to parse number")
            display("Unable to parse '{}': {}", num, err)
            cause(err)
            context(num: &'a str, err: std::num::ParseIntError) -> (err, num.into())
        }
    }
}

fn inner_main() -> Result<i32, Error> {
    let mut sum = 0;

    let stdin = io::stdin();
    for line in stdin.lock().lines() {
        let line = line?;
        let line = line.trim();
        sum += line.parse().context(line)?;
    }

    Ok(sum)
}

fn main() {
    match inner_main() {
        Ok(sum) => println!("{}", sum),
        Err(e) => {
            println!("ERROR: {}", e);
            process::exit(1);
        }
    }
}


UserScript to Highlight tab for queue item It may seem a bit obsessive and/or pedantic, but the following script allows me to see when posts are ready for reviewing.
I developed this UserScript for use with TamperMonkey to allow me to have a tab open for a review queue, and then when an item is ready for review, the tab will be highlighted with an asterisk on the far left. Until an item is ready, an animation effect is applied to the title to show that a timer is currently running before refreshing the page. I have considered inspecting the number of action buttons to discern if an item is ready to be reviewed instead of inspecting the URL but this technique seems to work fine.
Questions:

How does the ecmascript-6 usage look?
What would you change and/or code differently?

// ==UserScript==
// @name         Highlight active review
// @namespace    http://tampermonkey.net/
// @version      0.1
// @description  Bring attention to the user when a post is up for reviewing
// @author       Sam Onela
// @match        https://codereview.stackexchange.com/review/*
// ==/UserScript==

;(function() {
    'use strict';
    const INITIAL_CHECK_TIMEOUT = 1500;
    const INTERVAL_DELAY = 400;
    const RELOAD_DELAY = 6500;
    const CLICK_CHECK_DELAY = 6000;
    const ticks = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏'];
    let timeout = window.setTimeout(check, INITIAL_CHECK_TIMEOUT);
    let interval;
    function updateTitle(showAsterisk = false) {
        var titleMatches = document.title.match(/^([\W]{1})Review/);
        if (titleMatches && titleMatches.length > 1) {
            const index = ticks.indexOf(titleMatches[1]);
            document.title =  document.title.replace(titleMatches[1], ticks[(index+1)%ticks.length]);
        }
        else {
            document.title =  (showAsterisk?'*': ticks[0]) + document.title;
        }
    }
    function check(clickHandlerAdded = false){
        if (timeout) {
            window.clearTimeout(timeout);
            timeout = 0;
        }
        if (window.location.pathname.match(/\d+/)) { // drilled in to a review item
            updateTitle(true);
            if (!clickHandlerAdded) {
                document.addEventListener('click', (event) => timeout = window.setTimeout(check, CLICK_CHECK_DELAY, true));
            }
        }
        else { //add spinner animation and then reload
            updateTitle();
            if (interval === undefined) {
                interval = window.setInterval(updateTitle, INTERVAL_DELAY);
            }
            window.setTimeout(_=> window.location = window.location, RELOAD_DELAY);
        }
    }
})();

Follow-up (meta) question
Perhaps this is too meta (and could be asked in a question on CR meta), and might not be seen by target user, but I'll ask anyway: What is the reason for the downvote? If something is unclear, please ask for clarification.
I understand votes are supposed to be anonymous and downvotes could also be used if the user thinks the OP didn't research "enough" or the code question is not useful...

How does the es-6 usage look? What would you change and/or code differently?

1. Arrow Functions
I would personally recommend using arrow functions where you have not to stay consistent with the ES6 usage throughout your code.
Example:
;(function() {

... can be changed to:
;(() => {

Additionally, parenthesis are optional when only one parameter is given.
Example:
document.addEventListener('click', (event) => timeout ...

... can be changed to:
document.addEventListener('click', event => timeout ...

2. Constant Arrow Function Expressions
Since it does not appear that you have any intention of overwriting your functions, I would recommend using function expressions defined as constants, rather than function declarations.
Example:
function updateTitle(showAsterisk = false) {

... can be changed to:
const updateTitle = (showAsterisk = false) => {

3. Don't Use var
If you can use let or const, use them instead.
Example:
var titleMatches = document.title.match(/^([\W]{1})Review/);

... can be changed to:
const titleMatches = document.title.match(/^([\W]{1})Review/);

4. The window. Prefix is Unnecessary
You do not need to use the window prefix on window.setTimeout, window.clearTimeout, or window.location.
Example:
window.clearTimeout(timeout);

... can be changed to:
clearTimeout(timeout);

Final Result
Ultimately, the final result of the changes I would make would look like the following:
// ==UserScript==
// @name         Highlight active review
// @namespace    http://tampermonkey.net/
// @version      0.1
// @description  Bring attention to the user when a post is up for reviewing
// @author       Sam Onela
// @match        https://codereview.stackexchange.com/review/*
// ==/UserScript==

;(() =>
{
    'use strict';

    const INITIAL_CHECK_TIMEOUT = 1500;
    const INTERVAL_DELAY        = 400;
    const RELOAD_DELAY          = 6500;
    const CLICK_CHECK_DELAY     = 6000;
    const ticks                 = [
        '⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏'
    ];

    let timeout = setTimeout(check, INITIAL_CHECK_TIMEOUT);
    let interval;

    const updateTitle = (showAsterisk = false) =>
    {
        let titleMatches = document.title.match(/^([\W]{1})Review/);

        if (titleMatches && titleMatches.length > 1)
        {
            const index    = ticks.indexOf(titleMatches[1]);
            document.title = document.title.replace(titleMatches[1], ticks[( index + 1) % ticks.length]);
        }

        else
        {
            document.title = (showAsterisk ? '*' : ticks[0]) + document.title;
        }
    };

    const check = (clickHandlerAdded = false) =>
    {
        if (timeout)
        {
            clearTimeout(timeout);
            timeout = 0;
        }

        if (window.location.pathname.match(/\d+/)) // drilled in to a review item
        {
            updateTitle(true);

            if (!clickHandlerAdded)
            {
                document.addEventListener('click', event => timeout = setTimeout(check, CLICK_CHECK_DELAY, true));
            }
        }

        else //add spinner animation and then reload
        {
            updateTitle();

            if (interval === undefined)
            {
                interval = setInterval(updateTitle, INTERVAL_DELAY);
            }

            setTimeout(_ => window.location = window.location, RELOAD_DELAY);
        }
    };

})();

Hope this helps!


Optimising Prefix Sum I have a question: Starting with a 1-indexed array of zeros and a list of operations, for each operation add a value to each the array element between two given indices, inclusive. Once all operations have been performed, return the maximum value in the array.
Given some queries in the form [a,b,k]:
[[1,5,3],[4,8,7],[6,9,1]]

Add the values of k between the indices a and b inclusive:
[0,0,0, 0, 0,0,0,0,0, 0]
[3,3,3, 3, 3,0,0,0,0, 0]
[3,3,3,10,10,7,7,7,0, 0]
[3,3,3,10,10,8,8,8,1, 0]

And then return the max value which is 10
My attempt is:
from itertools import accumulate

def Operations(size, Array):

    values = [0] * (size+2)
    for a,b,k in Array:
        values[a] += k
        values[b+1] -= k

    values = list(accumulate(values))

    Result = max(values)
    return Result

def main():
    nm = input().split()

    n = int(nm[0])

    m = int(nm[1])

    queries = []

    for _ in range(m):
        queries.append(list(map(int, input().rstrip().split())))

    result = Operations(n, queries)

if __name__ == "__main__":
    main()

This code works, however, hits runtime errors when the arrays get too large. I have no idea how to lower the runtime further.
Example input is as follows:
5 3
1 2 100
2 5 100
3 4 100

The solution looks good, my suggestions:

Generators: the function itertools.accumulate returns a generator which doesn't create the whole list in memory, but calling list on it does create the actual list in memory. This is probably why you get the runtime error. Changing Operations from:
def Operations(size, Array):

    values = [0] * (size+2)
    for a,b,k in Array:
        values[a] += k
        values[b+1] -= k

    values = list(accumulate(values))

    Result = max(values)
    return Result

To:
def Operations(size, Array):
    values = [0] * (size + 2)
    for a, b, k in Array:
        values[a] += k
        values[b + 1] -= k
    return max(accumulate(values))

solves the problem and all tests pass.
PEP 8: function names should be lowercase, with words separated by underscores as necessary to improve readability. Variable names follow the same convention as function names. More info. The function name is probably given by HackerRank, but the variable names Array and Result should be lowercase.

FYI, this is my old review on the same problem.


Simple Gradient Descent Algorithm In Haskell After becoming interested in various bits of functional programming provided by non-functional languages, I've recently decided to start learning Haskell.  As I'm fairly experienced with conventional imperative programming languages,  I decided to make my "hello world" something a little more complex - an implementation of gradient descent with 2 variables.

The idea with this code is that you fill in the training set for the algorithm in the code, and then run the code something similar to this:

descentFunc = singleDescend trainingSet 0.1
deltas = descend descentFunc ( 100, 1 ) 100000

Where 0.1 is the learning rate (referred to as lr in the code) 100000 is the number of iterations for the loop, and ( 100, 1 ) is an initial guess for the coefficients.

The actual code that's running is below.  As I'm entirely new to Haskell, I was wondering whether code such as this is acceptable?  And whether there's any obvious idioms I'm missing/misusing or any glaring style errors I've made.

Any comments on my implementation of the algorithm are welcome also.

import Data.List

trainingSet = [ (1.0,10.0),(2.0,20.0),(3.0,30.0),(4.0,40.0) ]

hypothesis (d1,d2) x = d1 + (d2 * x)

squareDiff d (x,y) = diff * diff
    where diff = ( hypothesis d x ) - y

costFunc ts d = scaleFactor * sum squares
    where scaleFactor = 1.0 / (2 * genericLength ts)
          squares = map (squareDiff d) ts

diff d (x,y) = (hypothesis d x) - y

descendTheta thetaFunc deltaFunc ts lr deltas =
    deltaFunc deltas - (scaleFactor * sum scaledDiffs)
    where scaleFactor = lr / genericLength ts
          diffs = map (diff deltas) ts
          scaledDiffs = map (\(x,y) -> x * thetaFunc y) $ zip diffs ts

descendThetaZero = descendTheta (\_ -> 1) fst
descendThetaOne = descendTheta (fst) snd

singleDescend ts lr deltas = (thetaZero,thetaOne)
    where thetaZero = descendThetaZero ts lr deltas
          thetaOne = descendThetaOne ts lr deltas

descend func deltas i
    | i == 0 = deltas
    | otherwise = descend func ( func deltas ) ( i - 1 )

one obvious change would be to add type signatures,
i additionally introduced some type synonyms to distinguish between all those Doubles. A bit explanation could be done towards the algorithm - I don't know what it actually want to achieve. I am no programmer so the non understanding comes quite often.

Next thing is it seems costFunc is never used - why is it there?

and I've replaced genericLength by length and converting the result - as the library says length is faster.

type TrainData = [(Double,Double)]
type LearnRate = Double
type DeltaPair = (Double,Double)

trainingSet ::  TrainData
trainingSet = [ (1.0,10.0),(2.0,20.0),(3.0,30.0),(4.0,40.0) ]

hypothesis :: DeltaPair -> Double -> Double
hypothesis (d1,d2) x = d1 + (d2 * x)

costFunc :: TrainData -> DeltaPair -> Double
costFunc ts d = scaleFactor * sum squares
    where scaleFactor = 1.0 / (2 * fromIntegral (length ts))
          squares = map (square . diff d) ts
          square x = x * x

diff :: DeltaPair -> (Double, Double) -> Double
diff d (x,y) = hypothesis d x - y

descendTheta :: ((Double, Double) -> Double) -> (DeltaPair -> Double) -> TrainData -> LearnRate -> DeltaPair -> Double
descendTheta thetaFunc deltaFunc ts lr deltas =
    deltaFunc deltas - (scaleFactor * sum scaledDiffs)
    where scaleFactor = lr / fromIntegral (length ts)
          diffs = map (diff deltas) ts
          scaledDiffs = map (\(x,y) -> x * thetaFunc y) $ zip diffs ts

descendThetaZero :: TrainData -> LearnRate -> (Double, Double) -> Double
descendThetaZero = descendTheta (const 1) fst

descendThetaOne :: TrainData -> LearnRate -> (Double, Double) -> Double
descendThetaOne = descendTheta fst snd

singleDescend :: TrainData -> LearnRate -> DeltaPair -> DeltaPair
singleDescend ts lr deltas = (thetaZero, thetaOne)
    where thetaZero = descendThetaZero ts lr deltas
          thetaOne = descendThetaOne ts lr deltas

descend :: (DeltaPair -> DeltaPair) -> DeltaPair -> Int -> DeltaPair
descend f deltas i
    | i < 0 = error "no negative numbers"
    | i == 0 = deltas
    | otherwise = descend f (f deltas) (i-1)

main :: IO ()
main = print (descend descendFunc (100, 1) 10000)
    where descendFunc = singleDescend trainingSet 0.1


Team git commit cleaner I cleaned a big repository for my team with this Python code. My goal was for every developer in my team to check if some bad email are in the commit, and replace the information by the good one. I use git filter-branch and a for loop in bash.

Because I can't make an array in an array in bash, I created a Python script to handle all the developers in my team.

Any idea on how I can optimize this code? git filter-branch take a long time.

# coding=utf-8
import subprocess
import os

def generate_command(dev):
    emails_string = ""
    for email in dev["emails"]:
        emails_string += '"%s" ' % email
    return """git filter-branch -f --env-filter 'OLD_EMAILS=(%s)
CORRECT_NAME="%s"
CORRECT_EMAIL="%s"
for email in ${OLD_EMAILS[@]};
do
        if [ "$GIT_COMMITTER_EMAIL" = "$email" ]
        then
                export GIT_COMMITTER_NAME="$CORRECT_NAME"
                export GIT_COMMITTER_EMAIL="$CORRECT_EMAIL"
        fi
        if [ "$GIT_AUTHOR_EMAIL" = "$email" ]
        then
                export GIT_AUTHOR_NAME="$CORRECT_NAME"
                export GIT_AUTHOR_EMAIL="$CORRECT_EMAIL"
        fi
done' --tag-name-filter cat -- --branches --tags""" % (emails_string.strip(),
                                                       dev["author_name"],
                                                       dev["author_email"])

developers = [
    {
        "emails": ["bad_email_author1@mycompany.com", "bad_email2_author1@mycompany.com"],
        "author_name": "first dev",
        "author_email": "good_email_author1@mycompany.com"
    },
    {
        "emails": ["bad_email_author2@mycompany.com", "bad_email2_author2@mycompany.com"],
        "author_name": "second dev",
        "author_email": "good_email_author2@mycompany.com"
    }
]

if __name__ == '__main__':
    for developer in developers:
        subprocess.call(generate_command(developer), shell=True)

First reaction: wow this is scary: Python script generating Bash which again calls some Bash in it. But I see the filter-env technique comes straight out from an example in the docs.

I would have written this in pure Bash, using a helper function that takes as parameters:

author name
author email
one or more bad email addresses

And then for each bad email address, call git filter-branch like you did,
but all in pure Bash.

As far as the Python part is concerned, this can be done better:

emails_string = ""
for email in dev["emails"]:
    emails_string += '"%s" ' % email

Using a list comprehension:

emails_string = " ".join(['"%s"' % email for email in dev["emails"]])

With this, you don't need to .strip() the emails_string when you generate the command string.


My Google Maps Script I've written a script with the Google Maps JavaScript API v3.

I've ran it through JSHint and everything seems to be valid.

I'm wondering if my script has room for improvement.

Any suggestions are very much welcome!

(function () {
    'use strict';

    var map,
        markers = [];

    function initMap() {
        var paris = new google.maps.LatLng(48.856614, 2.3522219000000177),
            options = {
                // required
                center: paris,
                zoom: 10,
                // disable all user interaction
                disableDefaultUI: true,
                navigationControl: false,
                mapTypeControl: false,
                scaleControl: false,
                scrollwheel: false,
                draggable: false,
                zoomControl: false,
                disableDoubleClickZoom: true
            };

        map = new google.maps.Map(document.getElementById('map'), options);

        addMarker(paris, false);

        // center map responsively
        google.maps.event.addDomListener(window, 'resize', function () {
            var center = map.getCenter();

            google.maps.event.trigger(map, 'resize');

            map.setCenter(center);
        });
    }

    function geocodeAddress() {
        var geocoder = new google.maps.Geocoder(),
            addressInp = document.getElementById('address-inp');

        geocoder.geocode({ 'address': addressInp.value }, function (results, status) {
            if (status !== google.maps.GeocoderStatus.OK) {
                alert('Geocode was not successful for the following reason: ' + status);

                return;
            }

            map.fitBounds(results[0].geometry.viewport);
            map.setCenter(results[0].geometry.location);

            deleteAllMarkers();
            addMarker(results[0].geometry.location, true);

            addressInp.value = results[0].formatted_address;
        });
    }

    function addMarker(location, animateDrop) {
        var marker = new google.maps.Marker({
            map: map,
            position: location
        });

        if (animateDrop) {
            marker.setAnimation(google.maps.Animation.DROP);
        }

        google.maps.event.addListener(marker, 'click', function () {
            if (marker.getAnimation() !== null) {
                marker.setAnimation(null);
            } else {
                marker.setAnimation(google.maps.Animation.BOUNCE);
            }
        });

        markers.push(marker);
    }

    function deleteAllMarkers() {
        for (var i = 0; i < markers.length; i++) {
            markers[i].setMap(null);
        }
    }

    google.maps.event.addDomListener(window, 'load', initMap);

    window.onload = function () {
        var addressInp = document.getElementById('address-inp'),
            searchBtn =  document.getElementById('search-btn');

        addressInp.onkeydown = function (e) {
            if (e.keyCode === 13) {
                searchBtn.click();
                addressInp.blur();
            }
        };

        searchBtn.onclick = function () {
            geocodeAddress();
        };
    };
})();

Your initMap function is called by the Google Maps' DOM listener.  I would actually move this to the anonymous window.onload function that you have.

The reason is is more philosophical than practical.  You're trying to use Google maps' api to initialize your maps objects.  It seems un-intuitive that Maps should initialize maps.

If Google ever changed their API for their DOM listener, it could mess up this functionality.

The only other issue I would consider is some protection against the initMap being called twice.  Just a flag at the top or something simple like that:

if (mapsInitialized)
    return;
mapsInitialized = true;


Counting all the vowels in a passage of text My son just started high school and in Math, he was asked to do some stats homework. The exercise was to count all the vowels in a passage of text, then answer some questions about frequency.

Out of curiosity, I wrote a quick Python script to count the vowels:

text = "Australian Rules Football is a ball game played by two teams of eighteen players with an ellipsoid ball on a large oval field with four upright posts at each end. Each team attempts to score points by kicking the ball through the appropriate posts (goals) and prevent their opponents from scoring. The team scoring the most points in a given time is the winner. Usually this period is divided into four quarters of play.Play begins at the beginning of a quarter or after a goal, with a tap contest between two opposing players (rucks) in the centre of the ground after the umpire either throws the ball up or bounces it down."

vowels = {'a':0, 'e':0, 'i':0, 'o':0, 'u':0}

for t in text:
    if t.lower() == 'a':
        vowels['a'] = 1 + vowels['a']
    elif t.lower() == 'e':
        vowels['e'] = 1 + vowels['e']
    elif t.lower() == 'i':
        vowels['i'] = 1 + vowels['i']
    elif t.lower() == 'o':
        vowels['o'] = 1 + vowels['o']
    elif t.lower() == 'u':
        vowels['u'] = 1 + vowels['u']

print vowels

I don't claim to be very good at Python. I enjoy mucking around with it, but I think I'm missing something here. The script works, but I'm clearly repeating myself in the for loop. What's the better "Pythonic" was for me to have done this?

Change to lower once: text.lower() instead of t.lower() inside loop.
Use t in vowels to check if the character is vowels.
vowels = {...} can be replaced with dict.fromkeys('aeiou', 0) (See dict.fromkeys)

Caution: Use this only if the value is immutable.

>>> dict.fromkeys('aeiou', 0)
{'a': 0, 'i': 0, 'e': 0, 'u': 0, 'o': 0}

vowels = dict.fromkeys('aeiou', 0)

for t in text.lower():  # Change to lower once.
    if t in vowels:
        vowels[t] += 1

print vowels

Alternatively, you can use try ... except KeyError ...:

for t in text.lower():  # Change to lower once.
    try:
        vowels[t] += 1
    except KeyError:  # Ignore consonants.
        pass

Or using battery included, collections.Counter:

from collections import Counter
vowels = Counter(c for c in text.lower() if c in 'aeiou')
# => Counter({'e': 54, 'a': 41, 'o': 40, 'i': 37, 'u': 14})


Creating comma-separated '?' placeholders for SQL query parameters I have come up with a code that takes the total number of an array and creates ? for each, then separates them by a comma; leaving me a list like the one below:

?, ?, ?, ?, ?...

I am using this for my sql queries with prepared statements, instead of having to create the prepared statement each time, I want to just have an easily accessible class that does the heavy work.

So far I have a for loop code which I came up with to get the comma-separated question marks. First I create an array of [0] => ? ... based on the total number of the original array and then implode the value and add the comma between each element.

<?php
$arr = array(
    'name' => 'James Bond',
    'age' => 60,
    'hobbies' => 'Saving England',
    'country' => 'England',
    'language' => 'English',
    'children' => 'too many to count'
);

for($i = 0; $i < count($arr); $i++)
{
    $question[$i] = '?';
}
print_r( implode(', ', $question) );
// outputs: ?, ?, ?, ?, ?, ?
?>

Now my question is, is there a more efficient way of achieving this, such as a one-liner that just takes the total array number and creates the number automatically?

The ? array can be filled using array_fill:

implode(', ', array_fill(0, sizeof($arr), '?'))


Console input prompt loop in F# I'm learning F#, starting with baby steps. Is there a way to write this code in a more succint way, while keeping the same semantics? Maybe by using a Seq or something like that?

let rec repeatingPrompt predicate (message:string) =
    Console.WriteLine(message)
    let value = Console.ReadLine()
    if predicate value then
        value
    else
        repeatingPrompt predicate message

You can use Seq.initInfinite and Seq.find to do this, though I'm not sure it's actually better:

let repeatingPrompt predicate (message:string) =
    let prompt _ =
        Console.WriteLine(message)
        Console.ReadLine()

    Seq.initInfinite prompt |> Seq.find predicate


The seasoned Zombie sniper: a query to find unanswered questions with specific tags The query (on SEDE)

This query builds upon the previous and includes the suggestions by rofl as well as the now mentioned activity index.

DECLARE @tag_name nvarchar(35) = '##TagName##';
DECLARE @min_score int = ##MinScore:int?0##;
DECLARE @max_score int = ##MaxScore:int?0##;

-- CTE to find the maximum answer score on a question. Originally suggested by
-- rofl on https://codereview.stackexchange.com/a/189966/21002
WITH BestAnswer AS (
  SELECT
    ParentId,
    Max(Score) as Score,
    MAX(LastActivityDate) as LastActivityDate
  FROM Posts
  WHERE PostTypeId = 2
  GROUP BY ParentId
)

SELECT
  q.Id AS [Post Link],
  q.Tags,
  q.LastActivityDate as [Last question activity],
  a.LastActivityDate as [Last answer   activity],
  CAST(a.LastActivityDate - q.LastActivityDate as int) as [Activity Index]
FROM
  Posts q
    INNER JOIN PostTags pt
    ON    q.Id = pt.PostId
    INNER JOIN Tags t
    ON    pt.TagId = t.id
      AND t.TagName = @tag_name
    INNER JOIN BestAnswer a
    ON    q.Id = a.ParentId
      AND a.Score >= @min_score
      AND a.Score <= @max_score
WHERE
      q.PostTypeId = 1
  AND q.ClosedDate IS NULL
  AND q.AnswerCount > 0
  AND q.AcceptedAnswerId IS NULL
ORDER BY
  [Activity Index] DESC,
  q.LastActivityDate ASC

-- TagName: Tag name as you would use in your post
-- MinScore: Highest answer has at least votes:
-- MaxScore: Highest answer has at most votes:

This will probably be the last one in the Zombie series.

Motivation

Zombies! The dread of any barbecue. The grilled brains are sticky, their odour is icky, and they never stop talking about crossdead. But that is not the kind of Zombie we suffer from.

We are looking for easy-to revive question Zombies. Questions, that have at least one 0-score answer but count as /unanswered. A revival just needs a single click in those cases: an upvote. We only need to find those Schroedinger questions and cure them.

The first cure accidentally included closed questions, as did the second. This query remedies this fault and only looks for the Zombies that haven't been thrown into jail. It also includes an activity index (see below).

The goal

The previous questions have shown that the requirements on the query were not really communicated well. This time I'll try to list all requirements for the query:

it must consist of a subset of /unanswered:

it must not contain closed questions
it must not contain questions that have an accepted answer
it must not contain questions that have an answer with a positive (>0) score, unless the MaxScore is greater than 0

it must not contain questions that have answers with a score greater than the MaxScore
it must not contain questions that have answers with a score lesser than the MinScore
it should only return questions that have been tagged with ##Tagname##
it should include the last activity date of the question
it should include the latest activity date the question's answers
it should include an activity index
it should order the results by the activity index descending first and then ascending by date.

The activity index is basically the difference between the last question activity and the latest question's answers' activity, e.g.

$$ \text{activity_index} = \max_{a \text{ answers } q} a_{\text{LastActivityDate}} - q_{\text{LastActivityDate}}. $$

Since any answer activity automatically updates the question's LastActivityDate it is always non-positive. The lower the index, the more likely that a new answer is necessary and the current 0-score answer is insufficient or misses the point.

You have made a nice looking query here. I just have to nit-pick a little bit.

You query the table Posts with a synonym q where I would expect p instead. I know you want to indicate that it is a question but nevertheless I would expect p instead of q.
Every sql-statement is written in capital-letters but the AS keyword is written in lower-case-letters.

What you didn't take into account is the entering of a tag-synonym which would be  nice to have as well.


Bash - Compile C++ File Function As I am starting to be tired of reverse-searching in history for the C++ compile command, which I'm using - details on my flags are written here, I defined the following .bash_aliases function:

function compile-cpp {

    if [ -z "$1" ]
    then
    {
        echo "Need a file to compile as argument."
    }
    else
    {
        # extract file name from path
        filename=$(basename "$1")

        # cut the extension
        filename="${filename%.*}"

        # compile
        g++ $1 -std=c++14 -Wall -Wextra -Werror -Wpedantic -pedantic-errors -o "$filename"
    }
    fi

}

The goal was to take one cpp file as argument and compile it under the same name as the source's.

So, e.g. when I call:

compile-cpp delete.cpp

I expect it to output a file named as:

delete

In the same directory.

No problems, so far, detected by me, but as I am a C++ beginner, I can't know for sure, if this command will more or less always work.

Note: I don't wish to use a Makefile yet. The projects and various examples I work with are so little and there's so many of them, I believe it would be counterproductive.

I found the following question, probably trying to achieve somewhat similar thing:

Compile and run C++ code

But since I have different demands, looking at his code, I believe it is not its duplicate, in spite of having close to the same titles.

You could just go:

export CXXFLAGS=-std=c++14 -Wall -Wextra -Werror -Wpedantic -pedantic-errors

Then at any point you want to compile:

make plop.cpp

This will build the executable plop. Use the CXXFLAGS and does NOT need a makefile.

If you want to keep your function, I would allow people to override the defaults using the environment:

function compile-cpp {

    if [ -z "$1" ]
    then
    {
        echo "Need a file to compile as argument."
    }
    else
    {
        # extract file name from path
        filename=$(basename "$1")

        # cut the extension
        filename="${filename%.*}"

        # compile
        if [ -z "${VERBOSE}" ]
        then
        {
            # Normally just want to see the file that is being compiled.
            echo ${CXX:-g++} $1 -o ${filename}
        }
        else
        {
            # But if things are going wrong then you want to see the full command LINE.
            echo ${CXX:-g++} $1 ${CXXFLAGS:-'-std=c++14 -Wall -Wextra -Werror -Wpedantic -pedantic-errors'} -o "$filename"
        }
        fi
        ${CXX:-g++} $1 ${CXXFLAGS:-'-std=c++14 -Wall -Wextra -Werror -Wpedantic -pedantic-errors'} -o "$filename"
    }
    fi

}

Here:

${CXX:-g++} means if the environment variable is empty or null use `g++`
            otherwise use the value of `${CXX}`.

Same for the `${CXXFLAGS}`

I would prefer using the make technique, as this provides you all the facilities of Make in addition to the compiler (so it checks file date stamps) and other stuff.

## Use the fortran compiler for shits and giggles
CXX=fortran compile-cpp plop.cpp
VERBOSE=1 compile-cpp plop.cpp


Class to share data between two threads implemented as FIFO with std::queue I am trying to improve my code that involves a thread (threadA) handling some UDP communication and another thread (threadB) that works with the output of that communication.
My basic idea is to .push() new data into a std::queue within threadA.
threadB is looking for new data periodically - if it gets new data it processes it. Then .pop is called and the data is deleted.
Implementation:

threadA will call addNewElement() any time new UDP data arrives.

threadB will call getNewestPtr() to access the data.

when done with the data, threadB will call doneWithNewest().

I am managing the access to the data (which comes in the form of std::array) through the use of std::shared_ptr. I know that the shared_ptr will manage the access to the pointer in a thread safe manner, but the underlying object is not going to be managed. Here lies my problem - in my implementation of the class, I am not sure where to put in memory locks, to ensure thread safety. My implementation already works, I have been testing it for a few days without problems, but I have the feeling I am doing something wrong, as I have not used any std::mutex. Anyway, here is my code:
commbufferfifo.h
#ifndef COMMBUFFERFIFO_H
#define COMMBUFFERFIFO_H

#include <queue>
#include <array>
#include <memory>

const size_t COMMLENGTH = 56;

/**
 * @brief The commbufferfifo class
 * @details used to safely share data between the comm thread and the visu thread
 */
class commbufferfifo
{
public:
    commbufferfifo();

    /**
     * @brief getNewestPtr  only accessed in visu thread
     * @return returns nullptr if fifo is empty!
     */
    std::shared_ptr<std::array<int16_t, COMMLENGTH>> getNewestPtr();///< @brief

    void doneWithNewest();///< @brief call when newest object can be deleted

    void addNewElement(const std::array<int16_t, COMMLENGTH> &commBuffer);  ///< @brief only accessed in comm thread

private:
    std::queue<std::shared_ptr<std::array<int16_t, COMMLENGTH>>> m_fifo;
};

#endif // COMMBUFFERFIFO_H

commbufferfifo.cpp
#include "commbufferfifo.h"

commbufferfifo::commbufferfifo()
{

}

std::shared_ptr<std::array<int16_t, COMMLENGTH> > commbufferfifo::getNewestPtr()
{
    if (m_fifo.empty())
        return nullptr;
    else
        return m_fifo.front();
}

void commbufferfifo::doneWithNewest()
{
    if (m_fifo.empty())
        return;
    else
        m_fifo.pop();
}

void commbufferfifo::addNewElement(const std::array<int16_t, COMMLENGTH> &commBuffer)
{
    m_fifo.push(std::make_shared<std::array<int16_t, COMMLENGTH>>(commBuffer));
}

some pseudo usage:
main.cpp
#include "includes/commbufferfifo.h"

#include <unistd.h>

void threadA(std::shared_ptr<commbufferfifo> myPointer){
    std::array<int16_t, COMMLENGTH> arr;
    for (int iteration = 0; iteration < 1000; iteration++){
        arr.at(0) = iteration;
        myPointer->addNewElement(arr);
        usleep(100);
    }
}

int main()
{
    auto sharedFifo = std::make_shared<commbufferfifo>();
    std::thread tA(&threadA, sharedFifo);

    usleep(10000);
    for(;;){
        if (sharedFifo->getNewestPtr() == nullptr)
            break;
        std::cout << "sharedFifo->getNewestPtr()->at(0) = " << sharedFifo->getNewestPtr()->at(0) << "\n";   //use data
        sharedFifo->doneWithNewest();                                                                       //data will be deleted
        usleep(1000);
    }

    tA.join();

    std::cout << "------------------------------------\n";
    std::cout << "done" << std::endl;
    return 0;
}
```

This is more of a stackoverflow question but whatever.

There is no guarantee that std::queue is thread safe and methods empty()/front()/push() will properly interact in a multi-threaded environment. They most certainly cause data races. For this reason you need to use std::mutex to ensure that std::queue isn't accessed simultaneously from multiple threads ensuring that the data is read/written correctly.

What are you going to do when the queue is empty? What if you want to wait for next element? Will you put sleep(10ms) periodically? What if it isn't responsive enough and has too big of a lag? The proper method is to utilize std::condition_variable which has proper wait/notify methods.

It isn't worth to create a shared pointer to view some 100 bytes. You'd better transfer 100 bytes of data by copying it.

This isn't a C++ approach to transfer data by simply copying chunks of bytes. What you should do instead is to template your thread-safe queue over some class T so it moves T's instances from sender to receiver in a safe way.


How to avoid carrying around a lock with my variable when multiple threads instantiation in Python? I created an application using multiple threads defined in multiple class files which read and write into a "shared variable" (a dictionary).

To ensure thread-safety, I am using a Lock, and I pass it as argument to my Thread class next to my variable.

What bothers me is that two objects which are intrinsically linked have to be passed to my class, while ideally only the shared variable should be. This would reduce the code and bring greater certainty while avoiding confusion in the parameters.

Here is how my program looks:

inc_thread.py

from threading import Thread

class IncThread(Thread):

    def __init__(self, shared_dict, lock):
        Thread.__init__(self, daemon=True)
        self._shared_dict = shared_dict
        self._lock = lock

    def run(self):
        while 1:
            with self._lock:
                self._shared_dict['some_value'] += 1

dec_thread.py

from threading import Thread

class DecThread(Thread):

    def __init__(self, shared_dict, lock):
        Thread.__init__(self, daemon=True)
        self._shared_dict = shared_dict
        self._lock = lock

    def run(self):
        while 1:
            with self._lock:
                self._shared_dict['some_value'] -= 1

main.py

import time

from threading import RLock
from dec_thread import DecThread
from inc_thread import IncThread

if __name__ == '__main__':
    lock = RLock()
    shared_dict = {'some_value': 0}

    t1 = DecThread(shared_dict, lock)
    t2 = IncThread(shared_dict, lock)

    t1.start()
    t2.start()

    # Ensure equity regarding wich thread started first
    with lock:
        shared_dict['some_value'] = 0

    while 1:
        print(shared_dict['some_value'])
        time.sleep(1)

Is it the correct way to do it?

What I also thought:

Subclass dict and override get / set methods with an internal lock, but it becomes complicated when I want to set other datastructures (nested dict, list, set, etc.) as values.
Using Queue but it does not seem to fit my needs, as it is by default blocking for put / get while my design is more of "fire and forget".

Your concern seems legit. Fusing data that needs thread-safe operations and means of those operations would be nice.

However, this:

  Subclass dict and override get / set methods with an internal lock

will not work, because += - which you use - will not be atomic.

Things get worse with nested data structures: we actually have several objects and some of them carry references to others. If there are also some more references to those objects - how do we check that? Example:

a = [1,2,3]
b = {'a' : a} # or some crafty wrapper on top of that

If we make sure that every b['a'] is atomic, a can still be changed directly.

Making a wrapper that accepts mutation function and executes it in a locked context would not really work if you need in-place operations, not replacements of the whole object.

What you can do is take a closer look at context managers and try to create an object that is both dictionary and context manager.

Or simply do something like:

class FlexibleDict(dict):
    pass

shared_dict = FlexibleDict(shared_dict)
shared_dict.lock = lock

and pass that around.


Finding the tallest and find the index number of the next taller guy in a circular queue Input given is:

  Each test case contains two lines. The first line contains a single integer N (1 ≤ N ≤ 105), the number of persons playing the game. The second line contains N integers i(height) (1 ≤ hi ≤ 109) the height of the i-th person. They are numbered 1 to N starting from Druid.

Output given is:

  For each test case print N lines, in the i-th line print 2 numbers, the index of the first person taller than the i-th person on his left, and the index of the first person taller than the i-th person on his right. If no one is taller than the i-th person print -1 -1.

Input is:

3
5
172 170 168 171 169
3
172 169 172
1
172

The output obtained is:

-1 -1
 1 4
 2 4
 1 1
 4 1
-1 -1
 1 3
-1 -1
-1 -1

The code is:

t = input()

num =  []

def get_druid_index(arr,n):
     druid_max_index = arr.index(max(arr))

     if n == 1:
         num.append(-1)
         num.append(-1)

     for j in range(n):
          if j == druid_max_index:
              num.append(-1)
              num.append(-1)
          else:
              cur = arr[j]
              l = j
              r = j
              for k in range(n):

                   if arr[l] > cur:
                       num.append(l + 1)
                       break

                   if l == 0:
                       l = n

                   l = l - 1

               for k in range(n):
                   if r == n:
                       r = 0

                   if arr[r] > cur:
                       num.append(r + 1)
                       break

                   r = r + 1

for i in range(t):

   arr = []

   n = input()
   arr = raw_input()
   arr = arr.split()

   get_druid_index(arr,n)
count = 0

for i in range(len(num)/2):
    print num[count],num[count + 1]

    count = count + 2

I optimized the code as much as possible but I get TLE from the online judge. Please offer suggestions for improvements.

I think you can write a linear time solution borrowing ideas from this. The idea is to keep a stack of decreasing maxima (SDM). Let's consider first the problem without taking into account the circularity...

Say your input was [7, 4, 5, 3]. When you are processing the third item (5), you want your SDM to be [(7, 1), (4, 2)], where he second item in the tuple is the 1-based index of that item in the original array, as per your problem's description. You search the SDM for the last entry larger than the item being processed, which in this case is (7, 1), so the index of the first item larger than 5 to its left is 1. You then discard all items in the SDM to the right of this one, and insert the current value in it's position, so the SDM now becomes [(7, 1), (5, 2)]. Rinse, repeat, and you have all your first items larger than to the left. For the symmetric item to the right, do the same scanning from right to left.

At first this may seem as an \$O(n^2)\$ algorithm, since you have to search a list that could potentially be as large as the array (e.g. if the input is monotonically decreasing) for every item. Because the SDM is sorted, you could try to use binary search to make that bound \$O(n\log n)\$, but that would actually slow down the process!

You have to keep in mind that, as the stack is being searched sequentially, entries that aren't larger than the one being processed are being removed from the stack! So the more work an item requires to find it's answer, the easier it makes it for the ones remaining. A careful analysis of this shows that, on average, each item takes constant time to process, so the resulting time complexity is \$O(n)\$, which is basically as good as it gets.

You can factor in the circularity of the problem into the above approach, by first finding the maximum, and always starting your circular iteration, whether to the left or right, from it, so that you will always have the maximum in your SDM.

But enough talking, here's an implementation:

def taller_to_the_sides(list_):
    max_index = list_.index(max(list_))
    sdm = []
    left = []
    for index in range(len(list_)):
        index += max_index
        index %= len(list_)
        item = list_[index]
        while sdm and sdm[-1][0] <= item:
            sdm.pop()
        if sdm:
            left.append(sdm[-1][1] + 1)
        else:
            left.append(-1)
        sdm.append((item, index))
    if max_index > 0:
        left = left[-max_index:]+left[:-max_index]
    sdm = []
    right = []
    for index in range(len(list_)):
        index = max_index - index
        index %= len(list_)
        item = list_[index]
        while sdm and sdm[-1][0] <= item:
            sdm.pop()
        if sdm:
            right.append(sdm[-1][1] + 1)
        else:
            right.append(-1)
        sdm.append((item, index))
    right = right[max_index::-1] + right[:max_index:-1]
    return zip(left, right)

There are two almost identical blocks in that function that should probably be refactored into a common external function, but I felt it would better show what was going on to keep them separated.

Anyway, when you run this you get the expected output:

>>> list(taller_to_the_sides([172, 170, 168, 171, 169]))
[(-1, -1), (1, 4), (2, 4), (1, 1), (4, 1)]
>>> list(taller_to_the_sides([169, 172, 170, 168, 171]))
[(5, 2), (-1, -1), (2, 5), (3, 5), (2, 2)]

The second example is the first one shifted one item to the right, to show that maxima not in the first position also work. And since the algorithm is linear, it handles large inputs reasonably fast:

a = [random.random() for _ in range(100)]
>>> %timeit taller_to_the_sides(a)
10000 loops, best of 3: 172 µs per loop
a = [random.random() for _ in range(1000)]
>>> %timeit taller_to_the_sides(a)
1000 loops, best of 3: 1.78 ms per loop
a = [random.random() for _ in range(10000)]
>>> %timeit taller_to_the_sides(a)
100 loops, best of 3: 18.3 ms per loop

To be honest, I was expecting it to be faster, but do notice also how the time scaling is roughly linear, as expected.


Type-safe Dictionary for various types Assume the following situation: you have an object that can store any object based on a key (basically, IDictionary<string, object>). You want to store objects of various types into it that are not directly related. (For example, the dictionary can be an ASP.NET Session, or it can represent a dictionary that will be serialized to disk for persistent storage.)

I don't want to create a single class that will contain all those objects, because they are not directly related, they come from different places. But if you store each object separately, it means you have to use casts when getting some value and there is no type-check when you're setting it.

To solve this, I created a generic type that encapsulates the key along with the associated type and a couple of extension methods that use it:

class TypedKey<T>
{
    public string Name { get; private set; }

    public TypedKey(string name)
    {
        Name = name;
    }
}

static class DictionaryExtensions
{
    public static T Get<T>(this IDictionary<string, object> dictionary, TypedKey<T> key)
    {
        return (T)dictionary[key.Name];
    }

    public static void Set<T>(this IDictionary<string, object> dictionary, TypedKey<T> key, T value)
    {
        dictionary[key.Name] = value;
    }
}

Usage:

private static readonly TypedKey<int> AgeKey = new TypedKey<int>("age");

…

dictionary.Get(AgeKey) > 18
dictionary.Set(AgeKey, age)

This has the type-safety (both on get and set) of using a property, while being backed by a dictionary that can store anything.

What do you think about this pattern?

Your suggestion is not really type-safe as you can still pass a key of the wrong type. Therefore I would just use a normal (string) key. But I would add a generic TryGet method which takes account of the type. The setter needs not to be generic.

static class DictionaryExtensions
{
    public static T Get<T>(this IDictionary<string, object> dictionary, string key)
    {
        return (T)dictionary[key];
    }

    public static bool TryGet<T>(this IDictionary<string, object> dictionary,
                                 string key, out T value)
    {
        object result;
        if (dictionary.TryGetValue(key, out result) && result is T) {
            value = (T)result;
            return true;
        }
        value = default(T);
        return false;
    }

    public static void Set(this IDictionary<string, object> dictionary,
                           string key, object value)
    {
        dictionary[key] = value;
    }
}

You can then use the dictionary like this.

int age = 20;
dictionary.Set("age", age);

// ...

age = dictionary.Get<int>("age");

// or the safe way
if (dictionary.TryGet("age", out age)) {
    Console.WriteLine("The age is {0}", age);
} else {
    Console.WriteLine("Age not found or of wrong type");
}

Note that the compiler can infer the generic type when using TryGet.

UPDATE

In despite of my suggestion above, I must agree that your solution is elegant. Here is another suggestion which is based on your solution but which encapsulates the dictionary instead of providing a key. Well, it acts as wrapper and as key at the same time

public class Property<T>
{
    Dictionary<object, object> _dict;

    public Property (Dictionary<object, object> dict)
    {
        _dict = dict;
    }

    public T Value {
        get { return (T)_dict[this]; }
        set { _dict[this] = value; }
    }
}

Alternatively, a string key could be provided in the Property's constructor.

You can use it like this

private static readonly Dictionary<object, object> _properties =
    new Dictionary<object, object>();
private static readonly Property<int> _age = new Property<int>(_properties);

...

_age.Value > 18
_age.Value = age


Evaluating a polynomial at a given value of x and n To solve a polynomial equation I have written following program, is there any way to optimize this program.

Following code is working code:

Program takes input n,x where n is the deg of polynomial and x is the point at which the polynomial is to be evaluated.

   #include<stdio.h>

   int power(int x, int y);

   int main()
  {
  // n is the deg of polynomial
  // x is the value of polynomial for which it is to be evaluated
  int x=0,n=0,A[10],sum=0,i=0,M=0,y=0;

  printf("Enter value of n,x \n");
  scanf("%d %d",&n,&x);

  printf("Enter the coeficients\n");

  for(i=0;i<=n;i++)          //Taking coefiecent values
  {
     scanf("%d",&A[i]);
  }

  y=n;
  for(i=0;i<=n;i++)
  {
    M=power(x,y);
    sum=sum+(A[i]*M);
    y--;
   }

  printf("\nSum of polynomial is %d",sum);

  return 0;
 }

 int power(int x, int y)
 {
  int result = x;
  int i;
  if(y == 0) return 1;
  if(x < 0 || y < 0) return 0;

  for(i = 1; i < y; ++i)
  result *= x;

  return result;
}

By following Horner's Method only n multiplications are needed.

#include <stdio.h>

int main()
{
   // n is the deg of polynomial
   // x is the value of polynomial for which it is to be evaluated
   int x=0,n=0,A[10],b,i;

   printf("Enter value of n,x \n");
   scanf("%d %d",&n,&x);

   //Taking coefficient values
   printf("Enter the coeficients\n");

   for(i=0;i<=n;i++) scanf("%d",&A[i]);

   for(i = n - 1,b = A[n];i >= 0;i--) b = b*x + A[i];

   printf("\nSum of polynomial is %d\n",b);

   return 0;
}


Lock free MPMC Ring buffer implementation in C I have written a lock free MPMC FIFO in C based on a ring buffer. It uses gcc's atomic built-ins to achieve thread safety. The queue is designed to return -1 if it's full on enqueue or empty on dequeue.

After some feedback, I've changed by design. I have tested this code to work, but testing multithreaded code is hardly enough to prove it's correct.

struct queue
{
    void** buf;
    size_t num;
    uint64_t writePos;
    uint64_t readPos;
};

queue_t* createQueue(size_t num)
{
    struct queue* new = xmalloc(sizeof(*new));
    new->buf = xmalloc(sizeof(void*) * num);
    memset(new->buf, 0, sizeof(void*)*num);
    new->readPos = 0;
    new->writePos = 0;
    new->num = num;
    return new;
}

void destroyQueue(queue_t* queue)
{
    if(queue)
    {
        xfree(queue->buf);
        xfree(queue);
    }
}

int enqueue(queue_t* queue, void* item)
{
    for(int i = 0; i < kNumTries; i++)
    {
        if(__sync_bool_compare_and_swap(&queue->buf[queue->writePos % queue->num], NULL, item))
        {
            __sync_fetch_and_add(&queue->writePos, 1);
            return 0;
        }
    }
    return -1;
}

void* dequeue(queue_t* queue)
{
    for(int i = 0; i < kNumTries; i++)
    {
        void* value = queue->buf[queue->readPos % queue->num];
        if(value && __sync_bool_compare_and_swap(&queue->buf[queue->readPos % queue->num], value, NULL))
        {
            __sync_fetch_and_add(&queue->readPos, 1);
            return value;
        }
    }
    return NULL;
}

(link to the previous iteration)

Did you test it?

I think you have a problem if a thread calls enqueue and is interrupted
just before the memcpy, having calculated pos to be slot-1.  If another
thread now calls enqueue and completes it will write to slot-2 and increment the pendingRead counter, leaving
slot-1 still unwritten.  If a third thread now reads before the first thread
can complete its write, the reader will read from the unwritten slot-1.  This
is becase the second thread correctly set the queue to say there is a pending
read (slot-2) but dequeue assumes the data must be in the next available
slot according to readPos.

A few other comments:

queue_t is defined (elsewhere) as a pointer type. I prefer pointers to be
explicit.
You have mixed use of struct queue * and queue_t
Return from malloc is not checked.
buf in struct queue should be void* not char **
Why are writePos and readPos of type uint64_t and not size_t?
After all, num is size_t.  Why the difference?
Loss of precision assigning num in createQueue


Implement function in doubly linked list The following is the entire function for implementing a doubly linked list. Can anyone show me if there is a better way to write these functions, especially the delete_at function please?

#include<stdio.h>
#include<stdlib.h>
typedef struct linked_list
{
    int point;
    struct linked_list *p_node;
    struct linked_list *n_node;
}node;

node *create_head_node(node *, int);
node *insert_head(node *, int);
void insert_tail(node *, int);
node *insert_at(node *, int, int);
int count_node(node *);
void print_list(node *);
node *delete_tail(node *);
node *delete_head(node *);
node *delete_at(node *, int);

node *create_head_node(node *head, int point){
    head->point=point;
    head->p_node=NULL;
    head->n_node=NULL;
    return head;
}
node *insert_head(node *head, int point){
    if (head==NULL){
        printf("no head exist");
        return 0;
    }
    node *temp=(node*)malloc(sizeof(node));
    temp->point=point;
    temp->p_node=NULL;
    temp->n_node=head;
    head->p_node=temp;
    head=temp;
    return head;
}
void insert_tail(node *head, int point){
    if (head==NULL){
        printf("no head exist");
        return;
    }
    node *p=head;
    node *temp=(node*)malloc(sizeof(node));
    temp->point=point;
    temp->n_node=NULL;
    while (p->n_node!=NULL)
    {
        p=p->n_node;
    }
    p->n_node=temp;
    temp->p_node=p;
}
node *insert_at(node *head, int point, int pos){
    if (head==NULL){
        printf("no head exist");
        return 0;
    }
    int count=count_node(head);
    while (pos>count)
    {
        printf("choose %d positions to add. choose again: ", count); scanf("%d", &pos);
    }
        node *p=head;
        for (int i = 0; i < pos; i++)
        {
            p=p->n_node;
        }
        node *temp=(node*)malloc(sizeof(node));
    temp->point=point;
    temp->n_node=p;
    temp->p_node=p->p_node;
    if(p->p_node!=NULL) p->p_node->n_node=temp;
    p->p_node=temp;
    return head;
}
int count_node(node *head){
    node *p=head;
    int count=0;
    while (p!=NULL)
    {
        count++;
        p=p->n_node;
    }
    free(p);
    return count;
}
void print_list(node *head){
    if (head==NULL){ printf("nothing to print"); return;}
    node *p=head;
    while (p!=NULL)
    {
        printf("%d ", p->point);
        p=p->n_node;
    }
}
node *delete_head(node *head){
    if (head==NULL){
        printf("no head exist\n");
        return 0;
    }
    node *p=head;
    // head->p_node=NULL;
    if(p->n_node!=NULL){
        head=p->n_node;
        free(p);
    }
    else head=NULL;
    return head;
}
node *delete_tail(node *head){
    if (head==NULL){
        printf("no head exist\n");
        return 0;
    }
    node *p=head;
    while (p->n_node!=NULL)
    {
        p=p->n_node;
    }
    if (p->p_node!=NULL){
        p->p_node->n_node=NULL;
        free(p);
    }
    else head=NULL;
    return head;
}
node *delete_at(node *head, int pos){
    int count=count_node(head);
    if(head==NULL){
        printf("no head exist");
        return 0;
    }
    while (count<pos)
    {
        printf("choose %d positions to delete. choose again", count); scanf("%d", &pos);
    }

    node *p=head;
    if(pos==0) head=delete_head(head);
    else if(pos==count-1) head=delete_tail(head);
    else{
        for (int i = 0; i < pos; i++)
        {
            p=p->n_node;
        }
        p->p_node->n_node=p->n_node;
        p->n_node->p_node=p->p_node;
        free(p);
    }
    return head;
}

Design

Why do you need a create_head_node() that takes a node as the first parameter? Seems counter intuitive. Personally I would just remove that function does not seem needed to me.

The other thing is that you don't seem to have a way to initialize an empty list (yes this can simply be the NULL pointer but it does leave it easy to accidentally have an uninitialized list).

You can make the design better by separating Node and the list into different types. That way a lot of the work for tail is removed.

i.e. It is easy to have A List and a Node.

struct Node;
struct List;
typedef struct Node Node;
typedef struct List List;

struct List
{
    Node*   head;
    Node*   tail;
    int     count;
};

struct Node
{
    int     value;
    Node*   next;
    Node*   prev;
};

Code Review

The create_head_node() seem to be more like reset the head node to this value and leak the rest of the list.

node *create_head_node(node *head, int point){
    head->point=point;
    head->p_node=NULL;
    head->n_node=NULL;
    return head;
}
----
node*  list = NULL;
list = insert_head(list, 10);
list = insert_head(list, 20);

/*
 * now lets reset the head to 30 and leak the tail (20)
 */
list = create_head_node(list, 30);

Why does it matter if there is currently no head node?

node *insert_head(node *head, int point){
    if (head==NULL){
        printf("no head exist");
        return 0;
    }

Can't I add to the head of an empty list?
Also why are you returning 0. That is an integer. An empty pointer is represented by NULL.

Again I am not sure we I can add to the tail of an empty list.

void insert_tail(node *head, int point){
    if (head==NULL){
        printf("no head exist");
        return;
    }

node *insert_at(node *head, int point, int pos){

Yes sure it is an error.

    while (pos>count)
    {
        printf("choose %d positions to add. choose again: ", count); scanf("%d", &pos);
    }

But this code can not determine the context that it is being used within. So it should not be generating an error message. You should be returning some form of status for the calling code to check. If the calling code decides an error message is appropriate then it can print the error message.

This is called the separation of concerns. Code either handles resource management (creating an maintaining a linked list) or it handles business logic (deciding if an error is critical or how to inform the user about errors).

Does this section not look exectly like insert_head()?

        node *temp=(node*)malloc(sizeof(node));
    temp->point=point;
    temp->n_node=p;
    temp->p_node=p->p_node;
    if(p->p_node!=NULL) p->p_node->n_node=temp;
    p->p_node=temp;
    return head;

int count_node(node *head){
    ....
    free(p);

You are freeing nodes that are still in the list?

A Simpler version;

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

struct Node;
struct List;
typedef struct Node Node;
typedef struct List List;

struct List
{
    Node*   head;
    Node*   tail;
    int     count;
};

struct Node
{
    int     value;
    Node*   next;
    Node*   prev;
};

List* createList();
void  destroyList(List* list);

int   countList(List* list);

bool insertHead(List* list, int value);
bool insertTail(List* list, int value);
bool insertAt(List* list, int value, int pos);

bool deleteHead(List* list);
bool deleteTail(List* list);
bool deleteAt(List* list, int pos);

typedef void (*Action)(int);
void visitNode(List* list, Action action);

/* Don't define a print function.
 * Use the visitor pattern to visit each node.
 * Then printing the list can simply be a function that prints each node as it is visited.
 */

/* Private Function */
Node* allocateNewNode(int value, Node* next, Node* prev)
{
    Node* newNode = (Node*)malloc(sizeof(Node));
    newNode->value = value;
    newNode->next  = next;
    newNode->prev  = NULL;
    return newNode;
}

List* createList()
{
    List* newList = (List*)malloc(sizeof(List));
    newList->head = NULL;
    newList->tail = NULL;
    newList->count = 0;
    return newList;
}
void  destroyList(List* list)
{
    /*
     * remove all elements
     */
    Node* head;
    Node* next;
    for(head = list->head;head;head = next) {
        next = head->next;
        free(head);
    }
    /*
     * remove the list
     */
    free(list);
}
int countList(List* list)
{
    return list->count;
}

bool insertHead(List* list, int value)
{
    Node* newNode = allocateNewNode(value, list->head, NULL);
    if (list->head) {
        list->head->prev = newNode;
    }
    list->head = newNode;
    if (list->tail == NULL) {
        list->tail = newNode;
    }
    ++newList->count;
    return true;
}
bool insertTail(List* list, int value)
{
    Node* newNode = allocateNewNode(value, NULL, list->tail);
    if (list->tail) {
        list->tail->next = newNode;
    }
    list->tail = newNode;
    if (list->head == NULL) {
        list->head = newNode;
    }
    ++newList->count;
    return true;
}
bool insertAt(List* list, int value, int pos)
{
    if (pos == 0) {
        return insertHead(list, value);
    }
    if (pos >= list->count) {
        return insertTail(list, value);
    }

    Node* loop = list->head;
    while(--pos) {
        loop = loop->next;
    }
    Node* newNode = allocateNewNode(value, loop->next, loop);
    if (newNode->next) {
        newNode->next->prev = newNode;
    }
    if (newNode->prev) {
        newNode->prev->next = newNode;
    }
    ++newList->count;
    return true;
}

bool deleteHead(List* list)
{
    Node* old = list->head;
    if (list->head) {
        list->head = list->head->next;
        if (list->head == NULL) {
            list->tail = NULL;
        }
        --newList->count;
    }
    free(old);
    return true;
}
bool deleteTail(List* list)
{
    Node* old = list->tail;
    if (list->tail) {
        list->tail = list->tail->prev;
        if (list->tail == NULL) {
            list->head = NULL;
        }
        --newList->count;
    }
    free(old);
    return true;
}
void visitNode(List* list, Action action)
{
    Node* loop = list->head;
    for(;loop;loop = loop->next) {
        action(loop->value);
    }
}

void printInt(int value)    {fprintf(stdout, "%d ", value);}
void printList(List* list)  {visitNode(list, printInt);}


Count the status messages in a column VBA Can someone review my code and provide feedback. For context in column A will be a status message of "OK" or "NOK" and this little function just counts the number of times this appear so I can update a label in another procedure. Not too sure if this is the most efficient way of doing this because looping will create delay going row by row if the data set is very large as in the main procedure this function will be called to update the label is in a loop so depending on the size of the data it can be small or very large and will trigger this function for each row.
Function UploadStatus(ByRef WS As Worksheet, ByVal StartRow As Long, ByVal EndRow As Long, Optional ByVal strMsg As String) As String
Dim OK As Long
Dim NOK As Long
Dim i As Long
Dim uploadMsg As String

If StartRow = 0 And EndRow = 0 Then Exit Function
With WS
    For i = StartRow To EndRow
        If .Range("A" & i).value = "OK" Then
            OK = OK + 1
        Else
            If .Range("A" & i).value <> vbNullString Then
                NOK = NOK + 1
            End If
        End If
    Next i
End With

If OK < 2 Then
    uploadMsg = OK & " OK row, "
Else
    uploadMsg = OK & " OK rows, "
End If

If NOK < 2 Then
    uploadMsg = uploadMsg & NOK & " NOK row"
Else
    uploadMsg = uploadMsg & NOK & " NOK rows"
End If
If strMsg <> vbNullString Then
    UploadStatus = strMsg & " " & uploadMsg
Else
    UploadStatus = uploadMsg
End If
End Function

Don't reinvent the wheel. Your homemade function will never be as fast as the built in functions.
Function UploadStatus(ByRef WS As Worksheet, ByVal StartRow As Long, ByVal EndRow As Long, Optional ByVal strMsg As String) As String
  Dim OK As Long
  Dim NOK As Long
  Dim uploadMsg As String

  If StartRow = 0 And EndRow = 0 Then
    Exit Function
  End If

  OK = WorksheetFunction.CountIf("A" & StartRow & ":A" & EndRow, "OK")
  NOK = WorksheetFunction.CountIf("A" & StartRow & ":A" & EndRow, "NOK")

  uploadMsg = ResultStringer(OK & "OK")
  uploadMsg = uploadMsg & ResultStringer(NOK & "NOK")

  If strMsg <> vbNullString Then
    UploadStatus = strMsg & " " & uploadMsg
  Else
    UploadStatus = uploadMsg
  End If

End Function

Private Function ResultStringer(ByVal Count As Long, ByVal ID as String) as String

  If Count > 1 Then
    ResultStringer = Count & ID & " rows, "
  Else
    ResultStringer = Count & ID & " row, "
  End If

End Function


Hash password and verify password with Node's Crypto I would like to make a safe way to store hashes of user passwords and verify them later. Here is crypt.js:

'use strict'

const crypto = require('crypto')

module.exports = {

    keyLength: 512,
    hashAlg: 'sha512',
    rounds: 10000,

    randHex(length) {
        return crypto.randomBytes(length / 2).toString('hex')
    },

    hashPassword(plainText) {
        let salt = this.randHex(64)
        let pbkdf2 = crypto.pbkdf2Sync(
            plainText,
            salt,
            this.rounds,
            this.keyLength,
            this.hashAlg
        ).toString('hex')

        return `${this.hashAlg}:${this.keyLength}:${this.rounds}:${salt}:${pbkdf2}`
    },

    verifyPassword(givenPassword, hashedPassword) {
        let splits = hashedPassword.split(':')
        let hashAlg = splits[0]
        let keyLength = parseInt(splits[1])
        let rounds = parseInt(splits[2])
        let salt = splits[3]
        let pbkdf2 = splits[4]

        let testPbkdf2 = crypto.pbkdf2Sync(
            givenPassword,
            salt,
            rounds,
            keyLength,
            hashAlg
        ).toString('hex')

        return testPbkdf2 === pbkdf2
    }

}

I have some questions.

It seems that pbkdf2 might be the best that Node's Crypt offers. Is this true? If so, is there a good reason to avoid pbkdf2Sync and use pbkdf2 instead?
I am adding in a lot of info such as the salt, rounds, hashing algorithm, etc. so that I can change this stuff and still be able to verify passwords hashed the old way. The database will have someone's password as sha512:512:10000:d15...d5k:a6a...1b9e Is it okay to do this?
What other dangers are there with crypt.js?

Glad to see you are using an adaptive encryption approach in PBKDF2.

You might consider Argon2 library as a more specific use password encryption tool as Argon2 is now the OWASP-recommended password encryption algorithm for new implementations.

Even though you are using an adaptive encryption approach, you are not really doing anything to facilitate the "adaptive" part.  Let's say this code has been working in an application for a year or two and you decide at some point that you want to increase your number of iterations from 10K to 20K, or change the underlying encryption algorithm, or any of the other settings. What happens when you verify a password against a hash made using old settings?

Your code, as shown, could still handle the verification part in that the hash contains all the needed information for re-hashing the password being verified. This is a very common and reasonable way to be able to perform password verification.  However, a password verification event is the only opportunity you have to update the stored hash to a new hash based on new default settings.  As such, you may consider being able to trigger an update of the password hash to a new hash if the current hash settings are different than the ones for the stored hash that is being used for verification.

Of course this logic doesn't need to exist in this specific module (it could exist outside this module higher up in application call stack), but it might mean that the verifyPassword() function might need to be aware of current settings and trigger a callback to update password hash, or change the return such that caller gets indication that there is need to update hash upon successful verification of password in old hash format.

Based on above comments, you might consider deriving your default password encryption settings from configuration rather than having them hard-coded in your module.

You may consider adding the ability to log out hash creation timings so as to allow you to more easily tune the work factor value to meet appropriate use case for your application (i.e. say you wanted the hashing process to take 1 second, you could tune the rounds value to achieve that approximate completion time for you application and environment).  This is another reason why you should probably store hash settings in config, to make it easier to tune the encryption for your application without having to actually change your code.


Use of nested namespaces in 3D graphics engine library I know that nested namespaces are used in C++ very rarely. But I think it's a nice solution to exclude types from global scope and sometimes it helps to write programs faster when we use things like "IntelliSense" or search for documentation (maybe it's similar for C# developers).

I've tried to organize my simple 3D graphics engine library in this way. Here is its main header file:

#ifndef RAZOR_H
#define RAZOR_H

#pragma warning (disable: 4482)

#include <Windows.h>
#include <fstream>
#include <iostream>
#include <string>
#include <fstream>
#include <vector>
#include <map>

#include <gl\GL.h>
#include "Externals\wglext.h"
#include "Externals\gl3.h"

namespace Razor
{
    namespace Framework
    {
        #include "DisplayOrientation.h"
        #include "ButtonState.h"
        #include "GameTime.h"
        #include "Rect.h"
        #include "Point.h"
        #include "Color.h"
        #include "Vector2.h"
        #include "Vector3.h"
        #include "Vector4.h"
        #include "Matrix.h"

        #include "GameWindow.h"
        #include "Game.h"

        namespace IO
        {
            #include "File.h"
        }

        namespace Input
        {
            #include "ButtonState.h"
            #include "MouseState.h"
            #include "Keyboard.h"
            #include "Mouse.h"
            #include "TouchPanel.h"
        }

        namespace Graphics
        {
            #include "PrimitiveType.h"
            #include "VertexElementFormat.h"
            #include "VertexElementUsage.h"
            #include "VertexElement.h"
            #include "VertexDeclaration.h"
            #include "OpenGLExtensions.h"
            #include "DepthFormat.h"
            #include "BufferUsage.h"
            #include "PresentationParameters.h"
            #include "RenderTarget.h"
            #include "OpenGLVersion.h"
            #include "Viewport.h"
            #include "Shader.h"
            #include "VertexShader.h"
            #include "FragmentShader.h"
            #include "GeometryShader.h"
            #include "ShaderContainer.h"
            #include "VertexBuffer.h"
            #include "GraphicsDevice.h"
            #include "VertexBuffer.h"
            #include "GraphicsDeviceManager.h"
        }
    }
}

#endif // RAZOR_H

What kind of problems can I get in the future if I organize my library in this way? Do you think this is really a bad style of coding?

I don't think there is anything inherently wrong with nested namespaces.

I also disagree that it is rare. It is just not exposed directly.

std::tr1
CompanyName::ProductName
CompanyName::ProductName::Details

But like all features they can be abused so use judiciously. But saying that I see nothing wrong with your namespace hierarchy. BUT the way you are implementing it is a very bad idea.

If I include the file "File.h" I expect everything included to be declared in the correct namespace. But the way you define it, it is now possible to include it in a way that puts the classes in the wrong namespace.

Each header file should be designed so that it can not be used incorrectly.


Show all primes in go I'm trying to go through Adrian simple programming tasks; Elementary task 8     It's basically asking for an infinite loop to print every prime number as it finds it.

Are there too many if statements? Call it intuitive knowledge, but something is just bugging me, like I'm missing something. Like, I can remove one of the if statements. So I'm wondering what you guys will find.

package main

import "fmt"

func main() {
    var current_prime int
    var prime bool
    current_prime = 0

    for {
        prime = true
        current_prime++

        for i := 2; i < current_prime; i++ {
            if current_prime % i == 0 {
                prime = false
                i = current_prime
            }
        }

        if prime {
            fmt.Println("found prime:", current_prime);
        }
    }
}

Simple Programming Problems

  Write a program that prints all prime numbers. (Note: if your
  programming language does not support arbitrary size numbers, printing
  all primes up to the largest number you can easily represent is fine
  too.)

Your program says one is prime. That is not correct.

The only even prime number is two. You don't appear take advantage of that. That is inefficient.

A number can only be divisible by a number less than or equal to its square root. You don't appear take advantage of that. That is inefficient.

You are asked to "[print] all primes up to the largest number you can easily represent." In Go, type int is either 32 or 64 bits. You don't guarantee the largest number by using type int64. Your program is not correct.

You have no program termination condition. Your program is not correct.

And so on.

For example, fixing your code,

package main

import (
    "fmt"
    "math"
)

func main() {
    fmt.Println("prime numbers:")
    fmt.Println(2)

    for n := int64(3); n > 0; n += 2 {
        prime := true

        r := int64(math.Sqrt(float64(n))) + 1
        for i := int64(3); i < r; i += 2 {
            if n%i == 0 {
                prime = false
                break
            }
        }

        if prime {
            fmt.Println(n)
        }
    }
}

To provide a measure of performance, the results from a Go benchmark for all prime numbers up to 32,771:

BenchmarkPeterSO-4           500       2661556 ns/op
BenchmarkIbnRushd-4            3     492864429 ns/op


JSON to data structure in Javascript I've never formally learned javascript, and I feel very much like I'm unaware of 'good' practice.

I'm working on this open source project, and in particular the code that converts the incoming JSON into a usable data structure in this file

The relevent function looks like this:

   function start() {
       key = "toppage";
       utterances = {};
       links = {};
       colours = {};
       icons = {};
       labels = {};
       slide_number = {};
       var req = new XMLHttpRequest();
       req.open("GET", "pageset.json");
       req.overrideMimeType("application/json");
       req.send(null);
       req.onreadystatechange = function() {
           if (req.readyState == 4 && req.status == 200) {
               var obj = JSON.parse(req.responseText);
               for (grid in obj.Grid) {
           console.log(obj.Grid[grid][0])
                   labels[obj.Grid[grid][0]] = obj.Grid[grid][1];
                   utterances[obj.Grid[grid][0]] = obj.Grid[grid][2];
                   links[obj.Grid[grid][0]] = obj.Grid[grid][3];
                   icons[obj.Grid[grid][0]] = obj.Grid[grid][4];
                   colours[obj.Grid[grid][0]] = obj.Grid[grid][5];
                   slide_number[obj.Grid[grid][0]] = obj.Grid[grid][6];
           if(obj.Grid[grid][6]==0){
            key=obj.Grid[grid][0]
            }
               }
               grid_size_rows = obj.Settings[0];
               grid_size_columns = obj.Settings[0];
               setup_messagewindow();
               setup_table();
               load_page(key);
           }
       };
       //TODO - needs an error message if the JSON doesn't load
   }

I have NO idea if this is a reasonable way to populate structures in javascript.  It works, but I feel like it could be a hell of a lot more elegant.  Any comments people have would be welcome.

First of all,you declare them without anything,so it's probably declared globally,this is not good,with this amount of variables in global.

Second of all,ES6 Destructuring assignment

var arr=[1,2,3,4,5];
var [a,b,c,d,e]=arr;
console.log(a,b,c,d,e); // 1,2,3,4,5

Using the same way,

for (grid in obj.Grid) {
    var i=obj.Grid[grid][0];
    var [unused,labels[i], utterances[i], links[i], icons[i], colours[i], slide_number[i]]
    =obj.Grid[grid]
    //....
}


First Code, But I Feel It Is So Ugly, Can You Explain Why? Question:- Why is this code so ugly ??
This is my first code, it is working fine, but I feel it doesn't looks as standard code … I am still studying java through youtube, as I have just completed two topics language fundamentals and operators and assignments in java.
I have tried to create code that calculates loan on particular date and at particular rate of interest, it takes user inputs.  Please Run This Code At Your IDE.
I have created this code by just knowing two topics or say first two chapters of basic java.

package learningJava;
import java.util.Scanner;

public class Test
{

    public static void main(String[] args)

     {
     Scanner sc = new Scanner (System.in);

    System.out.println("if Days is not calculated just type 0");

    System.out.print("Days: ");

    double day = sc.nextDouble();

    if (day<=0)
    {
    System.out.println("Old Year: ");

    double oldyear = sc.nextDouble();

    System.out.println("Old Month: ");

    double oldmonth = sc.nextDouble();

    System.out.println("Old Date: ");

    double olddate = sc.nextDouble();

    System.out.println("New Year: ");

    double newyear = sc.nextDouble();

    System.out.println("New Month: ");

    double newmonth = sc.nextDouble();

    System.out.println("New Date: ");

    double newdate = sc.nextDouble();

     day = ((newyear*365)+(newmonth*30)+(newdate))-(((oldyear*365)+(oldmonth*30)+(olddate)));

     System.out.println("Days = "+day);

     System.out.print("Principle: ");

     double principle = sc.nextDouble();

     System.out.print("Rate Of Interest: ");

     double rateofinterest = sc.nextDouble();

     double interest = (((principle*(rateofinterest/100))/30)*day);

     System.out.println("--------------------------------");

     System.out.println("Interest = Rs " +interest);

     System.out.println("                                 ");

     System.out.println("Total Amount = Rs " +(principle+interest));

     System.out.println("--------------------------------");

     System.out.println("Thankyou!!");

    }

else
    {

     System.out.print("Principle: ");

     double principle = sc.nextDouble();

     System.out.print("Rate Of Interest: ");

     double rateofinterest = sc.nextDouble();

     double interest = (((principle*(rateofinterest/100))/30)*day);

     System.out.println("--------------------------------");

     System.out.println("Interest = Rs " +interest);

     System.out.println("                                 ");

     System.out.println("Total Amount = Rs " +(principle+interest));

     System.out.println("--------------------------------");

     System.out.println("Thankyou!!");

     }

}

}

it is a bit hard to compare it with "standard code" if you only worked through first two chapters ;)

You should consider three principles in the following order:

The code should work
The code should be readable
The code should be extendable

Only the 1st is proofable. But maybe not that easy as one might think. Consider edge cases and errors. It takes some experience to know them. But usually something like

Boundary values - e.g. 0 for a lower boundary
Wrong format of parameter e.g. 0,5 instead of 0.5 (for english local)
Input is out of allowed boundary

There are a lot of ways to handle them, like to ask for a new value or to simply abort.

If your code is readable is matter of taste. I prefer short code blocks in methods that are named. Also repeated code should be avoided.
That said here is my first concrete suggestion for you: There are common code blocks where you ask the user for a value (System.out.print(...)) and then read the value (sc.nextDouble()). You could combine them to a method, like

private static double askForDouble(String label, Scanner scanner){
    System.out.print(label);
    return scanner.nextDouble();
}

which you can use then instead of every occurance:

System.out.print("Days: ");
double day = sc.nextDouble();

becomes

double day = askForDouble("Days: ", sc);

I would also put every calculation in separate functions although that would not give any benefit at the moment. But later you could write unit tests.

There are more possible improvements, but you should go on with the tutorials to learn about them.


Create Excel file multiple times with different names I want to create a new Excel files from an Apache POI object. The filename should contain the standard name, the current date and a counter, if necessary, to prevent collisions. I have a working solution here:

private File createFile() throws FileNotFoundException, IOException {
    LOG.debug("Create File");
    DateFormat df = new SimpleDateFormat("ddMMyyyy");
    Date d = new Date(System.currentTimeMillis());
    while (true) {
        File f = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + ".xlsx");
        File f2 = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + " ("
                + fcounter + ").xlsx");
        if (!f.exists()) {
            try (FileOutputStream outputStream = new FileOutputStream(
                    "C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + ".xlsx")) {
                wb.write(outputStream);
                outputStream.close();
                wb.close();
                return f;
            } catch (Exception e) {
                LOG.debug(e.toString());
            }
        } else if (!new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + " ("
                + fcounter + ").xlsx").exists()) {
            try (FileOutputStream outputStream = new FileOutputStream("C:\\Users\\" + filepath
                    + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + " (" + fcounter + ").xlsx")) {
                wb.write(outputStream);
                outputStream.close();
                wb.close();
                return f2;
            } catch (Exception e) {
                LOG.debug(e.toString());
            }
        }
        fcounter++;
    }
}

Is there an existing solution or a shorter way to do it? I think my code is very long for such a task.

Short answer

Yes, there are shorter ways to do. Here is how I would save a workbook into a file:

private static File createFile(XSSFWorkbook wb, String folderTo) throws FileNotFoundException, IOException {
    LOG.debug("Create File");
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    File f = Paths.get(folderTo, "Bloomberg Rechnung-" + timestamp + ".xlsx").toFile();
    try (FileOutputStream fos = new FileOutputStream(f)) {
        wb.write(fos);
    }
    return f;
}

Note that I'm not totally comfortable with this method because it has 2 responsibilities: creating an excel file and filling it.

Long answer

Now you might wonder how I ended with the code above, so let's begin...

Filename requirement

You stated

  Note that if the file is not existing it should be created without counter

This kind of requirement is the kind of which clutters the code with at least one if/else and is in general irrelevant. If you are okay to have a number in the filename (going from 1 to n), then why not having 0 ?

After all, the requirement behind that is that no filename should be erased by another with the same name. So the real problem to resolve here is how to make sure this method won't create 2 files with the same name ?

Since you stated

  I want to create a new excel file if my program is run

I assumed using a timestamp should be enough (guaranteeing you one unique filename per second).

Here is createFile() after this refactor:

private File createFile() throws FileNotFoundException, IOException {
    LOG.debug("Create File");
    DateFormat df = new SimpleDateFormat("yyyyMMddhhmmss");
    Date d = new Date(System.currentTimeMillis());
    File f = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + ".xlsx");
    try (FileOutputStream outputStream = new FileOutputStream(
            "C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + ".xlsx")) {
        wb.write(outputStream);
        outputStream.close();
        wb.close();
    } catch (Exception e) {
        LOG.debug(e.toString());
    }
    return f;
}

Using LocalDateTime

Since Java 8, there's a new API to use when you are using dates. I refactored the code to use this new API instead of the old one.

private File createFile() {
    LOG.debug("Create File");
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    File f = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + timestamp + ".xlsx");
    try (FileOutputStream outputStream = new FileOutputStream(
            "C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + timestamp + ".xlsx")) {
        wb.write(outputStream);
        outputStream.close();
        wb.close();
    } catch (Exception e) {
        LOG.debug(e.toString());
    }
    return f;
}

Removing duplication

You are creating the File twice. Instead just reuse the existing you already have.

private File createFile() {
    LOG.debug("Create File");
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    File f = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + timestamp + ".xlsx");
    try (FileOutputStream outputStream = new FileOutputStream(f)) {
        wb.write(outputStream);
        outputStream.close();
        wb.close();
    } catch (Exception e) {
        LOG.debug(e.toString());
    }
    return f;
}

Removing smells

In the current code, there are (to my sense) three code smells:

Exceptions are (badly) swallowed
wb is closed but not opened in this method. This could be a major flaw because it induces a nasty side-effect. It is a general idiom that the one who opens a resource is the one who closes it.
outputStream is declared in the try, no need to close it manually ! (it will be done automatically)

Since in the original method, you declared throws FileNotFoundException, IOException, I assumed it's ok to remove the try/catch.

private File createFile() throws FileNotFoundException, IOException {
    LOG.debug("Create File");
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    File f = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + timestamp + ".xlsx");
    try (FileOutputStream outputStream = new FileOutputStream(f)) {
        wb.write(outputStream);
    }
    return f;
}

Going further

So now, I could have stopped here. But something is still bothering me:

The hardcoded string for the path
The fact that you directly depends on wb, making the method hard to test

The method could event be marked static so anyone reading this code knows that this method doesn't have a side-effect on an object attribute.

private static File createFile(XSSFWorkbook wb, String folderTo) throws FileNotFoundException, IOException {
    LOG.debug("Create File");
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    File f = Paths.get(folderTo, "Bloomberg Rechnung-" + timestamp + ".xlsx").toFile();
    try (FileOutputStream outputStream = new FileOutputStream(f)) {
        wb.write(outputStream);
    }
    return f;
}

Going even further

As I said in the first chapter, the method has actually 2 responsibilities. I would split those in something like that:

private static File createFile(String folderTo) {
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    return Paths.get(folderTo, "Bloomberg Rechnung-" + timestamp + ".xlsx").toFile();
}

private static void saveWorkbook(XSSFWorkbook wb, File f) throws FileNotFoundException, IOException {
    try (FileOutputStream outputStream = new FileOutputStream(f)) {
        wb.write(outputStream);
    }
}

I also voluntarily removed the log statement. I'm not sure it really add any value when only logging "Create File". Either add more information such as where the file was created, with which name, etc. or get rid of it.

In the end you could use these methods like this:

saveWorkbook(wb, createFile("C:\Users\Desktop"));

or if you need to store a reference on the File:

File f = createFile("C:\Users\Desktop");
saveWorkbook(wb, f);

Update

Following your comment, this is how I would implement the file creation based only on a date + a counter (if needed)

private static File createFile(String folderTo) {
    String date = LocalDate.now().format(DateTimeFormatter.ofPattern("yyyyMMdd"));
    return createUniqueFileForDate(folderTo, date);
}

private static File createUniqueFileForDate(String folderTo, String date) {
    String index = "";
    File f = null;
    for (int i = 1;; i++) {
        f = Paths.get(folderTo, "Bloomberg Rechnung-" + date + index + ".xlsx").toFile();
        if (!f.exists()) {
            break;
        }
        index = " (" + i + ")";
    }
    return f;
}


Solving a variation of the "Partition Problem" In my version of the Partition Problem, I have a set of weights that are all powers of three (1, 3, 9, 27 etc.). I only have one of each weight. There is some object (the weight of this object is input) on the left side of a scale and I need to add weights to either side to balance it. I can choose not to use a weight if I so choose (denoted by a '-').

Right now, my program converts the inputted weight into ternary and, if all the digits are 1's and 0's, just feeds in the appropriate factor of three into a list. Otherwise, it generates every possibility, iterating through them until both sides are equal. This is, predictably, very slow. I know the partition problem is NP-C but is there any optimizations I can make here?

import string
import pdb
import itertools

def nearestpowerof3(x):
    numbers = '012'
    if x < 0:
        sign = -1
    elif x == 0:
        return numbers[0]
    else:
        sign = 1
    x *= sign
    digits = list()
    while x:
        digits.append(numbers[x % 3])
        x = int(x / 3)
    if sign < 0:
        digits.append('-')
    digits.reverse()
    ternary = ''.join(digits)
    exp = int(len(ternary))
    return exp, 3 ** exp, ternary

def answer(x):
    product = []

    tern = str(nearestpowerof3(x)[2])
    print(tern)
    if tern.find('2') == -1:
        print('HELLO')
        for digit in tern[::-1]:
            if digit == '1':
                product.append("R")
            elif digit == '0':
                product.append("L")
        return product
    else:
        for product in itertools.product('RL-', repeat=len(tern) + 1):
            left = x
            right = 0
            for i in range(len(product)):
                if product[i] == 'R':
                    right += 3**i
                elif product[i] == 'L':
                    left += 3**i
            if left == right:
                if product[-1] == '-' and product[-2] != '-':
                    product = product[:-1]
                return product
print(answer(10000000))

I don't see a partition problem here. What the task suggests is a representation of a given number in the form of \$\Sigma a_n3^n\$ where \$a_n \in \{-1,0,1\}\$. I highly recommend proving to (or at least convincing) yourself that every number has such representation. Then you may want to see how few first numbers are represented:

    1                   1
    2                  -1  1
    3                   0  1
    4                   1  1
    5                  -1 -1  1
    6                   0 -1  1
    7                   1 -1  1
    8                  -1  0  1
    9                   0  0  1
    10                  1  0  1
    11                 -1  1  1
    12                  0  1  1
    13                  1  1  1
    14                 -1 -1 -1  1

(first column is for 1, second for 3, third for 9, etc) and figure out the rule of sequencing the coefficients for each power of 3 (think reminders). The actual coding becomes straightforward.