Exercise from Accelerated C++ I am working through the exercises in the book Accelerated C++.  I wanted to get some feedback on proper use of the Standard Library, correctness, overall style, portability, etc.

/*******************************************************************************
Accelerated C++ Chapter 3

Write a program to count how many times each distinct word appears in its input
*******************************************************************************/

#include <iostream>
#include <vector>
#include <string>

int main()
{
    std::cout << "Enter a list of words followed by end-of-file: ";

    typedef std::vector<std::string>::size_type vector_sz;
    std::vector<std::string> words;
    std::string word;
    while (std::cin >> word)
    {
        words.push_back(word);
    }

    vector_sz size = words.size();
    if(size == 0)
    {
        std::cout << "Please enter some words!";
        return -1;
    }

    std::vector<int> frequency;
    std::vector<std::string> unique_words;
    int count = 0;
    bool isUnique;

    unique_words.push_back(words[0]);

    for(unsigned i = 0; i < words.size(); ++i)
    {
        isUnique = true;

        for(unsigned j = 0; j < unique_words.size(); ++j)
        {
            if(words[i] == unique_words[j])
            {
                isUnique = false;
            }
        }
      if(isUnique)
      {
          unique_words.push_back(words[i]);
      }
    }

    for(unsigned i = 0; i < unique_words.size(); ++i)
    {
        for(unsigned j = 0; j < words.size(); ++j)
        {
            if(unique_words[i] == words[j])
            {
                count += 1;
            }
        }
        frequency.push_back(count);
        count = 0;
    }

    for(unsigned i = 0; i < unique_words.size(); ++i)
    {
        std::cout << unique_words[i] << "   " << frequency[i] << std::endl;
    }
    return 0;
}

You could cut your code in half if you used a map:

typedef   std::map<std::string, unsigned int> Map;

Map tMap;
while(std::cin >> word)
{
    // When accessing a map with `[]` If the value does not exist
    // it is created and initialized to zero.
    //
    // The plus plus then increments the stored value
    tMap[word]++;
}

// Iterate through map and print frequency

Couple other notes from the code you have:

After unique_words.push_back(words[0]); you can start the i loop at index 1 instead of 0.
When checking for uniqueness in the first pair of loops, once you determine uniqueness to be false you can stop looping for that word: while(j < unique_words.size() && isUnique) ...


Thoughts on this conversion of code from Step.js to Q promise library? I have some code that I'd written using this pattern with Step.js.  In this case, talking to MongoDB in Node:

Step(
    function connectToDatabaseWithAuthorization() {
        mongodb.connect(mongoConnectURI, this);
    },

    function getCommitCollection(err, conn) {
        if (err) handleResErr(res, err);

        conn.collection('commits', this);
    },

    function addCommitToCollection(err, coll) {
        if (err) handleResErr(res, err);

        var commit = JSON.parse(req.param('commit', null));
        commit.commit_date = requestTime;
        commit.commit_id = common.makeIdFromCommit(commit);
        coll.insert(commit, {safe: true}, this);
    },

    function respondWithShowAndVerifyUrlsInJson(err, records) {
        if (err) handleResErr(res, err);

        res.json({
            commit_date: records[0].commit_date
        });
    }
)

When I went to update my Node libraries, I noticed Step wasn't getting much attention or undergoing much development.  And I wasn't happy about the error handling being spread into each function in the steps.

So I went to research modern alternatives, and Async.js seemed more popular.  But when I asked about how to convert it, someone suggested I look into Q Promises instead.

I came up with this transformation, which seems to work:

Q.try(function() {

    return Q.ninvoke(mongodb, 'connect', mongoConnectURI);

}).then(function (conn) {

    return Q.ninvoke(conn, 'collection', 'commits');

}).then(function (coll) {

    var commit = JSON.parse(req.param('commit', null));
    commit.commit_date = requestTime;
    commit.commit_id = common.makeIdFromCommit(commit);
    return Q.ninvoke(coll, 'insert', commit, {safe: true});

}).then(function (records) {

    res.json({
        commit_date: records[0].commit_date
    });

}).catch(function (err) {

    handleResErr(res, err);

});

Not a large piece of code, but I could use a sanity check.  I was having problems with figuring out if I should be using .done() in there somewhere, and also I couldn't get .finally to work.  If I put it on the end of the chain, it ran after the first promise and not after all of them.  :-/

While I mostly think the generality makes it an improvement, it does really obscure the method calls.  Q.ninvoke(mongodb, 'connect' mongoConnectURI) is quite a bit uglier than mongodb.connect(mongoConnectURI).  Have people made some better way to do mongodb calls with promises than this?

Interesting that you're getting .finally() after your first method rather than at the end. Should I read that to mean that your code executed as: connect-->finally-->[DONE] or did control flow somewhere else in the promise chain subsequent to finally?

Regarding the aesthetics - I couldn't agree with your more; ninvoke and its relatives are not the easiest to read. Two main options. One, you can look for a promise-enabled library for this or that. This is probably not my first choice unless the library would be giving you something more than just a promises wrapper. Two, you can get yourself some nicer looking functions by using the denodeify function. Consider if you wrote a module like this (you could of course inline this code as well which I often do for both initial development as well as if I find I really don't need more than a function or two):

var Q = require('q');
var lib = require('someNodeModule');

module.exports = {
   connect: Q.denodeify(lib.connect),
   read: Q.denodeify(lib.read),
   write: Q.denodeify(lib.write)
}

At this point, you have a new module with each of the above functions being promise-returning ones rather than callback-invoking ones. If you require the above as, say libp, you could then replace your various return Q.ninvoke(...) calls with much more clear calls such as a simple return libp.connect(...). The signature of the function stays the same (other than the obvious loss of the final callback).

I will try to return to this later this evening as well regarding the odd .finally() behavior. In the meantime, you might find this Q&A relevant to an issue you did not raise, but may encounter. More generally, the individual providing the answer, Bergi, appears to be quite expert with promises (from other answers I have seen), so you might check out some other answers or even solicit input on this. I am a journeyman, at best, with promises. I think I have a reasonable understanding of the concepts and library, but have not yet accumulated all the idioms that would be helpful.


Optimal buffer size for output stream I'm writing a method to save an input stream to an output stream. What's the optimal size for the buffer? Here's my method:

/**
* Saves the given InputStream to a file at the destination. Does not check whether the destination exists.
*
* @param inputStream
* @param destination
* @throws FileNotFoundException
* @throws IOException
*/
public static void saveInputStream(InputStream inputStream, File outputFile) throws FileNotFoundException, IOException {
  try (OutputStream out = new FileOutputStream(outputFile)) {
    byte[] buffer = new byte[2097152]; //This is set to two MB. But I have no idea whether this is optimal
    int length;
    while ((length = inputStream.read(buffer)) > 0) {
      out.write(buffer, 0, length);
    }
    inputStream.close();
  }
}

It depends on a lot of factors, there's no universally "optimal" size. 512kB is probably good enough.

If you want, you can always benchmark it for various buffer sizes: this will let you know what the best option is for your computer and OS.


Function to return subword of a camelcase string Given a camelcase string and an index, return the subword of the string that includes that index, e.g.:

find_word('CamelCaseString', 6) -> 'Case'
find_word('ACamelCaseString', 0) -> 'A'

My code:

def find_word(s, index):
    for i in range(index, 0, -1):
        if s[i].isupper():
            left = i
            break
    else:
        left = 0

    for i in range(index, len(s)-1):
        if s[i].islower() and s[i+1].isupper() or s[i:i+2].isupper():
            right = i
            break
    else:
        right = len(s) - 1

    return s[left:right+1]

Can this be made more concise/efficient?

Review

Add docstrings and tests... or both in the form of doctests!

def find_word(s, index):
    """
    Finds the CamalCased word surrounding the givin index in the string

    >>> find_word('CamelCaseString', 6)
    'Case'
    >>> find_word('ACamelCaseString', 0)
    'A'
    """

    ...

Loop like a native.

Instead of going over the indexes we can loop over the item directly

range(index, 0, -1)

We can loop over the item and index at the same time using enumerate

for i, s in enumerate(string[index:0:-1])

However this would be slower since it will create a new string object with every slice.
If we can be sure that the givin string is a CamalCase string

Then we can drop some of your second if statement

if s[i].islower() and s[i+1].isupper() or s[i:i+2].isupper():

Would be

 if s[i+1].isupper():

Actually your code (from a performance aspect) is quite good

We could however use a while loop to increment both side at once, for a little performance gain.

(slower, yet more readable) Alternative

A different approach to finding CamalCase words can be done with regex,

We can find all CamalCase words with the following regex: r"([A-Z][a-z]*)"

And we can use re.finditer to create a generator for our matches and loop over them, and return when our index is in between the end and the start.

import re

def find_word_2(string, index):
    for match in re.finditer(r"([A-Z][a-z]*)", string):
        if match.start() <= index < match.end():
            return match.group()

NOTE This yields more readable code, but it should be alot slower for large inputs.


How object oriented is my Crazy Eights game? I have this Crazy Eights game (which at the moment does not include crazy cards):

class Card(object):
    RANKS = ("2", "3", "4", "5", "6", "7", "8", "9", "T", "J", "Q", "K", "A")
    SUITS = ("c", "d", "h", "s")

    def __init__(self, rank, suit):
        self.rank = rank
        self.suit = suit

    def __str__(self):
        return self.rank + self.suit

    def __eq__(self, other):
        return self.rank == other.rank and self.suit == other.suit

    def canPlayOn(self, other):
        return self.rank == other.rank or self.suit == other.suit

class Hand(object):
    def __init__(self):
        self.cards = []

    def __str__(self):
        return " ".join([str(card) for card in self.cards])

    def __bool__(self):
        return bool(self.cards)

    def give(self, other, card):
        self.remove(card)
        other.add(card)

    def add(self, card):
        self.cards.append(card)

    def remove(self, card):
        self.cards.remove(card)

class Pile(Hand):
    @property
    def topCard(self):
        return self.cards[-1]

class DrawPile(Pile):
    def populate(self):
        for rank in Card.RANKS:
            for suit in Card.SUITS:
                self.add(Card(rank, suit))

    def deal(self, others, cards = 7):
        import random
        random.shuffle(self.cards)
        for _ in range(cards):
            for hand in others:
                self.give(hand, self.topCard)

class Player(Hand):
    def playMove(self, discardPile, drawPile):
        card = self.getCard(discardPile.topCard)
        self.give(discardPile, card) if card else \
                               drawPile.give(self, drawPile.topCard)

    def getCard(self, topCard):
        card = input("This is your hand: " + str(self) +
                     ". The is the top card: " + str(topCard) +
                     ". Enter your card (enter PU to pick up a card): ")
        while self.isInvalid(card, topCard):
            card = input("Invalid input. Enter your card: ")
        return Card(card[0], card[1]) if card != "PU" else None

    def isInvalid(self, card, topCard):
        if card == "PU":
            return False
        elif len(card) != 2 or card[0] not in Card.RANKS or \
           card[1] not in Card.SUITS or \
           Card(card[0], card[1]) not in self.cards or \
           not Card(card[0], card[1]).canPlayOn(topCard):
            return True
        return False

class Computer(Hand):
    def playMove(self, discardPile, drawPile):
        card = self.getCard(discardPile.topCard)
        if card:
            self.give(discardPile, card)
            print("Computer plays " + str(card), end = ". ")
        else:
            drawPile.give(self, drawPile.topCard)
            print("Computer picks up.", end = " ")
        print("Computer has " + str(len(self.cards)) + " cards remaining.")

    def getCard(self, topCard):
        for card in self.cards:
            if card.canPlayOn(topCard):
                return card
        return None

def main():
    drawPile = DrawPile()
    drawPile.populate()
    player = Player()
    computer = Computer()
    playerList = [player, computer]
    drawPile.deal(playerList)
    discardPile = Pile()
    drawPile.give(discardPile, drawPile.topCard)
    playerIndex = 0
    while isStillPlaying(playerList):
        playerList[playerIndex].playMove(discardPile, drawPile)
        if playerIndex < len(playerList) - 1:
            playerIndex += 1
        else:
            playerIndex = 0
    print("Computer wins!" if player else "You win!")

def isStillPlaying(playerList):
    for player in playerList:
        if not player:
            return False
    return True

if __name__ == "__main__":
    main()

Can someone provide a code review (particularity on my OOP skills)? Also, what would be the most object oriented way to implement "crazy cards"?

I'm just going to look at your OOP design for this review, as you requested.

Your canPlayOn() method does not belong in the Card class.  The purpose of your Card class should be to represent a card, and nothing more.  You should be able to use the same Card class for Blackjack, Bridge, or any other card game.  Logic specific to Crazy Eights does not belong there, and should probably be moved into the Player class.

I was initially thinking that your Hand should consist of a set of cards rather than an array, since the items are distinct and order doesn't matter.  Then I saw that your Pile derives from Hand, and in Pile, order does matter.  Maybe using an array is OK after all, if you accept that a Pile is a subclass of Hand.  I think that such an inheritance relationship is more of a hack than an accurate model, but I can let it slide because it seems to work in this case.

In DrawPile.deal(), I would rename the others parameter to recipient_hands.  Also imports should go at the beginning of the file.

I take issue with Player(Hand) and Computer(Hand).  First of all, I would say that a player has a hand rather than a player is a hand.  That suggest that composition should be favoured over inheritance.  Also, the computer is a kind of player, so there should be an inheritance relationship there.  Furthermore, there is a lot of redundant code between Player and Computer, differing mainly in what the output looks like.  Perhaps your Player and Computer classes could be merged into a Crazy8Player class, with the constructor taking name, narrator, and strategy arguments.


Cloning Distinct XML structures without data in PHP I struggled for the last couple days on this question and finally came up with a solution. But it's got so many loops I get dizzy just looking at the code - from recursive functions with loops to loops within loops.

I really expected to find a canned function in a popular XML class to extract all the unique nodes without data, but I am surprised how non-trivial I've found this endeavor.

While this works for my piddly example data, I have performance and scalability concerns when trying to use this code in production. For example, this elegant answer uses XSLT to solve the problem, but it's limited by node depth. So is there anyway to improve efficiency by cutting out a loop, or using less objects, or even utilizing a different class or technology completely? Are there any glaring failing conditions that aren't represented in my example data?

<?php
$xml = '
<root>
  <node/>
  <node>
    more
  </node>
  <node>
    another
  </node>
  <node>value</node>
</root>
';
$doc = new DOMDocument();
$doc->loadXML($xml);

// clone without data
$empty_xml = new DOMDocument();
$empty_xml->appendChild($empty_xml->importNode($doc->documentElement));
function clone_without_data(&$orig, &$clone, &$clonedoc){
  foreach ($orig->childNodes as $child){
    if(get_class($child) === "DOMElement")
      $new_node = $clone->appendChild($clonedoc->importNode($child));
    if($child->hasChildNodes())
      clone_without_data($child,$new_node,$clonedoc);
  }
}
clone_without_data($doc->documentElement, $empty_xml->documentElement, $empty_xml);

// remove all duplicates
$distinct_structure = new DOMDocument();
$distinct_structure->appendChild($distinct_structure->importNode($doc->documentElement));
foreach ($empty_xml->documentElement->childNodes as $child){
  $match = false;
  foreach ($distinct_structure->documentElement->childNodes as $element){
    if ($distinct_structure->saveXML($element) === $empty_xml->saveXML($child)) {
      $match = true;
      break;
    }
  }
  if (!$match)
    $distinct_structure->documentElement->appendChild($distinct_structure->importNode($child,true));
}
$distinct_structure->formatOutput = true;
echo $distinct_structure->saveXML();

This results in the unique XML structures stripped of all data:

<?xml version="1.0"?>
<root>
  <node/>
  <node>

  </node>
</root>

I would consider getting all nodes using getElementsByTagName('*') (note wildcard usage).

I would also consider simply cloning the $doc object.

That could simplify to this:

$doc = new DOMDocument();
$doc->loadXML($xml);
$clone = cloneDOMDocument($doc, true);

function cloneDOMDocument(DOMDocument $source, $remove_values = false) {
    $clone = clone $source;
    if($remove_values === true) {
        // select all nodes in clone
        $all_nodes = $clone->getElementsByTagName('*');

        // iterate all nodes in clone, resetting leaf values to empty string
        foreach($all_nodes as $node) {
            if($node->hasChildNodes() === false) {
                $node->nodeValue = '';
            }
        }
    }
    return $clone;
}


Let's speed that file sentence searching program Intro:
I've written a small piece of Python program which is looking for a given sentence in multiple sub directories of a given path.
I'm looking for improvements regarding the speed of my script.

Code:
from os import walk
from os.path import join

def get_magik_files(base_path):
    """
    Yields each path from all the base_path subdirectories

    :param base_path: this is the base path from where we'll start looking after .magik files
    :return: yield full path of a .magik file
    """
    for dirpath, _, filenames in walk(base_path):
        for filename in [f for f in filenames if f.endswith(".magik")]:
            yield join(dirpath, filename)

def search_sentence_in_file(base_path, sentence):
    """
    Prints each file path, line and line content where sentence was found

    :param base_path: this is the base path from where we'll start looking after .magik files
    :param sentence: the sentence we're looking up for
    :return: print the file path, line number and line content where sentence was found
    """
    for each_magik_file in get_magik_files(base_path):
        with open(each_magik_file) as magik_file:
            for line_number, line in enumerate(magik_file):
                if sentence in line:
                    print('[# FILE PATH    #] {} ...\n'
                          '[# LINE NUMBER  #] At line  {}\n'
                          '[# LINE CONTENT #] Content: {}'.format(each_magik_file, line_number, line.strip()))
                    print('---------------------------------------------------------------------------------')

def main():
    basepath = r'some_path'
    sentence_to_search = 'some sentence'

    search_sentence_in_file(basepath, sentence_to_search)

if __name__ == '__main__':
    main()

Miscellaneous:
As you may already figured out, the reason for my program being so slow resides in search_sentence_in_file(base_path, sentence) where I need to open each file, read it line by line and look for a specific sentence.
I know I could use a logging library instead of printing the results to see who matched what, but that wouldn't serve the purposes of the program. So I'm not looking for that (I'm building this to have a fast way of looking for certain classes / methods / slots definitions in multiple .magik files in a fast manner. Opening a log file won't satisfy me).
For whoever is interested in the Magik language, and as a bonus of taking your time to take a look at this question, here's a small introduction to Magik.

To sum up:

is there any way of improving the speed of my program ?
do you have other suggestions regarding the way I'm searching a sentence ?

PS: I'm looking for answers which aims a Windows distro.
Any other improvements are welcomed !

Yay, PEP 8

72 characters for docstrings, 79 for the code. The rest seems fine.

Separation of concerns

search_sentence_in_file should search, and return its results. Not print, it is the duty of the caller.

I feel it is also wrongly named as it search a sentence in several files. So at least add the missing s at the end of the name. And to make it even more reusable, why not pass an iterable of filepath (like the get_magic_files generator)?

Genericity

Besides search_sentence_in_file accepting an iterable, you could make get_magik_files more generic by passing the required extension as a parameter. This will let you extend your script to allow search in various kind of files.

First rewrite

from os import walk
from os.path import join, splitext

def get_files(base_path, extension=None):
    """
    Yields each path from all the base_path subdirectories

    :param base_path: this is the base path from where the
                      function start looking for relevant files
    :param extension: filter files using provided extension.
                      If None, no filter is applied.
    :return: yield full path of a requested file
    """
    if extension is None:
        def filter_files(filenames):
            yield from filenames
    else:
        def filter_files(filenames):
            for filename in filenames:
                if splitext(filename)[1] == extension:
                    yield filename

    for dirpath, _, filenames in walk(base_path):
        for filename in filter_files(filenames):
            yield join(dirpath, filename)

def search_sentence_in_files(files, sentence):
    """
    Yield each file path, line and line content where
    sentence was found.

    :param files: iterable of files to search the sentence into
    :param sentence: the sentence we're looking up for
    :return: yield the file path, line number and line
             content where sentence was found
    """
    for filepath in files:
        with open(filepath) as fp:
            for line_number, line in enumerate(fp):
                if sentence in line:
                    yield filepath, line_number, line.strip()

def main():
    basepath = r'some_path'
    sentence_to_search = 'some sentence'

    files = get_files(basepath, 'magik')
    results = search_sentence_in_files(files, sentence_to_search)
    for filepath, line, content in results:
        print('[# FILE PATH    #]', filepath, '...')
        print('[# LINE NUMBER  #] At line', line)
        print('[# LINE CONTENT #] Content:', content)
        print('-'*80)

if __name__ == '__main__':
    main()

Reusability

Your script make it hard to reuse for other purposes: different sentences, different kind of files. Better to add a CLI using argparse. Provide sensible default for your current usage but allows for customization at will.

from os import walk
from os.path import join, splitext
import argparse

def get_files(base_path, extension=None):
    """
    Yields each path from all the base_path subdirectories

    :param base_path: this is the base path from where the
                      function start looking for relevant files
    :param extension: filter files using provided extension.
                      If None, no filter is applied.
    :return: yield full path of a requested file
    """
    if extension is None:
        def filter_files(filenames):
            yield from filenames
    else:
        def filter_files(filenames):
            for filename in filenames:
                if splitext(filename)[1] == extension:
                    yield filename

    for dirpath, _, filenames in walk(base_path):
        for filename in filter_files(filenames):
            yield join(dirpath, filename)

def search_sentence_in_files(files, sentence):
    """
    Yield each file path, line and line content where
    sentence was found.

    :param files: iterable of files to search the sentence into
    :param sentence: the sentence we're looking up for
    :return: yield the file path, line number and line
             content where sentence was found
    """
    for filepath in files:
        with open(filepath) as fp:
            for line_number, line in enumerate(fp):
                if sentence in line:
                    yield filepath, line_number, line.strip()

def main(files, sentence):
    results = search_sentence_in_files(files, sentence)
    for filepath, line, content in results:
        print('[# FILE PATH    #]', filepath, '...')
        print('[# LINE NUMBER  #] At line', line)
        print('[# LINE CONTENT #] Content:', content)
        print('-'*80)

if __name__ == '__main__':
    parser = argparse.ArgumentParser(description='Search text in files')
    parser.add_argument('sentence')
    parser.add_argument('-p', '--basepath',
                        help='folder in wich files will be examinated',
                        default=r'some folder')
    parser.add_argument('-e', '--extension',
                        help='extension of files to examine',
                        default='magik')
    args = parser.parse_args()

    files = get_files(args.basepath, args.extension)
    main(files, args.sentence)

You can also allows for other improvements such as the sentence being a regular expression.


Simple Password Generator in JavaScript let generatorButton = document.getElementById("generator");

generatorButton.addEventListener("click", () => {
    let passwordLength = 16;
    let characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789~!@#$%^&*()_";
    let password = "";

    for (i = 0;i < passwordLength;i++){
        password = password + characters.charAt(Math.floor(Math.random() * Math.floor(characters.length - 1)));
    }

    document.getElementById("textField").value = password;
});
:root {
    --gray: #CBC4C4;
    --blue: #029DF1;
    --white: #FFF;
}

* {
    font-family: Roboto, sas-serif;
    margin: 0;
    padding: 0;
    margin-top: 15px;
}

body {
    background: var(--blue);
}

.container {
    width: 300px;
    margin: 0 auto;
    display: flex;
    flex-direction: column;
    justify-content: center;
    align-items: center;
    background: var(--gray);
    font-size: large;
    border-radius: 30px;
}

button {
    background: var(--blue);
    color: var(--white);
    padding: 20px 20px;
    margin: 10px 5px;
    font-size: large;
    cursor: pointer;
}

button:focus {
    border: 2px solid var(--blue);
    outline: none;
}

input {
    font-size: large;
    border: 3px solid var(--blue);
    outline: none;
    margin-bottom: 15px;
}

input::placeholder {
    opacity: 0.6;
}
<!DOCTYPE html>

    <link rel="stylesheet" href="style.css">
    <link href="https://fonts.googleapis.com/css?family=Roboto&display=swap" rel="stylesheet">
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <title>Password Generator</title>

Password Generator

        <button id="generator">Generate!</button>
        <input type="text" id="textField" placeholder="Your password goes here...">

I could add a few things onto this, like having a way to copy onto the clipboard and such. But really what I want to know is, how can I make this code shorter and more efficient? Specifically, my CSS is long. Maybe I should start using a CSS framework? Let me know.

JavaScript

You don't have to list down all characters. All printable characters in the ASCII range are found in 33-126. What you could do is generate a random number within this range, and convert them into characters using String.fromCharCode().

Also, it's always a good practice to separate business logic from view logic. You will want to be able to generate random passwords regardless of what your UI implementation is. In this case, I recommend putting your logic into its own function, and have the event handler call it.

Store elements you get from document.getElementById() and similar APIs to a variable, and use that variable to operate on the element. DOM operations are slow, and you will want to reduce it if possible. This example is small, and the impact is negligible, but it's a good thing to keep in mind.

CSS

For your CSS, I discourage the use of element selectors, unless you're writing a globally applied style. The same goes for the * selector. The problem with using these selectors is that they apply styles globally and will affect not just your code, but code from other developers in your team.

Elements have default styles provided by the browser, and some developers expect this when styling their CSS. If I build a perfectly fine component, but your styles globally turn the font-size to 10px, that's not a nice developer experience.

Use classes instead to target specific elements. I recommend you learn the BEM naming scheme. This allows you to namespace your CSS so that it's contained only to the component/widget you're working on. In this case, your password generator.

Never do outline: none;. This is bad for accessibility and keyboard navigation. Also, do not remove visual cues for interactive elements as this makes your app look non-responsive. For example, the native styling of a button will make it look pressed when you click. If you replace the border color, you lose this. So you should provide an :active styling and style it to give feedback to the user that a press did happen.

Also not a fan of font: large as this may be different on different browsers. Use a fixed value using the rem unit. rems are based on the font-size of the  element. By default,  is 16px which means 16px * 1rem = 16px. 1.5rem is 16px * 1.5rem = 24px, and so on. The neat thing about rem is if the user has a bumped up or bumped down font-size, the your font scales with it as well.

const getRandomCharCode = (start, end) =>
  Math.floor(start + (Math.random() * ((end + 1) - start)))

// Generate an array that's length long, and fill it with random characters
// from 33 to 126 of the ascii range.
const generatePassword = length =>
  Array(length).fill()
  .map(_ => String.fromCharCode(getRandomCharCode(33, 126)))
  .join('')

const generatorButton = document.getElementById("generator")
const generatorTextField = document.getElementById("textField")

generatorButton.addEventListener("click", () => {
  generatorTextField.value = generatePassword(16)
})
:root {
  --gray: #CBC4C4;
  --blue: #029DF1;
  --blue: #029DF1;
  --dark-blue: #007DD1;
  --white: #FFF;
}

body {
  background: var(--blue);
  font-family: sans-serif;
}

.generator {
  width: 300px;
  margin: 0 auto;
  padding: 15px;
  background: var(--gray);
  border-radius: 30px;
  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
}

.generator__button {
  background: var(--blue);
  color: var(--white);
  padding: 15px;
  margin: 0 0 15px;
  font-size: 1.25rem;
  cursor: pointer;
  border: 2px solid var(--blue);
}

.generator__button:active {
  background: var(--dark-blue);
}

.generator__field {
  margin: 0 0 15px;
  font-size: 1.25rem;
  border: 3px solid var(--blue);
}

.generator__field::placeholder {
  opacity: 0.6;
}

Password Generator

  <button class="generator__button" id="generator">Generate!</button>
  <input class="generator__field" id="textField" type="text" placeholder="Your password goes here...">


TSQL query to gather amounts for an employees, spouses, and children in one row I have a query that works as intended, kind of pulling from multiple records in a table, and flattening the data into one row. I am wondering if there is a faster/more semantically correct way to do this than multiple derived tables?

ID      Name      Relationship      Amount
-------------------------------------------
1       James     Employee           2000
1       James     Spouse             5000
1       James     Child             10000
2       Mary      Employee           2000
3       Pete      Employee           2000
3       Pete      Child             10000
3       Pete      Child             10000

And turning it into:

ID      Name      EmployeeAmt      SpouseAmt      ChildAmt
----------------------------------------------------------------------------
1       James     2000             5000           10000
2       Mary      2000             NULL           NULL
3       Pete      2000             NULL           10000

Here is the query:

CREATE TABLE #Source
(
   EmpID varchar(2) NULL
   ,Name varchar(20) NULL
   ,Relationship varchar(20) NULL
   ,Amount varchar(10) NULL
)

INSERT INTO #Source
VALUES( '1', 'James', 'Child', 10000)
     ,('1', 'James', 'Spouse', 5000)
     ,('1', 'James', 'EE', 2000)
     ,('2', 'Mary', 'EE', 2000)
     ,('3', 'Pete', 'EE', 2000)
     ,('3', 'Pete', 'Child', 10000)
     ,('3', 'Pete', 'Child', 10000)

SELECT DISTINCT A.EmpID, A.Name, A.EEAmount, B.SpouseAmount, C.ChildAmount
FROM (
          SELECT EmpID, Name, Relationship, Amount AS 'EEAmount'
          FROM #Source
          WHERE Relationship = 'EE'
     ) AS A

     LEFT JOIN

    (
          SELECT EmpID, Name, Relationship, Amount AS 'SpouseAmount'
          FROM #Source
          WHERE Relationship = 'Spouse'
    ) AS B ON A.EmpID = B.EmpID

    LEFT JOIN

    (
          SELECT EmpID, Name, Relationship, Amount AS 'ChildAmount'
          FROM #Source
          WHERE Relationship = 'Child'
    ) AS C ON A.EmpID = C.EmpID

SELECT * FROM #Source
DROP TABLE #Source

You can use T-SQL's operator PIVOT to do this.

SELECT EmpID, Name, [EE], [Spouse], [Child]
FROM
(
    SELECT EmpID, Name, Relationship, CAST(Amount AS INT) AS AmountINT
    FROM Source
) AS SourceTable
PIVOT
(
    MAX(AmountINT)
    --
    -- Define your target relationships here
    -- EE, Spouse, Child ...
    -- These relationships should be exactly the same as described in
    -- the SELECT statement
    --
    FOR Relationship IN ([EE], [Spouse], [Child])
) AS PivotTable;

Result

EmpID   Name    EE      Spouse  Child
1       James   2000    5000    10000
2       Mary    2000    NULL    NULL
3       Pete    2000    NULL    10000

You need to cast your Amount into numeric type before doing so.


Implementation of UNIX `tee` in Rust I am currently reading Michael Kerrisk's book, The Linux Programming Interface. This is one of the exercises from the end of chapter 4 (I'm working in Rust, not C).

A Rust implementation of the tee program on UNIX systems. According to it's man page

  The tee utility copies standard input to standard output, making a copy in zero or more files.  The output is unbuffered.

So running echo "foo" | tee will print "foo" to the terminal.
running echo "foo" | tee myfile will print "foo" to the terminal and also create the file myfile with the contents "foo".

It's a small example but I'd love feedback on whether this is idiomatic Rust code and whether the efficiency could be improved.

use std::env;
use std::io::{self, Read};
use std::io::prelude::Write;
use std::error::Error;
use std::fs::File;

fn main() {
    let fname = match env::args().nth(1) {
        Some(name) => name,
        None => String::from("/dev/null"),
    };

    let mut outfile = match File::create(&fname) {
        Ok(file) => file,
        Err(why) => panic!("{}", why.description()),
    };

    loop {
        let mut line = String::new();
        match io::stdin().read_to_string(&mut line) {
            Ok(_) => {}
            Err(_) => std::process::exit(0),
        }

        if line.is_empty() {
            std::process::exit(0);
        }

        println!("{}", &line);

        let _ = outfile.write(line.as_bytes());
    }
}

High-level issues:

  making a copy in zero or more files

It appears the code only handles zero or one file.

  The output is unbuffered.

The code is very much buffering the input. Check out read_to_string:

  Read all bytes until EOF in this source, placing them into buf.

That means that the program has to wait for the entire input string to exist before it writes a single byte. That input could be gigabytes!
The code assumes that all input is a UTF-8 encoded string. This cat cannot be used with images or zipfiles any any kind of binary data.

More tactical issues:

Don't use single items from a prelude. The point of a prelude is to be a glob import of all the things you might want.
Check out all the methods that exist on Option or Result. For example, unwrap_or_else encapsulates the default value case.
Hardcoding /dev/null ties the implementation to a specific OS (maybe OK because you are making cat) but also to a filesystem layout. This cat can't be used when /dev/ isn't available, which would likely be during a nasty system emergency. Would be better for the program to not write anything instead of writing to /dev/null.
Printing out the errors description shows less information than printing out the error:

  other os error

vs.

  Is a directory (os error 21)

expect may be more compact than a match or unwrap_or_else, depending on the desired error format.
I don't believe the loop will ever loop, since the entire input is read into the string as discussed above.
If the loop did loop, reallocating the entire string for each iteration is inefficient. Better to just clear it and reuse the memory.
There's no need to pass a reference when using println!; that macro automatically references all the arguments.
Don't ignore errors! let _ is a really big red flag. cat has few responsibilities, but writing output correctly is fairly critical! The program should exit when writing fails, such as when a pipe is closed.
The exit code isn't well defined in all cases. What is it when panicking?
Standard in and out would be repeatedly locked in each iteration. Not as efficient as a single lock.

use std::env;
use std::io::{self, Read, Write};
use std::fs::File;

fn main() {
    let fname = env::args().nth(1).unwrap_or_else(|| String::from("/dev/null"));

    let mut outfile = File::create(&fname).expect("Unable to create file");
    let mut line = String::new();

    loop {
        line.clear();

        io::stdin().read_to_string(&mut line).unwrap_or_else(|_| {
            std::process::exit(0)
        });

        if line.is_empty() {
            std::process::exit(0);
        }

        println!("{}", line);

        outfile.write(line.as_bytes()).expect("Unable to write!");
    }
}

Sorry for going a bit off the deep end, but the problem was interesting. I'd probably write it a little like this:

use std::env;
use std::io::{self, Read, Write};
use std::fs::File;

const BUFFER_SIZE: usize = 8 * 1024;

fn main() {
    let mut buf = vec![0; BUFFER_SIZE];

    let stdin = io::stdin();
    let mut stdin = stdin.lock();

    let stdout = io::stdout();
    let stdout = stdout.lock();

    let mut outputs: Vec<_> = env::args().skip(1).map(|fname| {
        let f = File::create(&fname).unwrap_or_else(|e| {
            panic!("Unable to create file {}: {}", fname, e);
        });

        Box::new(f) as Box<Write>
    }).collect();

    outputs.push(Box::new(stdout));

    loop {
        let bytes = stdin.read(&mut buf).expect("Unable to read input");

        if bytes == 0 { break }

        // **Very important**, otherwise you can end up with
        // Heartbleed-esque bugs! I'm chosing to shadow `buf` to
        // deliberately prevent using it again in this loop.
        let buf = &buf[..bytes];

        for output in &mut outputs {
            output.write_all(buf).expect("Unable to write output");
        }
    }
}


Xorgens4096 PRNG with global seeder in C# This is a basic implementation of the non-cryptographic Xorgens4096 pseudo random number generator with a global, thread safe seeder. The goal is to provide a high quality generator that can be instantiated in multiple threads while still guaranteeing that the seed for each generator is unique. To ensure uniqueness of each generator's state, SHA512 is used in feedback mode (if that's the right word).

Is this the right approach? Any criticism and tips are very much appreciated.

using System;
using System.Diagnostics;
using System.Text;
using System.Security.Cryptography;

// big state non-cryptographic PRNG with a global seeder

class Xorgens4096
{
    // seeder part

    static private SHA512 sha512;

    static public byte[] hash;

    // set master seed using some common data for uniqueness

    static Xorgens4096()
    {
        sha512 = SHA512.Create();

        SetMasterSeed(String.Format("{0}-{1}-{2}",
            DateTime.UtcNow.Ticks,
            Process.GetCurrentProcess().Id,
            Guid.NewGuid()));
    }

    // set master seed used to derive states of all generator instances

    static public void SetMasterSeed(string seed)
    {
        hash = Encoding.Unicode.GetBytes(seed);
    }

    // create generator state using a seed or master seed (thread safe)

    static private void Seed(string seed, ulong[] state)
    {
        lock (hash)
        {
            if (String.IsNullOrEmpty(seed) == false)
                hash = Encoding.Unicode.GetBytes(seed);

            for (int i = 0; i < 8; i++)
            {
                hash = sha512.ComputeHash(hash);
                Buffer.BlockCopy(hash, 0, state, i * 64, 64);
            }
        }
    }

    // generator part

    public ulong[] state = new ulong[64];
    public ulong index;
    public ulong weyl;

    public Xorgens4096(string seed = "")
    {
        Seed(seed);
    }

    public void Seed(string seed)
    {
        Seed(seed, state);
        index = 0;
        weyl = 0;
    }

    // actual xorgens4096 generator

    public ulong Gen()
    {
        ulong t = state[index = (index + 1) & 63];
        t ^= t << 33;
        t ^= t >> 26;

        ulong v = state[(index + 11) & 63];
        v ^= v << 27;
        v ^= v >> 29;

        state[index] = (v ^= t);

        weyl += 0x61c8864680b583eb;

        return v + (weyl ^ (weyl >> 27));
    }
}

The only thing I see that needs correcting is the locking.  Remember that

lock (hash) {
    // body
}

is syntactic sugar for:

var local = hash;
try
{
    System.Threading.Monitor.Enter(local);
    // body
}
finally
{
    System.Threading.Monitor.Exit(local);
}

When you update the hash variable in one thread, the next thread to come in will have a different object that it locks, and so the blocks will execute in parallel.  It's usually a mistake to try to reuse part of your model as a lock.  I always recommend a dedicated object like

static readonly Object _lock = new Object();

Other than that, I only suggest:

if (String.IsNullOrEmpty(seed) == false)

should be:

if (!String.IsNullOrEmpty(seed))


Converting a hexadecimal string to a binary string using Rust pattern matching lookup I am getting started in Rust, and as a part of my first exercise, I decided to write a small program that converts a hexadecimal string to a binary string using a pattern matched lookup. I would appreciate a constructive review.

fn main() {
    let binary_value = convert_to_binary_from_hex("0x39A7F8");
    println!("Converted: {}", binary_value);
}

fn convert_to_binary_from_hex(hex: &str) -> String {
   let to_binary = hex[2 ..]
       .chars()
       .map(|c| to_binary(c))
       .collect();

   to_binary
}

fn to_binary(c: char) -> String {
    let b = match c {
       '0' => "0000",
       '1' => "0001",
       '2' => "0010",
       '3' => "0011",
       '4' => "0100",
       '5' => "0101",
       '6' => "0110",
       '7' => "0111",
       '8' => "1000",
       '9' => "1001",
       'A' => "1010",
       'B' => "1011",
       'C' => "1100",
       'D' => "1101",
       'E' => "1110",
       'F' => "1111",
        _  => "",
    };

   b.to_string()
}

Rustfmt is a tool for automatically formatting Rust code to the community-accepted style.
Clippy is a tool for finding common mistakes that may not be compilation errors but are unlikely to be what the programmer intended.

Rustfmt points out that you are using 3-space indents (Rust uses 4), and that some of your lines don't need to be split.

Clippy points out:

warning: returning the result of a let binding from a block
 --> src/main.rs:9:5
  |
7 |     let to_binary = hex[2..].chars().map(|c| to_binary(c)).collect();
  |     ----------------------------------------------------------------- unnecessary let binding
8 |
9 |     to_binary
  |     ^^^^^^^^^
  |
  = note: #[warn(clippy::let_and_return)] on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
  |
7 |
8 |
9 |     hex[2..].chars().map(|c| to_binary(c)).collect()
  |

warning: redundant closure found
 --> src/main.rs:7:42
  |
7 |     let to_binary = hex[2..].chars().map(|c| to_binary(c)).collect();
  |                                          ^^^^^^^^^^^^^^^^ help: remove closure as shown: `to_binary`
  |
  = note: #[warn(clippy::redundant_closure)] on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

There's no reason to return a String here, a &'static str is lighter-weight. See What are the differences between Rust's String and str? for full details, but a String requires a heap allocation, while a &'static str is a reference to some existing data in the compiled binary.

With these changes, all of your tests pass:

fn main() {
    let binary_value = convert_to_binary_from_hex("0x39A7F8");
    println!("Converted: {}", binary_value);
}

fn convert_to_binary_from_hex(hex: &str) -> String {
    hex[2..].chars().map(to_binary).collect()
}

fn to_binary(c: char) -> &'static str {
    match c {
        '0' => "0000",
        '1' => "0001",
        '2' => "0010",
        '3' => "0011",
        '4' => "0100",
        '5' => "0101",
        '6' => "0110",
        '7' => "0111",
        '8' => "1000",
        '9' => "1001",
        'A' => "1010",
        'B' => "1011",
        'C' => "1100",
        'D' => "1101",
        'E' => "1110",
        'F' => "1111",
        _ => "",
    }
}

In fact, deleting all of your code causes your tests to pass. It would be a good idea to add some tests!

Depending on what routes you want to look down next, you could try:

to handle strings that do not start with 0x without panicking.
to handle upper- and lower-case hex
to reduce/avoid string munging yourself


Reverse part of a linked list I'm  learning data structures. I'm working on linked lists at the moment, and I'd like to have a strong grasp on them. Below is the last problem I solved. I'd like to know what you think of the solution and what can be done better.

The problem consists of reversing a linked list from position m to position n.
1 ≤ m ≤ n ≤ length of list. If we have a list 1->2->3->4->5->NULL, m = 2 and n = 4, the function should return 1->4->3->2->5->NULL.

   /**
 * Definition for singly-linked list.
 * struct ListNode {
 *     int val;
 *     struct ListNode *next;
 * };
 */
struct ListNode* reverseBetween(struct ListNode* head, int m, int n) {
    if (head == NULL || head->next == NULL || m - n == 0)
    {
        return head;
    }
    struct ListNode* prev = NULL, *cur = head;
    struct ListNode* before_m_node, *m_node, *after_n_node;
    int count = 0;

    // find m node, after loop cur points to m node, prev gets node befor or  NULL if there is none
    while (count != m)
    {
        ++count;
        if (count == m)
        {
            break;
        }
        prev = cur, cur = cur->next;
    }
    m_node = cur;
    before_m_node = prev;
    prev = cur, cur = cur->next;

    // now we look for n node, cur is currently pointing to node after m, prev points to node before
    // we will reverse links as we go and after loop cur will point to n node
    while (count != n)
    {
        ++count;
        if (count == n)
        {
            break;
        }
        struct ListNode* temp = cur->next;
        cur->next = prev;
        prev = cur, cur = temp;
    }

    after_n_node = cur->next;

    // if first node is not head of the list
    if (before_m_node != NULL)
    {
        cur->next = prev;
        m_node->next = after_n_node;
        before_m_node->next = cur;
        return head;
    }
    else
    {
        cur->next = prev;
        m_node->next = after_n_node;
        return cur;
    }
}

Redundancy in while loops

Your while loop has a condition that never gets used because there is an if statement inside the loop that actually ends the loop.  For example:

while (count != m)
{
    ++count;
    if (count == m)
    {
        break;
    }
    prev = cur, cur = cur->next;
}

You could simplify this by merging the increment into the loop condition.  Also, I don't using the comma operator to put two statements on one line.  So I would write your loop like this:

while (++count < m)
{
    prev = cur;
    cur  = cur->next;
}

And your other loop would look like this:

while (++count < n)
{
    struct ListNode* temp = cur->next;
    cur->next = prev;
    prev      = cur;
    cur       = temp;
}

Use typedef

I suggest using a typedef so you don't have to use struct so much.  For example:

typedef struct ListNode {
     int val;
     struct ListNode *next;
} ListNode;

And then use just ListNode instead of struct ListNode everywhere.


Optimize Node.js function I have the follow function:

Download.prototype.fundamentals = function(exchanges, cb){

    if (!(exchange = exchanges.pop())) return

    var that = this;

    this.login(function(err){

        if (err) return cb(true, exchange.code)

        var path = util.format(that.eoddata.fundamentals['path'], that.token, exchange.code)

        that.get(that.eoddata.fundamentals['host'], path, function(err, data){
            if (err) return cb(true, exchange.code)

            that.xml(data, function(err, data){
                if (err || data['@'].Message != 'Success') return cb(true, exchange.code)

                var stocks = data['FUNDAMENTALS']['FUNDAMENTAL']

                that.insert(stocks, exchange, function(){
                    cb(false, exchange.code)
                })

                that.fundamentals(exchanges, cb)
            })

        })

    })
}

I think it easy to understand, I pop() an element to a list and then do other operations, and then recall the function to pop() another element.

I have a doubt regarding the writing of the function, now it works, but it does not seem very optimized. Example: I do not like var that = this.

How could I rewrite it better?

Firstly, you are missing a var exchange; in your function.

Heavily nested callbacks are no fun for anyone. I would consider using the async library to try to simplify the callbacks.

https://github.com/caolan/async#waterfall

Something like this. The callbacks passed to waterfall are called in series, and any arguments passed to each callback function after the err argument, are passed as the first parameters of the next function.

var that = this is just something you kind of have to get used to. In some cases you can avoid it by using various methods to bind the value of 'this' to the proper value.

Download.prototype.fundamentals = function(exchanges, cb) {

  var that = this,
      exchange = exchanges.pop();

  if (!exchange) return;

  async.waterfall([
    function(callback) {
      that.login(callback);
    },
    function(callback) {
      var path = util.format(that.eoddata.fundamentals['path'], that.token, exchange.code)
      that.get(that.eoddata.fundamentals['host'], path, callback);
    },
    function(data, callback) {
      that.xml(data, function(err, xmldata) {
        err = err || xmldata['@'].Message != 'Success';
        callback(err, xmldata);
      });
    },
    function(xmldata, callback) {
      var stocks = xmldata['FUNDAMENTALS']['FUNDAMENTAL']
      that.insert(stocks, exchange, callback);
      that.fundamentals(exchanges, cb);
    }
  ], function(err) {
    cb(err, exchange.code);
  });
}


Written numbers to numerical representation So I took a stab at this task to begin learning common lisp.

The idea is that you give it a written representation of a number such as "three thousand and forty nine" and it will output 3049.

I was looking for some input on my lisp and where I'm being daft or even what parts are good.

So without further a due, my code:

(defmacro do-push (result value)
 `(if (not (eq ,value 0))
   (progn (setf ,result (append ,result (cons ,value nil)))
      (setf ,value 0))))

(defun should-push (string)
 (or (equalp string "and")) (not (should-multiply string)))

(defmacro sum-list (L)
 `(reduce '+ ,L))

(defun get-input ()
 (princ "Please enter a number: ")
 (read-line))

(defun tokenize (string)
 (loop for i = 0 then (incf j)
       as j = (position #\Space string :start i)
       collect (subseq string i j)
       while j))

(defun should-multiply (string)
 (or (equalp string "hundred") (equalp string "thousand") (equalp string "million")))

(defun get-needed-operation (string)
 (cond ((should-multiply string) (symbol-function '*))
       ((equalp string "and") (symbol-function '+))
       (t nil)))

(defun string-value (string)
 (cond ((equalp string "one") 1)
       ((equalp string "two") 2)
       ((equalp string "three") 3)
       ((equalp string "four") 4)
       ((equalp string "five") 5)
       ((equalp string "six") 6)
       ((equalp string "seven") 7)
       ((equalp string "eight") 8)
       ((equalp string "nine") 9)
       ((equalp string "ten") 10)
       ((equalp string "eleven") 11)
       ((equalp string "twelve") 12)
       ((equalp string "thirteen") 13)
       ((equalp string "fourteen") 14)
       ((equalp string "fifteen") 15)
       ((equalp string "sixteen") 16)
       ((equalp string "seventeen") 17)
       ((equalp string "eighteen") 18)
       ((equalp string "nineteen") 19)
       ((equalp string "twenty") 20)
       ((equalp string "thirty") 30)
       ((equalp string "forty") 40)
       ((equalp string "fifty") 50)
       ((equalp string "sixty") 60)
       ((equalp string "seventy") 70)
       ((equalp string "eighty") 80)
       ((equalp string "ninety") 90)
       ((equalp string "hundred") 100)
       ((equalp string "thousand") 1000)
       ((equalp string "million") 1000000)))

(let ((result '())
      (current 0))
 (loop for word in (tokenize "one million three hundred thousand eighteen thousand six hundred and seventy three") do
  (let ((operation (get-needed-operation word))
        (value (string-value word)))
   (progn (when (and (eq value nil) (eq operation nil))
       (format t "Unexpected token: ~a~c" word #\newline))

      (if (eq operation nil) (setq operation (symbol-function '+)))

      (when (should-push word)
       (do-push result current))

      (when (not (eq value nil))
       (setq current (funcall operation value current)))))
       finally (do-push result current))
 (format t "Result: ~a" (sum-list result)))

My logic is to have a list that contains all the detected "sub-numbers" which are just added together to get the final number.

Certain words (hundred, thousand, etc) imply a multiply operation to the current number such that three hundred becomes 3 * 100 = 300

Certain words (and, one, two, etc) imply an addition operation such that sixty seven becomes 60 + 7 = 67

Some words cause the current number to be pushed to the result list, resetting the current number.

There are a couple of issues with your code. Let me describe a few:

numbers are not EQ, use EQL or =.
Common Lisp has documentation strings. Use them.
if not progn can be replaced with a single unless

The next function does not work as expected. I have inserted a newline and you see that the parentheses are wrong.

(defun should-push (string)
 (or (equalp string "and"))
 (not (should-multiply string)))

Don't write trivial macros. If you want to have a name for a trivial function, then use a function - not a macro. If the calling overhead bothers you, then declare it inline.

(defmacro sum-list (L)
  `(reduce '+ ,L))

The next may or may not work as you think. Since Common Lisp output streams can be buffered, call FORCE-OUTPUT when you actually want to see the output.

(defun get-input ()
  (princ "Please enter a number: ")
  (read-line))

or equalp can be replaced with (member foo '("a" "b" "c") :test #'equalp). Get the data out of the logic.
instead of (symbol-function '+) use #'+
instead of COND EQUALP use ASSOC with the :test keyword. Get the data out of the code. Alternatively use a macro similar to CASE, which works for strings.
for the last form, write a function, too. Always provide functions. Don't print the result. Just return the result.
inside a LET you don't need a PROGN
EQ nil is better replaced with the NULL predicate.
format t "~c" #\newline is just format t "~%"
when not is unless


Counting valleys traversed below sea level, given elevation changes Problem Statement:

  Gary is an avid hiker. He tracks his hikes meticulously, paying close
  attention to small details like topography. During his last hike he
  took exactly steps. For every step he took, he noted if it was an
  uphill, or a downhill,  step. Gary's hikes start and end at sea
  level and each step up or down represents a  unit change in altitude.
  We define the following terms:

  A mountain is a sequence of consecutive steps above sea level,
  starting with a step up from sea level and ending with a step down to
  sea level. A valley is a sequence of consecutive steps below sea
  level, starting with a step down from sea level and ending with a step
  up to sea level. Given Gary's sequence of up and down steps during his
  last hike, find and print the number of valleys he walked through.

  For example, if Gary's path is, he first enters a valley units deep.
  Then he climbs out an up onto a mountain units high. Finally, he
  returns to sea level and ends his hike.

  Function Description

  Complete the countingValleys function in the editor below. It must
  return an integer that denotes the number of valleys Gary traversed.

  countingValleys has the following parameter(s):

  n: the number of steps Gary takes
  s: a string describing his path

  Input Format

  The first line contains an integer, the number of steps in Gary's
  hike.  The second line contains a single string, of characters that
  describe his path.

  Output Format

  Print a single integer that denotes the number of
  valleys Gary walked through during his hike.

My solution goes like this :

def countingValleys(n: Int, s: String): Int = {
  def rec(path: List[Char], counter: Int, valleys: Int): Int = {
    path match  {
      case Nil => valleys
      case x::xs if (x == 'U') && ((counter + 1) == 0) =>
        rec(xs, counter+1, valleys+1 )
      case x::xs if x == 'U' => rec(xs, counter +1, valleys)
      case x::xs if x == 'D' => rec(xs, counter -1, valleys)
    }
  }
  rec(s.toList, 0, 0)
}

Sample Input:

8
UDDDUDUU

Sample Output:

1

I wonder if there is a more idiomatic solution in functional programming style.

There's nothing wrong with or "not functional" about your approach. This might be opinion territory, but personally I would consider using foldLeft as it might be more readable and you can potentially eliminate some of the complexity of your cases.

(also I got rid of n here as it is completely irrelevant, I know it's a problem from a website so you have to follow the format they gave you)

def countingValleys (steps: String): Int = steps
  .foldLeft((0, 0)) {
    case ((valleys, elevation), 'D') => (valleys, elevation - 1)
    case ((valleys, -1), 'U') => (valleys + 1, 0)
    case ((valleys, elevation), _) => (valleys, elevation + 1)
  }
  ._1


C# to Access SQL query based on UI controls I have a WPF application (using MVVM pattern) that allows users to perform a search for a record/records based on their selection choices and entries into the different UI controls (TextBox, CheckBox, RadioButton, DatePicker, TimerPicker,etc).  It does this by retrieving information from an Access database.

I have an Access query method in my AccessWorker class that looks like this (note that this is simplified a bit, because I can't post everything I have):

public DataSet GetRecords(string customQuery = "")
{
    var data = new DataSet();

    try
    {
        using (var con = new OleDbConnection(ConnectionString))
        {
            string queryString = "SELECT * FROM Records " + customQuery;

            using (var da = new OleDbDataAdapter(queryString, con))
            {
                da.Fill(data, "Records");
            }
        }
    }
    catch (Exception e)
    {
        ExceptionLogger.Record(e.Message);
    }

    return data;
}

As you can see, I pass a custom query argument to this method in a form of a string.  I construct this query string using a StringBuilder in my ViewModel and then I pass it to GetRecords() method.  Here's the GetQueryString() method that does the string building:

private string GetQueryString()
{
    var query = new StringBuilder();

    if (!String.IsNullOrWhiteSpace(RecNumber))
    {
        query.Append("Record_Number = '" + RecNumber + "'");
    }

    if (!String.IsNullOrWhiteSpace(EmployeeId))
    {
        if (!String.IsNullOrWhiteSpace(query.ToString()))
        {
            query.Append(" AND ");
        }

        query.Append("Employee_ID LIKE '%" + EmployeeId + "%'");
    }

    if (!String.IsNullOrWhiteSpace(Location))
    {
        if (!String.IsNullOrWhiteSpace(query.ToString()))
        {
            query.Append(" AND ");
        }

        query.Append("Location LIKE '%" + Location + "%'");
    }

    if (!String.IsNullOrWhiteSpace(FirstName))
    {
        if (!String.IsNullOrWhiteSpace(query.ToString()))
        {
            query.Append(" AND ");
        }

        query.Append("First_Name LIKE '%" + FirstName + "%'");
    }

    if (!String.IsNullOrWhiteSpace(LastName))
    {
        if (!String.IsNullOrWhiteSpace(query.ToString()))
        {
            query.Append(" AND ");
        }

        query.Append("Last_Name LIKE '%" + LastName + "%'");
    }

    //...and so on...
    return String.Format("WHERE {0}", query.ToString());
}

The above method is severely shortened here so that I wouldn't have to post a very long wall of code, but I'm dealing with 25 different controls and so there could be a condition for everyone of them that would have to be part of this query string.  The method itself, as you can imagine, is rather too large by anyone's standard.  When I took a C++ class in college, my instructor told me that if I can help it, I should keep my methods no longer than what I can see on the screen.  I'm bowing my head in shame... heh

I don't believe that this is the most elegant way of accomplishing this task.  Unfortunately, I haven't come up with a better approach.  I do my best at trying to learn new patterns every day, but I've been programming for under a year and a half, so any guidance would be greatly appreciated.

By the way, based on some of the comments, I have to clear up that I'm using properties that are bound to control, not controls' values....

My initial thought is that since view models are only (or should only be) concerned with presentation logic, it shouldn't know how to construct an SQL string. I would move the code which builds up the query into a separate Query Object. Also I would have some helper methods (e.g. extension methods) to try and get rid of the repetitive null/empty/whitespace checks. For example:

public class GetRecordsQuery
{
    // If there's a lot of parameters, maybe refactor this to accept an object
    public void Execute(string recordNumber, string employeeId, string location,
                        string firstName, string lastName)
    {
        var query = new StringBuilder();

        query
            .Eq("Record_Number", recordNumber).And()
            .Like("Employee_ID", employeeId).And()
            .Like("Location", location).And()
            .Like("First_Name", firstName).And()
            .Like("Last_Name", lastName)
            .EndQuery();

        // or however AccessWorker members are invoked
        return AccessWorker.GetRecords(String.Format("WHERE {0}", query));
    }
}

public static class StringBuilderExtensions
{
    private const string AndOp = " AND ";

    public static StringBuilder And(this StringBuilder sb)
    {
        var str = sb.ToString();

        if (String.IsNullOrWhiteSpace(str))
            return sb;

        if (str.EndsWith(AndOp))
            return sb;

        return sb.Append(AndOp);
    }

    public static StringBuilder EndQuery(this StringBuilder sb)
    {
        var str = sb.ToString();

        if (String.IsNullOrWhiteSpace(str))
            return sb;

        if (!str.EndsWith(AndOp))
            return sb;

        var length = AndOp.Length;
        var start = sb.Length - length;

        return sb.Remove(start, length);
    }

    public static StringBuilder Eq(this StringBuilder sb, string column, string value)
    {
        if (String.IsNullOrWhiteSpace(value))
            return sb;

        return sb.AppendFormat("{0} = '{1}'", column, value);
    }

    public static StringBuilder Like(this StringBuilder sb, string column, string value)
    {
        if (String.IsNullOrWhiteSpace(value))
            return sb;

        return sb.AppendFormat("{0} LIKE '%{1}%'", column, value);
    }
}

EDIT 1: Corrected code to prevent AND operators being appended when one already exists.

EDIT 2: Added an EndQuery method to trim off any trailing AND operators.


PHP Action Verification I am creating a site that is very heavily relying on Ajax. To stop major spamming on my site, I decided to implement a PHP system that checks how many actions have been made in the last 5 minutes. I wrote this fairly quickly and I was wondering if there are any major issues with my code. Read the footnote as to why I can't test this. If you see any major errors here, please let me know. Also, spelling errors would be the fault of auto-correct.

<?php
    session_start();
    require_once(".conf.php");
    //Main config
    $verification = $_GET['verification_token'];
    $requestactions = $_GET['request_actions'];
    $username = $_SESSION['username'];
    $login = $_SESSION['login'];
    $ip = $_SERVER['SERVER_ADDR'];
    //Set up config
    $maxactions = 40;
    $maxactionserror = 'Hmm, it seems you exceded the max actions allowed here. Try again in a few, Thank you!';
    $actiontable = 'actions'
    $currentactions = 0;
    $timeout = 1;
    $cookieexpire = 600 //Time in MS
    $blockreason = 'exceeded action requests';
    //Query config
    $firstquery = mysql_query("SELECT * FROM '$actiontable' WHERE username = '$username'");
    $rows = mysql_num_rows($firstquery);

    function check() {
        if ($rows) {
            if ($rows >= $maxactions) {
                setcookie['reasonblocked', $blockreason, time() + $cookieexpire);
                setcookie['ipblocked', $ip, time() + $cookieexpire);
                setcookie['blockfinished', 'timestamp', time() + $cookieexpire);
                $blockquery = mysql_query("INSERT INTO '$actiontable'('eligible') VALUES ('0') WHERE 'username' = '$username'");
                echo $maxactionserror;
                unset($login);
                unset($username);
                session_destroy();
            } else {
                while ($get = mysql_fetch_array($firstquery)) {
                    $currentactions = $get['cactions'];
                    $newactions = $currentactions++;
                    $secondquery = mysql_query("INSERT INTO '$actiontable'('username', 'auth_token', 'cactions', 'userip') VALUES ('$username', '$verification', '$newactions', '$ip')");
                    echo 'success';
                    check();
                }
            }
        } else {
            $secondquery = mysql_query("INSERT INTO '$actiontable'('username', 'auth_token', 'cactions') VALUES ('$username', '$verification', '1')");
            echo 'success';
        }
    }
?>

PS. I am on vacation so I am unable to test this. That is why I posted on here.

Okay, so the first thing here: prevent yourself from SQL injections. Of course you can escape your parameters but parametrized queries with PDO or so would be much better. You can also use mysqli - here's an example:

Also this won't execute since you have syntax errors in there (ie a missing ; here:  $cookieexpire = 600 //Time in MS).

Also, don't rely on cookies for this. If someone really wants to spam, it is really easy just to delete your cookies or just return them in a valid state... store the ipaddress (or ipaddr and the browser-string) and then save that in a database (or something like redis).

There is much more to talk about here, but this would be a good start.


Rust Simple Stats Program I am brand new to Rust and want to know if there are any convention violations in my code and ways how I can make it more readable:
use std::io;

fn main() {
    let mut vec: Vec<i32> = Vec::new();
    loop {
        println!("Enter any number, 0 to terminate");
        let mut num = String::new();
        io::stdin()
            .read_line(&mut num);
        if num.trim().parse::<i32>().unwrap() == 0 {break;}
        vec.push(num.trim().parse::<i32>().unwrap());
    }
    let sum: i32 = vec.iter().sum();
    println!("Sum: {}", sum);
    println!("Min: {}", vec.iter().min().unwrap());
    println!("Max: {}", vec.iter().max().unwrap());
    println!("Mean: {}", sum as f32 / vec.len() as f32);
}

io::stdin().read_line(&mut num);

Here you are ignoring the Result from read_line; you should have seen a compiler warning about this. Add .unwrap() or .expect() to check the error and panic if there is a problem reading:
io::stdin().read_line(&mut num).expect("Read error");

(Below I'll be talking about handling errors instead of panicking. This case is different because an IO error during reading shouldn't occur in normal usage and there isn't really any way to recover from it.)

        if num.trim().parse::<i32>().unwrap() == 0 {break;}
        vec.push(num.trim().parse::<i32>().unwrap());

In these two lines you are parsing the number twice. Put the parse result in a variable instead.
        let num = num.trim().parse::<i32>().unwrap();
        if num == 0 {break;}
        vec.push(num);

If the user makes a typo and enters something that does not parse as i32 (e.g. a letter) then the program will panic since you are .unwrap()ing the parse result. It probably makes sense to tell the user and let them reenter instead. We can combine that with the handling of 0 inputs:
        match num.trim().parse::<i32>() {
            Err(_) => {
                println!("{:?} wasn't an integer. Try again.", num);
            }
            Ok(0) => {
                break;
            }
            Ok(x) => {
                vec.push(x);
            }
        }

Finally, you used .unwrap() for the min and max, which means that it will panic if the user doesn't enter any numbers. It's generally better for a program to not panic on any input. We can eliminate the unwrap from a single print like this:
    if let Some(min) = vec.iter().min() {
        println!("Min: {}", min);
    }

But, since three of the four outputs don't make sense for zero items, I think it makes more sense to put them inside a single condition, even if that means keeping the unwraps:
    println!("Sum: {}", sum);
    if !vec.is_empty() {
        println!("Min: {}", vec.iter().min().unwrap());
        println!("Max: {}", vec.iter().max().unwrap());
        println!("Mean: {}", sum as f32 / vec.len() as f32);
    }

One more thing. If the input to the program reaches end-of-file without containing 0, it will loop infinitely, reading no characters and producing error messages. (Well, your initial version didn't, because it panicked on a parse error instead). The right thing to do is to check whether read_line met end of file; this is reported in the return value but also detectable by the fact that num will be the empty string (without any newline).
        io::stdin().read_line(&mut num).unwrap();
        if num == "" {
            break;
        }

Putting together all my suggested changes:
use std::io;

fn main() {
    let mut vec: Vec<i32> = Vec::new();
    loop {
        println!("Enter any number, 0 to terminate");
        let mut num = String::new();
        io::stdin().read_line(&mut num).unwrap();
        if num == "" {
            break;
        }
        match num.trim().parse::<i32>() {
            Err(_) => {
                println!("{:?} wasn't an integer. Try again.", num);
            }
            Ok(0) => {
                break;
            }
            Ok(x) => {
                vec.push(x);
            }
        }
    }
    let sum: i32 = vec.iter().sum();
    println!("Sum: {}", sum);
    if !vec.is_empty() {
        println!("Min: {}", vec.iter().min().unwrap());
        println!("Max: {}", vec.iter().max().unwrap());
        println!("Mean: {}", sum as f32 / vec.len() as f32);
    }
}


Checking for Prime Numbers def is_prime(n):
    if n % 2 == 0 and n > 2:
        return False
    return all(n % i for i in xrange(3, int(math.sqrt(n)) + 1, 2))

The code works fine for a range of numbers after that it shows:

OverflowError: Python int too large to convert to C long

How can my code be made faster and more efficient?

On a 64-bit CPython 2.x,

all(i for i in xrange(2**63 - 1))

works, but

all(i for i in xrange(2**63))

overflows.  See this explanation, and a workaround.  I suggest rewriting this using an old-fashioned while loop.

With Python 3.x,

all(i for i in range(2**63))

works.

You've implemented trial division.  A more efficient algorithm is the sieve-of-eratosthenes, especially if you want to test multiple numbers for primality.  However, the Sieve of Eratosthenes requires more memory, so you'll have to resort to some hacks to make it work for large numbers.

For truly large numbers, you'll have to implement more sophisticated algorithms.


Wanted a cat. Got lolcat Wanna learn Rust? Ya rly!

I can haz FizzBuzz? No wai!

Gimme /bin/cat then.  But rustc say LOL! Must handle teh err0rz!

Now iz all err0r handlurz, so /bin/lolcat.  Rust sux, or OP sux?

Plain English translation:

Wanted to learn Rust, but didn't want to write FizzBuzz. How about writing /bin/cat? But for every function call, Rust insists that you do something with the Result, which may be an error. Simple task turned out to be complicated by all the error handling. What went wrong?

// Using rustc 1.0.0-beta.3
use std::env;
use std::io::Read;
use std::io::Write;
use std::fs::File;
use std::path::Path;
use std::process;

macro_rules! println_stderr(
    ($($arg:tt)*) => (
        match writeln!(&mut ::std::io::stderr(), $($arg)* ) {
            Err(e) => panic!("Unable to write to stderr: {}", e),
            Ok(_) => {},
        }
    )
);

fn main() {
    let args: Vec<_> = env::args().collect();
    if args.len() < 2 {
        println_stderr!("Usage: {} GIMME_TEH_FILEZ",
                Path::new(&args[0]).file_name().unwrap().to_str().unwrap());
        process::exit(1);
    }

    let path = Path::new(&args[1]);
    let mut file = match File::open(path) {
        Err(e) => {
            println_stderr!("{}", e);
            process::exit(1);
        },
        Ok(f) => f,
    };

    let mut buf = String::new();
    match file.read_to_string(&mut buf) {
        Err(e) => {
            println_stderr!("{}", e);
            process::exit(1);
        },
        Ok(_) => {},
    }
    print!("{}", buf);
}

C would have allowed you to just ignore errors and pretend everything was good. A naïve cat would be very short, but rather unreliable also. A well‐written cat will go and use all the error codes that are returned to produce a useful result.
Python and similar languages would raise exceptions in these cases, leading to you getting tracebacks in case of errors on a naïve approach; again, a well‐written cat is going to handle all of these exceptions manually, producing a neater result.
Rust makes error‐handling explicit and makes it harder for you to ignore errors, though you still can ignore them if you want by unwrapping results, which gets you something more like the default arrangement in languages like Python. The explicit nature of error handling can be a pain in places (most notably for simple scripts), but it makes for a much more solid result in general as it scales very well.

One must realise that in these common tasks that people tend to assume must succeed there are actually possibilities of failure.

Writing to stderr and stdout may fail. They might be being piped to a read‐only file, or on a device that is not ready or full. print!, for example, will panic if writing fails.
By using String, you’ve constrained your lolcat to only use UTF‐8; if you pass a non‐UTF‐8 file, it will blow up in your face. You might want to vec![] instead of String::new(), read_to_end instead of read_to_string and stdout().write(&buf) instead of print!("{}", buf).

Macros can be quite good at alleviating these sorts of pains; there is try!, for example, which works well at handling Results; if you refactored your code so that all of the important stuff was inside a function which could then return Result<(), io::Error>, for example, you could use try! on all of the fallible I/O operations. As it is, you can do things like writing a new macro to assist, and also give a more helpful message explaining where the error occurred.

Here’s something more like what I would write; of course, there are various points that are subjective, and I have made some stylistic changes to be in line with what I believe to be more common standard style:

use std::env;
use std::io::{Read, Write, stdout};
use std::fs::File;
use std::path::Path;
use std::process;

macro_rules! println_stderr {
    ($($arg:tt)*) => {
        match writeln!(&mut ::std::io::stderr(), $($arg)*) {
            Err(e) => panic!("Unable to write to stderr: {}", e),
            Ok(_) => (),
        }
    }
}

macro_rules! try_or_die {
    ($e:expr, $err_msg:expr) => {
        match $e {
            Ok(x) => x,
            Err(e) => {
                println_stderr!(concat!($err_msg, ": {}"), e);
                process::exit(1);
            },
        }
    }
}

fn main() {
    let args: Vec<_> = env::args().collect();
    if args.len() < 2 {
        println_stderr!("Usage: {} GIMME_TEH_FILEZ",
                        Path::new(&args[0]).file_name().unwrap().to_str().unwrap());
        process::exit(1);
    }

    let path = Path::new(&args[1]);
    let mut file = try_or_die!(File::open(path), "failed to open input file");

    let mut buf = vec![];
    try_or_die!(file.read_to_end(&mut buf), "failed to read input file");
    try_or_die!(stdout().write(&buf), "failed to write output");
}

(At first I had try_or_exit, but then try_or_die sounded better.)


Finding out which user has made the most progress I have some code to accomplish the objective of finding out which user has made the most progress. Progress is measured in integer values, the higher the better. SubmittedAnswers consist of among others a int UserId and a int Progress.

The answers variable consist of a list of SubmittedAnswers as described above. To find out which user has made the most progress of this list I have the following algorithm:

/// <summary>
/// Method used to find out which user had made the most progress in a given time range.
/// </summary>
/// <param name="timeRange"></param>
/// <returns></returns>
public async Task<User> MostProgressAsync(ITimeRange timeRange)
{
    IList<SubmittedAnswer> answers = await GetAnswersInRange(timeRange);

    IEnumerable<IGrouping<int, SubmittedAnswer>> answersOfUsers = answers.GroupBy(a => a.UserId);

    int maxProgress = int.MinValue;
    var maxProgressUserId = 0;
    foreach (IGrouping<int, SubmittedAnswer> answersOfUser in answersOfUsers)
    {
        int progress = answersOfUser.Sum(a => a.Progress);
        if (progress > maxProgress)
        {
            maxProgress = progress;
            maxProgressUserId = answersOfUser.Key;
        }
    }

    return new User { UserId = maxProgressUserId };
}

SubmittedAnswers and User looks as follows:

public class SubmittedAnswer
{
    public ObjectId Id { get; set; }
    public int SubmittedAnswerId { get; set; }
    public BsonDateTime SubmitDateTime { get; set; }
    public bool Correct { get; set; }
    public int Progress { get; set; }
}

public class User
{
    public int UserId { get; set; }
}

Is this a proper way to accomplish this objective? Can it be improved?

Personally I would like to solve the issue a bit more functional, without declaring the variables above a foreach loop.

The method's name can be improved. I like that you ended it with Async, it makes it compliant with the convention for async Task<T> methods - however there's another naming convention programmers use all the time: method names should start with a verb! FindUserWithMostProgressAsync might be a better name, unless that method is located in the User class itself (then it would be misplaced).

IList<SubmittedAnswer> answers = await GetAnswersInRange(timeRange);

This tells me GetAnswersInRange is an async function that isn't following the convention - should be GetAnswersInRangeAsync, but then it might be a better idea to just call it GetAnswersAsync and let overloading speak the "in range" part: parameterless returns all answers, overload taking an ITimeRange returns all answers within the specified time range - and that can be nicely documented with XML comments.

Now, returning an IList<SubmittedAnswer> when all you really need is to iterate the results, is bad communication: IEnumerable<SubmittedAnswer> would have worked better.

IEnumerable<IGrouping<int, SubmittedAnswer>> answersOfUsers = answers.GroupBy(a => a.UserId);

LINQ came with C# 3.5, and that version introduced a little wonder to avoid declaring obscure types such as an IEnumerable<IGrouping<int, SubmittedAnswer>> - implicit typing changes strictly nothing to how the code works, saves you a bunch of typing, and makes shorter, more readable code here:

var answersOfUsers = answers.GroupBy(a => a.UserId);

Explicit typing is actually quite annoying here:

foreach (IGrouping<int, SubmittedAnswer> answersOfUser in answersOfUsers)

Compare to:

foreach (var answersOfUser in answersOfUsers)

There's a typo there that becomes immediately apparent: the loop variable is named exactly the same thing at the collection that's being iterated - that code wouldn't compile... so I'll assume you meant to have this:

foreach (var answerOfUser in answersOfUsers)

Now now now... you have a grouping, and you're entering a foreach loop, only to filter some more - that foreach loop could be converted to a LINQ query.

So we have all the answers in the time range we're interested in:

var answers = await GetAnswersInRange(timeRange);

And what we're after, is the UserId for the user that has accumulated the most progress with these answers. Grouping by UserId was a good first step.

var userAnswers = answers.GroupBy(answer => answer.UserId);

Next, we want to know how much progress each user has - we can do this by projecting a selection into an anonymous type, with the Select method:

var userProgress = userAnswers.Select(userAnswer =>
    new { UserId = userAnswer.Key, Progress = userAnswer.Sum(answer => answer.Progress) });

And this is where var has a clear advantage over explicit typing - exactly what is the type of userProgress here? It's an IQueryable<T>, where T is an anonymous type with some int UserId and int Progress members: we don't need to care what the actual type is.

What's next? Well, what we have now could be visualized as a table with a UserId and a Progess column. The next step is to find out which user has the most progess, and we can sort the results to get that, in descending order so we have the top user, well, at the top:

var sortedUserProgress = userProgress.OrderByDescending(progress => progress.Progress);

And then, all we need to do is grab the first "row":

var topUserId = sortedUserProgress.First().UserId;

And then we can create the User object for the return value:

return new User { UserId = topUserId };

...That's all nice, but it's excessively verbose:

var answers = await GetAnswersInRange(timeRange);
var userAnswers = answers.GroupBy(answer => answer.UserId);
var userProgress = userAnswers.Select(userAnswer =>
    new { UserId = userAnswer.Key, Progress = userAnswer.Sum(answer => answer.Progress) });
var sortedUserProgress = userProgress.OrderByDescending(progress => progress.Progress);
var topUserId = sortedUserProgress.First().UserId;

The good news is that LINQ is awesome, and lets you chain all these methods, so you can actually do this:

var answers = await GetAnswersInRange(timeRange);
var userId = answers.GroupBy(answer => answer.UserId)
                    .Select(userAnswer => new
                        {
                            UserId = userAnswer.Key,
                            Progress = userAnswer.Sum(answer => answer.Progress)
                        })
                    .OrderByDescending(progress => progress.Progress)
                    .First().UserId;

Note that .First() will throw an exception if there is no element in the sorted results. You could use .FirstOrDefault() instead, but then the .UserId member call would have to go on a separate instruction, because FirstOrDefault() will return null when there's no "first element", which means you'd be calling .UserId on a null reference, which would obviously throw a NullReferenceException... and your current code returns a valid User object with UserId set to 0, which leads me to believe the rest of your code has checks that verify whether a UserId is zero or not... which is rather ugly. How about this instead?

var answers = await GetAnswersInRange(timeRange);
var result = answers.GroupBy(answer => answer.UserId)
                    .Select(userAnswer => new
                        {
                            UserId = userAnswer.Key,
                            Progress = userAnswer.Sum(answer => answer.Progress)
                        })
                    .OrderByDescending(progress => progress.Progress)
                    .FirstOrDefault();

return result == null
    ? null
    : new User { UserId = result.UserId };

Returning null when there's no data for the specified timeRange makes more sense to me than returning a User object with some meaningless default/hard-coded values.


New to Rust: Hangman game I decided to try my hand at a Hangman game to familiarize myself a bit with the language.
use std::io::{stdin, stdout, Write};

fn main() {
    let stdin = stdin();
    let mut stdout = stdout();

    let to_guess = String::from("BENEDICT");

    let mut letters: u128 = 0;

    loop {
        // Replace unguessed letters of to_guess by underscores
        let current = to_guess.chars().map(|x| {
            if x <= 127 as char && letters & (1 << x as u8) > 0 {
                x
            } else {
                '_'
            }
        });
        // Add spaces between letters
        let current = current
            .map(|x| x.to_string() + &' '.to_string())
            .collect::<Vec<String>>()
            .join(" ");

        println!("Current word: {}", current.trim_end());

        if !current.contains('_') {
            println!("Congratulations!");
            break;
        }

        print!("Enter new letter: ");
        stdout.flush().unwrap();

        let mut letter = String::new();
        stdin.read_line(&mut letter).expect("Invalid letter");

        if letter.trim().len() != 1 {
            println!("Use only one ASCII letter");
            continue;
        }

        let letter = letter.to_uppercase().chars().next().unwrap();

        letters |= 1 << letter as u8
    }
}

The to_guess word is hardcoded, and there is an unlimited number of guesses.
To store letters already guessed, I use an u128 and flag the corresponding bit. It should be ok since all ASCII letters are represented by u8 < 128.
Can you help me review the code, are there "Rust" things I missed?

Welcome to Rust.  Here's some suggestions to get you started:
Creating idiomatic use declarations
It is not common in Rust to bring a function into scope directly via a use declaration.  Instead, io::stdin and io::stdout are preferred.  See the section Creating Idiomatic use Paths in the book for more information.
Using structs to self-document code
You included this sentence in your explanation of the code:

To store letters already guessed, I use an u128 and flag the
corresponding bit. It should be ok since all ASCII letters are
represented by u8 < 128.

It is preferable to include this information in the code itself.  In this case, we can introduce a struct:
#[derive(Clone, Copy, Debug, Default)]
struct AsciiSet {
    flag: u128,
}

impl AsciiSet {
    fn new() -> Self {
        Self { flag: 0 }
    }

    fn push(&mut self, letter: u8) {
        self.flag |= (1 << letter);
    }

    fn contains(&self, letter: u8) -> bool {
        (self.flag & (1 << letter)) != 0
    }
}

and modify the code accordingly:
let mut letters = AsciiSet::new();

loop {
    // ...
    if letters.contains(letter) { /* ... /* }

    // ...
    letters.push(letter);
}

Checking for ASCII
letter as u8 truncates the character if !letter.is_ascii().  Check for this case and prompt the user accordingly.


Drawing and moving flowers So I've got this module called risar, which draws. But that's not really of importance. I wrote this code which sets 20 flowers on the background.

Now they need to move downwards by 5 units. The code works fine but it looks horribly awkward to me. I'd like it to look more "fancy" or maybe that less loops would be used or I'd like to see simply another way to do it. I need help with 2nd function; the 1st one looks okay. I'm relatively new to Python.

def doFlowers():
    flowers = []
    colors = ["black_flower.svg","blue_flower.svg", "brown_flower.svg", "green_flower.svg","purple_flower.svg"]
    for i in range(20):
        x = random.randint(20, (risar.maxX-20))
        y = random.randint(20, 300)
        flower = risar.picture(x, y, barve[i//4]) # 0, 1, 2, 3, 4
        flowers.append(flower)
    return flowers
flowers = doFlower()

def moving(flower):
    for i in range(len(flower)):
        while flower[i].y() < risar.maxY:
            for p in flower: #p is position
                p.setPos(p.x(), p.y()+5)

Let's start with your naming.  flower is supposed to be a list, right?  Let's make it plural, then.

def moving(flowers):
    for i in range(len(flowers)):
        while flowers[i].y() < risar.maxY:
            for p in flowers: #p is position
                p.setPos(p.x(), p.y()+5)

Next, notice that you don't actually care about i; you just want to operate on every single flower.

def moving(flowers):
    for f in flowers:
        while f.y() < risar.maxY:
            for p in flowers: #p is position
                p.setPos(p.x(), p.y()+5)

Hmm… not only are there nested loops, but two of them iterate over all flowers.  What's going on?  I think you're trying to move all flowers "simultaneously" until all of them have their y-coordinate ≥ risar.maxY.  Then just say this, which I think is a bit more expressive:

def moving(flowers):
    while any([f.y() < risar.maxY for f in flowers]):
        for flower in flowers:
            flower.setPos(flower.x(), flower.y() + 5)