rock paper scissors game simplify in Python I wrote rock paper scissors game using what I have learnt yet.It is while,for statements,list,tuple,dictionary and such simple things only.So I am interested how can I simplify this code using only what I have learnt(simple things)

Here is code

    k = input("Hello guys what are you names: ")
    j = input("and yours? ")
    print(f"""ok {k} and {j} let the game begin!"
    P.s. Type exit to quit game""")
    list = ["rock","paper","scissors"]
    while True:

        a_player = input(f"{k} choose: rock, scissors, paper : ").lower()
        if a_player == "exit":
            break
        b_player = input(f"{j} choose: rock, scissors, paper: ").lower()
        if b_player == "exit":
            break
        for type in list:
            if a_player == type and b_player==type:
                print("  Draw guys, draw")
                break
        if b_player != list[1] and b_player!=list[0] and b_player!=list[2] or (a_player != list[1] and a_player != list[0] and a_player != list[2]) :
            print("Please type correctly")

        if a_player == list[1]:
            if b_player == list[0]:
                print(f" {k} wins: paper beats rock")
            elif b_player ==list[2] :
                print(f"{j} wins: scissors beat paper")

        elif a_player == list[2]:
            if b_player == list[0]:
                print(f"{j} wins: rock beats scissors")
            elif b_player == list[1]:
                print(f"{k} wins: scissors beat paper")

        elif a_player == list[0]:
            if b_player == list[2]:
                print(f"{k} wins: rock beats scissors")
            elif b_player==list[1]:
                print(f"{j} wins: paper beats rock")

I'm new to this community, but I'll give this my best shot:

Avoid single letter variable names. Names like k and j convey little about what values they represent, so replace them with something like:

a_player_name = input("Hello guys what are you names: ")
b_player_name = input("and yours? ")

This code:

a_player = input(f"{k} choose: rock, scissors, paper : ").lower()

creates the need to catch errors down the line and is repeated so I recommend replacing it with:

 def input_choices(string):
    while True:
        choice = input(string)
        if choice.lower() in list:
            return choice
        else:
            print('Please type correctly')

a_player = input_choices(f"{k} choose: rock, scissors, paper : ")

This avoids the need for:

    if b_player != list[1] and b_player!=list[0] and b_player!=list[2] or (a_player != list[1] and a_player != list[0] and a_player != list[2]) :
        print("Please type correctly")

Which I hope you agree is a vast improvement. Remember you can always format conditionals like this to improve readablity:

if (
    b_player != list[0] and b_player != list[1] and b_player != list[2]
    or a_player != list[1] and a_player != list[0] and a_player != list[2]
):
    print("Please type correctly")

Or even better use the in keyword

if (
    a_player not in list
    or b_player not in list
):
    print("Please type correctly")

As for the actual rock-paper-scissors logic:

    if a_player == list[1]:
        if b_player == list[0]:
            print(f" {k} wins: paper beats rock")
        elif b_player ==list[2] :
            print(f"{j} wins: scissors beat paper")

    elif a_player == list[2]:
        if b_player == list[0]:
            print(f"{j} wins: rock beats scissors")
        elif b_player == list[1]:
            print(f"{k} wins: scissors beat paper")

    elif a_player == list[0]:
        if b_player == list[2]:
            print(f"{k} wins: rock beats scissors")
        elif b_player==list[1]:
            print(f"{j} wins: paper beats rock")

I'm sure this can be improved with table-driven logic, but I'm sure there's no end of examples for you to skim through in the 'Related' tab.

Hope that helped :)


Member function to turn class into dict I'm working on a project where I often want to turn class instances into dictionaries to write them to MongoDB. I'd like some feedback on the following:

def getDict(self):
    self.dict = dict()
    for key in self.__dict__.keys():
        if key not in self.NOT_INCLUDE: # class variable
            try:
                self.dict[key] = getattr(getattr(self, key), "getDict")()
            except:
                self.dict[key] = getattr(self, key)
    return self.dict

This will turn any class with getDict into a dictionary, including any instance properties that themselves have a getDict method.

Also please let me know if there's an easy way to do this with the standard library that I missed.

What are you Returning

This function is adding a new member variable, self.dict. Presumably you want to just return a new temporary variable. Also, dict() is actually less efficient than {}, so you might want to start with:

member_dict = {} # or however you want to name it

Iterating over a Dict

In python, iterating over a dict is the same as iterating over the keys, so we can just save that step:

for key in self.__dict__:

But really, you don't just want the keys, you want the values too, so let's just get both:

for key, val in self.__dict__.items():

getattr

Use getattr() only for variable attribute names. Otherwise, just reference the attribute directly. getattr(x, 'getDict')() is more verbose and harder to parse than x.getDict().

Catching Exceptions

Don't just catch every exception - catch the one you expect to see. In this case, AttributeError:

member_dict = {}
for key, val in self.__dict__.items():
    if key not in self.NOT_INCLUDE:
        try:
            member_dict[key] = val.getDict()
        except AttributeError:
            member_dict[key] = val
return member_dict

Add a Free Function

def getDictIfExists(obj):
     try:
         return obj.getDict()
     except AttributeError:
         return obj

This lets you write the whole thing as a generator expression

return {key: getDictIfExists(val)
        for key, val in self.__dict__.items()
        if key not in self.NOT_INCLUDE}


Rust Locker Problem Number Theory The Locker Problem is as followed:

Twenty bored students take turns walking down a hall that contains
a row of closed lockers, numbered 1 to 20. The first student opens all the
lockers; the second student closes all the lockers numbered 2, 4, 6, 8, 10, 12, 14,
16, 18, 20; the third student operates on the lockers numbered 3, 6, 9, 12, 15, 18:
if a locker was closed, he opens it, and if a locker was open, he closes it; and so
on. For the ith student, he works on the lockers numbered by multiples of i: if a
locker was closed, he opens it, and if a locker was open, he closes it. What is the
number of the lockers that remain open after all the students finish their walks?

I decided to take this problem the extra mile and instead allow the user determine how many students were in the hallway so the number isn't always 20. In what ways can I refactor my code?
use std::io;

fn main() {
    // false = closed
    // true = opened
    let mut lockercount = String::new();
    println!("How many students?");
    io::stdin().read_line(&mut lockercount).expect("expected int");
    let lc = lockercount.trim().parse().unwrap();
    let mut lockers = vec![false; lc];

    for i in 1..(lockers.len()+1) {
        let mut j: usize = 1;
        while i*j <= lc {
            lockers[(i*j)-1] = !lockers[(i*j)-1];
            j += 1;
        }
    }

    let mut lockernum = 1;
    println!("Opened lockers:");
    for v in lockers {
        if v == true {
            print!("{}, ", lockernum);
        }
        lockernum += 1;
    }
}

Your code seems fine. However, there is one part I'd definitely change: the scope of variables. For example, lockercount isn't necessary for more than the initial lc. We can keep its scope down:
    println!("How many students?");
    let lc = {
        let mut lockercount = String::new();
        io::stdin().read_line(&mut lockercount).expect("expected int");
        lockercount.trim().parse().unwrap()
    };

Similarly, I would prefer to use enumerate instead of an explicit counter. An explicit counter might not be incremented by accident, but enumerate on an iterator will always fit:
    println!("Opened lockers:");
    for (n, v) in lockers.iter().enumerate() {
        if *v == true {
            print!("{}, ", n + 1);
        }
    }

We could also get rid of the if via filter, but that's a little bit much:
    for (n, _v) in lockers.iter().enumerate().filter(|(_, &b)| b) {
        print!("{}, ", n + 1);
    }

On the algorithmic part of the code, I'd prefer to use the position explicitly instead of i*j:
    for i in 1..(lockers.len() + 1) {
        let mut pos: usize = i;
        while pos <= lc {
            lockers[pos - 1] = !lockers[pos - 1];
            pos += i;
        }
    }

Again, instead of explicitly changing a counter, I would prefer an immutable variable, for example via step_by:
    for i in 1..(lockers.len() + 1) {
        for pos in (i..lc).step_by(i) {
            lockers[pos - 1] = !lockers[pos - 1];
        }
    }


Beginner Java Counter code My professor wants me to do this:

  Write a number of interchangeable counters using the Counter interface below:

public interface Counter {
/** Current value of this counter. */
int value();
/** Increment this counter. */
void up();
/** Decrement this counter. */
void down();
}

I need comments on my work so far.  Do you think it's sufficient?  How do I work on the ResetableCounter?  I'm very new to Java and it's been a long time since I've done C++.

Develop the following:

  An interface ResetableCounter that supports the message void reset() in addition to those of Counter.

Here's what I did:

public interface ResetableCounter {
void reset();
int value();
void up();
void down();
}

  An implementation of ResetableCounter called BasicCounter that starts at the value 0 and counts up and down by +1 and -1 respectively.

Here's what I did:

public class BasicCounter implements ResetableCounter
    {
      int counterVariable = 0;
    public static void main(String[] args)
    {
        BasicCounter cnt = new BasicCounter();
        cnt.up();
        cnt.down();
        System.out.printf("The value is %d", cnt.counterVariable);
    }

    public void reset() {
        this.counterVariable = 0;
    }

    public int value() {
        return this.counterVariable;
    }

    public void up() {
        ++this.counterVariable;
    }

    public void down() {
        --this.counterVariable;
    }
    }

  An implementation of ResetableCounter called SquareCounter that starts at the value 2, counts up by squaring its current value, and counts down by taking the square root of its current value (always rounding up, i.e. 1.7 is rounded to 2, just like 1.2 is rounded to 2).

Here's what I did:

public class SquareCounter implements ResetableCounter {
int counterVariable = 2;
public static void main(String[] args) {
    SquareCounter cnt = new SquareCounter();
    cnt.up();
    cnt.down();
    double d = Math.ceil(cnt.counterVariable);
    System.out.printf("The value is %f", d);
}

public void reset() {
    this.counterVariable = 0;
}

public int value() {
    return this.counterVariable;
}

public void up() {
    Math.pow(this.counterVariable, 2);
}

public void down() {
    Math.sqrt(this.counterVariable);
}
 }

  An implementation of ResetableCounter called FlexibleCounter that allows clients to specify a start value as well as an additive increment (used for counting up) when a counter is created. For example new FlexibleCounter(-10, 3) would yield a counter with the current value -10; after a call to up() its value would be -7.

public class FlexibleCounter implements ResetableCounter
{
public static void main(String[] args)
{
    int start = Integer.parseInt(args[0]);
    int step = Integer.parseInt(args[1]);
    start.up();
    System.out.printf("The value is %d", count);
}

public void reset() {
    this.count = 0;
}

public int value() {
    return this.count;
}

public void up() {
    this.count = args[0] + args[1];
}

public void down() {
    --this.count;
}
}

  All of your implementations should be resetable, and each should contain a main method that tests whether the implementation works as expected using assert as we did in lecture (this is a simple approach to unit testing which we'll talk about more later).

An interface ResetableCounter that supports the message void reset() in addition to those of Counter.

Your professor provides this statement to you as a hint. It is practically saying ResetableCounter should extends Counter interface.

make countVariable or count(as @200_success suggested) private. As no other outside class should have any permission to change the count.


Finding average of stream of numbers A Brief Context

I recently gave an interview in a company where I was asked this particular question. After the interview was over, the HR gave the feedback that my solution (is correct programmatically) but is monolithic and not scalable. The interview was for frontend developer 2.

Question

Suppose there is a company which does some analytics work and returns data. The company (let's say X) gets a stream of numbers and it calculates average at that instant for all numbers received and returns the average. There could be other companies (let's say Y) which can ask for the average.

  Data:   1  3  5  7  9
  Avg:    1  2  3  4  5

So, in the above example, the average are calculated by company X.

After first no, average is 1,
After second no (i.e. 3), average is 2 ((3+1)/2) and so on

At any point in time, companies are only concerned about average at that time (basically, if average is asked after data value 5, you only need to return 3)

The geeksforgeeks link for problem is https://www.geeksforgeeks.org/average-of-a-stream-of-numbers/

Complexity

The solution needs to be O(1) time and O(1) space

My solution:

I wrote a closure in Javascript to save the current average and total number of values encountered so far. Then wrote another function to call it by passing some parameters. The parameters decide whether I should be updating the average or returning it.

function analytics() {
  const data = {
    avg: 0, //the average at that instant
    totalPoints: 0 //total number of values encountered till that point
  }
  return function(value) {
    //if value is not present, return the avg
    if (!value) {
      return data.avg
    }
    const newTotal = data.avg * data.totalPoints + value
    const newTotalP = data.totalPoints + 1
    //update the value for next usage
    data.avg = newTotal / newTotalP
    data.totalPoints = newTotalP
    return data.avg
  }
}

fn = analytics()

/*
 param = {
  type: //Company Names
  value: //Value to update
 }
*/

function y({
  type,
  value
}) {
  if (["Y"].indexOf(type) >= 0) {
    return fn()
  } else if (["X"].indexOf(type) >= 0) {
    return fn(value)
  }
}

console.log("get avg", y({
  type: "Y"
}))
console.log("update avg", y({
  type: "X",
  value: 1
}))
console.log("update avg", y({
  type: "X",
  value: 3
}))
console.log("update avg", y({
  type: "X",
  value: 5
}))
console.log("get avg", y({
  type: "Y"
}))
console.log("update avg", y({
  type: "X",
  value: 7
}))
console.log("update avg", y({
  type: "X",
  value: 9
}))
console.log("get avg", y({
  type: "Y"
}))

My concerns/doubts:

What did I miss here so that solution is not scalable
Any use cases when it might cause issues
How can it be re-written to make it scalable

Things the can be assumed won't cause any issues

Need not worry about precision issues while calculating the average
Need not worry about issues due to Number datatype not able to accomodate very large values
Data validation issues
Variable names etc (I re-wrote the solution again for codereview, so some names are not correct)

Hope, I am able to explain the situation properly and would like to get more insights here. Also, let me know if anyone needs any further explanation or something is missing

This is a good first-cut solution to the functional requirement of stated question. However, when you look at it as production code related to two companies (X and Y) these points arise:

Monolithic: The code should be well demarcated across X and Y company logic. A single analytics function is processing all of the logic with an (admittedly, nice) value control. This has merged the implementation into one piece of code. Say, Y no longer needs the average or, maybe needs something else. Change in code for Y will need to engage with the primary logic of X's code.
Scaling: Imagine a new company Z wants to get not just the average but also the number of data points seen by X (the totalPoints). Changing this monolithic analytics code would engage with all of the existing logic.

These are probably not the only points (and they are also both associated with monolithic and scaling issues). I am just trying to imagine some scaling paths and outline the monolithic-nature of this solution.

To get a fair view of this, consider some changes in the requirements and attempt to modify the code to incorporate them. The complexity required in the edit should be reasonably comparable to the requirement (not higher). Contact of the edit with other parts of the implementation should be minimal.

From a different standpoint, this code would be called a prototype of the logic. The production version would need to handle aspects beyond functionality (like: maintainability, scaling, business-logic separation, readability too).

I wonder if you were asked further questions by the interviewer after reading this program. Were they alluding towards these points.


Using a class for reading config from files The goal of my current piece of work is to read from a global config file all the settings and then from the local file only those settings that a user has chosen.

Firstly I wrote a function but then I needed a class. It's my first working class in Python, and I ask if you can kindly note its defects. Although it works, I think it can for sure be written more correctly and concisely.

Here's all the beginning:

# files to be read from
cmd_parameter = "config_1.cfg"
cmd_parameter_global = "global.cfg"

if len(sys.argv) == 2:
    cmd_parameter = sys.argv[1]
else:
    print("Error: there should be one argument after program name - config file name")
    exit(-1)

config_file = []
config_file_global = []

# reading user's config file
try:
    with open(cmd_parameter, 'r') as config_file_name:
        config_file = config_file_name.readlines()
except FileNotFoundError:
    print("File '", sys.argv[1], "' not exists or not available")
    exit(-1)

# reading global config file
try:
    with open(cmd_parameter_global, 'r') as config_file_global_name:
        config_file_global = config_file_global_name.readlines()
except FileNotFoundError:
    print("Global configuration file '", cmd_parameter_global, "' not exists or not available")
    exit(-1)

# parsing config file, reading values from it to list and matching parameters with a given argument
# parameters in config file should start with <conf_key_split1>, value should be separated from parameter by <conf_key_split2>
config_param_user = []
config_value_user = []
config_param_global = []
config_value_global = []
config = ""
conf_key_split1 = '.'
conf_key_split2 = '='
conf_key_split3 = ','

# reading user's config file
for line in config_file:
    if line.startswith(conf_key_split1):
        line = line.rstrip().split(conf_key_split1)
        line = line[1]

        line = line.rstrip().split(conf_key_split2)
        config_param_user.append(line[0])
        config_value_user.append(line[1])

# reading global config file
for line in config_file_global:
    if line.startswith(conf_key_split1):
        line = line.rstrip().split(conf_key_split1)
        line = line[1]

        line = line.rstrip().split(conf_key_split2)
        config_param_global.append(line[0])
        config_value_global.append(line[1])

And here is the class itself:

# class for reading config from user's and global config file the list of params names list and the list of values
class configFromFile():
    def config_from_file_userfile(self, param):
        index = config_param_user.index(param)
        if config_value_user[index]:
            config_value = config_value_user[index]
        else:
            print('The value of the parameter "', param, '" was not found in', cmd_parameter)
            exit(-1)

        return config_value

    def config_from_file_global(self, param):
        index = config_param_global.index(param)
        if config_value_global[index]:
            config_value = config_value_global[index]
        else:
            print('The value of the parameter "', param, '" was not found in', cmd_parameter_global)
            exit(-1)

        return config_value

    def main(self, param):
        if param in config_param_global:
            config = self.config_from_file_global(param)
        elif param in config_param_user:
            config = self.config_from_file_userfile(param)
        else:
            print('The parameter "', param, '" was not found in', cmd_parameter, 'and in', cmd_parameter_global)
            exit(-1)

        return config

Usage:

names = configFromFile().main('names')

Firstly, you state:

  I wrote a function but then I needed a class

Looking at the class you wrote, I would say you were incorrect there. The class has no __init__, holds no state, and only uses self to access other methods on the class. There is absolutely no reason for it to be a class.

Duplication is the programmer's arch enemy - every time you have the same thing written out more than once is an opportunity to reduce the amount of code you write, test and maintain. For example:

# reading user's config file
try:
    with open(cmd_parameter, 'r') as config_file_name:
        config_file = config_file_name.readlines()
except FileNotFoundError:
    print("File '", sys.argv[1], "' not exists or not available")
    exit(-1)

# reading global config file
try:
    with open(cmd_parameter_global, 'r') as config_file_global_name:
        config_file_global = config_file_global_name.readlines()
except FileNotFoundError:
    print("Global configuration file '", cmd_parameter_global, "' not exists or not available")
    exit(-1)

Note that these do more-or-less exactly the same thing, and should therefore be refactored to something like:

def extract_file(filename, message):
    try:
        with open(filename) as file_:
            return file_.readlines()
    except FileNotFoundError:
        print(message.format(filename))
        exit(-1)

and then called like:

config_file = extract_file(cmd_parameter,
                           "File '{}' not exists or not available")

You should apply this throughout your code - you have basically written the same program twice, with minor adjustments, for the two config files, which makes no sense.

The dict is a key data structure in Python, and would make all of your code vastly simpler. You could create a dictionary of the global configuration options, then update it with the local values. Lookups by name would then be O(1), rather than O(n), and you could get rid of all the indexing.

In terms of general structure, you have lots of code running at the top level of the script. This is generally a bad idea, and will give you problems if you later want to import this functionality elsewhere. You should define an entry point (usually, but not necessarily, called main) that takes the file locations as parameters and call it something like:

if __name__ == "__main__":
    if len(sys.argv) == 2:
        main('global.cfg', sys.argv[1])
    else:
        print("Error: there should be one argument after program name - config file name")
        exit(-1)


Handling views and templates My guts tells me that this isn't the proper way to handle views / templates, so that's why I'm asking you what could be done better.

Some background: This application I'm building now is for use in a school project with people that don't have much knowledge of PHP. Therefore I'm not using a framework like Laravel, or implementing a whole template engine, but I'm trying it to keep as simple as possible while still using classes and code separation.

The homeController:

Class homeController extends BaseController
{

    public function index()
    {
        $this->view->make("common/home");
    }
}

The View class:

Class View{

    public static $file;

    public function make($file){
        View::$file = $file;
        require_once DIR_VIEW . "default/template.php";
    }
}

The template.php file:

<!DOCTYPE html>
<html>
<head>
    <title>Just an title</title>
</head>
<body>
    <?php require_once 'header.php'; ?>

    <div id="mainContent">
        <?php require_once DIR_VIEW . View::$file . ".php"; ?>
    </div>

    <?php require_once 'footer.php'; ?>
</body>
</html>

BaseController:

class BaseController
{

    public $load;
    public $url;
    public $view;

    public function __construct()
    {
        $this->load = new Loader();
        $this->url = new Url();
        $this->view = new View();
    }
}

The files are required by an loader class with this piece of code: (don't know if it ads some relevance)

foreach (glob(DIR_LIB . "*.php") as $filename) {
    require_once $filename;
}

The problem is probably that i'm using an static variable $file inside the View class. I don't think that that is right, but don't know any better way (besides using global(), but I know that that isn't the solution.

What do you think? Is this good enough or not even close?

Note: there will always be 1 file included, so that's why I thought that a simple require would succeed.

You're over-complicating it.

First the static variable isn't required, all the variables in the scope of include/require statements are available in the document being included or required.  For example:

FileA.php

<?php
$variable1 = "Something";
require_once __DIR__ . "/FileB.php";

FileB.php

<?php
echo $variable1; // Outputs "Something"

So the $view variable will just be accessible in the template.php file.  That said I think your code could be changed up just a bit and be a lot cleaner.  I went through and tweaked it some with comments below.  Feel free to ask any questions you may have after reading through it:

View.php

<?php
class View {
    const DIR_VIEW = "/views/";

    function make ($view, $data = array()) {
        extract($data, EXTR_SKIP); // Extracts key/value pairs from data as variables

        ob_start();

        // Just require header here, later on you could add another option to make for layout,
        // to use different headers footers if needed.  Just include everything up to
        //
 in the header.php file.
        require __DIR__ . Views::DIR_VIEW . 'header.php';

        // Do this better, it's open to a directory traversal attack: http://en.wikipedia.org/wiki/Directory_traversal_attack
        require __DIR__ . Views::DIR_VIEW . $view . '.php';

        require __DIR__ . Views::DIR_VIEW . 'footer.php'; // Just require footer here

        return ob_get_clean();
    }
}

HomeController.php

<?php
class HomeController extends BaseController // Capitalize the H
{

    public function index()
    {
        // Add a return, have your router take the action method's return value and echo it.
        // This way your view class isn't just arbitrary printing things out, and will give you
        // flexibility later to have different types of returns (like for redirects you could
        // do Redirect::to('/'), similarly to laravel).

        return $this->view->make("common/home");
    }
}

common/home.php

Bam! <?php echo $someVariable; ?> Wow! Such Templates!

header.php

<!DOCTYPE html>

    <title><?php echo isset($title) ? $title : 'Your site'; ?></title>

        Home

footer.php

        &copy; Copyright 2014


A simple C++ Trie for working with strings Here is a simple C++ 11 trie for insertion and membership testing:

#include <stdio.h>
#include <stdlib.h>
#include <map>
#include <string.h>

class TrieNode{
  //TrieNodes can also be used as tries
public:
  std::map<char, TrieNode> children;
  bool isLeaf;

  bool isContained(char * string){
    if (strlen(string)==0){
      return isLeaf;
     }

    char searchChar = *string; //the next char we are looking at

    if (children.find(searchChar) == children.end()){
      return false;
    }

    /*search for everything but the first char from the string in the trie
      for the first char*/
    return (children[searchChar].isContained(1+string));
  }

  void insertString(char * string){
    if (strlen(string) == 0){
      //If an empty string has been parsed then this node is the end of a string
      isLeaf = true;
      return;
    }

    char searchChar = *string;

    if (children.find(searchChar) == children.end()){
      //Create a new trie for the first character of our string
      TrieNode newNode;
      children[searchChar] = newNode;
    }

    children[searchChar].insertString(string+1);
  }
};

int main(){
  //Sample main function
  TrieNode myTrie;
  printf("hello in trie = %d\n",myTrie.isContained((char *) "hello"));
  myTrie.insertString((char *) "hello");
  printf("hello in trie = %d\n",myTrie.isContained((char *) "hello"));
  return 0;
}

I am relatively new to C++ (I have been muddling around with it for maybe a year now) so am wondering whether I may have either accidentally given my code nasal demons and/or accidentally wasted a lot of memory.

A couple of thoughts.

Maybe a distinction between Trie and TrieNode might make sense. So TrieNode can be defined as an inner struct within an outer class Trie.
There is no compelling reason to use a char* string representation rather than using string.
unordered_map would do just as well as a map, and possibly be faster.
You can consider iterating using a while loop instead of recursion in isContained.


Project Euler #10 - sum of primes below two million The sum of the primes below 10 is 2 + 3 + 5 + 7 = 17.

  Find the sum of all the primes below two million.

public class Java {

public static void main(String[] args) {
    long start = System.nanoTime();

    long sum = 0;

    for (int i = 2; i < 2000000; i++) {
        boolean isPrime = true;

        if ((i % 2 == 0 || i % 3 == 0) && (i!=2 || i!=3 || i!=5)) {
            isPrime = false;
        } else {
            for (int j = 5; j <= Math.sqrt(i); j = j + 6) {
                if (i % j == 0 || i % (j + 2) == 0) {
                    isPrime = false;
                    break;
                }
            }
        }

        if (isPrime == true) {
            sum += i;
        }
    }

    System.out.println(sum);
    long end = System.nanoTime();
    System.out.println(end - start);
}

This is probably not efficient since i am using a lot of if-else statements and conditions.

Well, first of all, sorry, but your code is buggy. You are missing 2 and 3, because of this:

if ((i % 2 == 0 || i % 3 == 0)

...since this will be true for 2 and 3. So your result is off by 5.

Also this part is 100% useless:

(i!=2 || i!=3 || i!=5))

How many values for i you can imagine for which this statement is false?

Let's try i = 1 -> true, because 1 != 2 (i != 2)
Now we try i = 2 -> true, because 2 != 3 (i != 3)
Now we try i = 3 -> true, because 3 != 2 (i != 2)
Last try: i = 5 -> true, because 5 != 2 (i != 2)

So, this statement is always true.

Anyway, another solution (getting more speed by sacrificing memory) would be to not calculate if every number is a prime, but do something like this...

final int limit = 2000 * 1000;

final boolean[] knownNotPrime = new boolean[limit + 1];

long sum = 0;

for (int i = 2; i < limit; i++) {

    if (!knownNotPrime[i]) {
        sum += i;

        for (int j = 2; j <= (limit / i); j++) {
            knownNotPrime[j * i] = true;
        }
    }

}

In other words, for every prime you calculate the multiples (up to the limit) and store them in a boolean array (you could decrease the memory by a factor of 8 by using a bitfield, just didn't want to complicate the whole thing).

Since "being not a multiple of any number lesser than itself" is pretty much the definition of "prime", you already know if a number is a prime when you reach it and it's not marked in the array. And if it isn't, you know that it's a multiple of some lesser prime - and thus, all it's multiples will already be known as non-prime, no need to re-calculate them.

Your code takes around 217ms to run, mine takes 17, but on the other hand, mine takes way more than 12 times the memory, even if you used a bitfield.

Note: After using google and trying to remember what I learned in school, this is probably a variation of the Sieve of Eratosthenes and I have no idea how good this implementation is. Probably there are much better solutions out there, I guess.


Program that finds the stretch factor of a parabola Being a newcomer to C++, I decided to write a simple program that would accomplish a simple task.

I settled on writing a program that, given the vertex and point on a parabola, find the "stretch factor", or \$a\$ in the vertex-form equation \$y=a(x-h)^2+k\$.

This is my first go at working with pointers/references, and I expect this attempt to be rather feeble. The program works fine, however, I would love to know if I am employing any measures that are inefficient or that break good practice.

/*
 * pwidth.cpp
 * Given the vertex and a point on a parabola, attempts to find the "stretch factor",
 * which is the `a` var in vertex form equation y=a(x-h)^2+k.
 */

#include <iostream>
#include <cmath>

using namespace std;

float pwidth(int *vx, int *vy, int *px, int *py) {
    return (*py - *vy) / pow(*px - *vx, 2);
}

void gcin(int &cstor, const string &message) {
    cout << message;
    cin >> cstor;

    while (cin.fail()) {
        cout << message;

        cin.clear();
        cin.ignore(256, '\n');

        cin >> cstor;
    }
}

int main() {
    int vx = 0, vy = 0, px = 0, py = 0;

    gcin(vx, "enter vertex X: ");
    gcin(vy, "enter vertex Y: ");
    gcin(px, "enter point X: ");
    gcin(py, "enter point Y: ");

    cout << "a = " << pwidth(&vx, &vy, &px, &py) << endl;

    return 0;
}

This is fairly straightforward. Good work on separating out the user input into its own function! Here are some things that could use improvement:
Don't Use Pointers for Values
In your pwidth() function, you aren't changing the value of any of the arguments. They should be passed by value since they are simple types. You could even make them const to clarify that the function does not change them. (Some would say that's overkill for plain-old-datatypes. It's up to you.)
In general, in C++, it's better to use references than pointers because they are safer. But in both cases, you usually use a reference or pointer either when the value will be changed by the function, or in cases where the data is too large to pass on the stack. (You can use a const in the case where the data is too large to pass on the stack but won't change in the function.) The cstor reference in gcin() is great!
Naming
I think your names could be improved. pwidth and gcin don't mean anything to me. If pwidth() is calculating the "stretch factor", then call it stretch_factor() or something along those lines. (Or if the p stands for "parabola" just name it parabola_width().) I think gcin() would be better named something like prompt_user(), or get_user_input(), or get_int_from_user().
Likewise with variable names. px, vx, etc. should be point_x, vertex_x, etc. It's much easier to read.
Avoid using namespace std
This Stack Overflow question lays out why using namespace std is a bad idea.
Optimization
In your pwidth() function, you're calling the pow() function. That's a really expensive function. For the main() that you've written, it won't make much difference, but if you ever need to use pwidth() in time-critical code, that could become a bottleneck. Since all you're doing is squaring a value, I'd rewrite it like this:
float pwidth(int *vx, int *vy, int *px, int *py) {
    float delta_x = (*px - *vx);
    return (*py - *vy) / (delta_x * delta_x);
}

Why is that the case? Doing it locally is a single machine instruction - a multiply. Calling the pow() function involves first jumping to the code for the function, then doing some math, then jumping back to your code to finish up. Some of that may be hidden by the pipelining, but the math of the pow() function is also more complicated than a single instruction. You can test it for yourself by writing a loop like this and timing it:
float x = 1.2345678;
float sum = 0.0;
for (int i = 0; i < 10000000; ++i)
{
    sum += pow(x, 2.0);
}

Then do it again like, this:
float x = 1.2345678;
float sum = 0.0;
for (int i = 0; i < 10000000; ++i)
{
    sum += x * x;
}

(Just be careful that your compiler doesn't optimize away any of the loop when testing!)


Compute PI in Kotlin on a T-shirt I have written code in Kotlin with the objective of computing Pi in few enough lines so that it looks good on a t-shirt.

Can be cut and paste into http://try.kotlinlang.org under "My Programs" and run from browser - I just tested it with version 1.2.41 and it's working.

import kotlin.math.*
import java.math.BigInteger

fun main(args: Array<String>) {
  val r = (4*(4*arccot(5) - arccot(239))).toString()
  println("314 digits of Pi ${r[0]}.${r.substring(1).dropLast(5)}")
}

fun arccot(x:BigInteger):BigInteger {
  var precision = 10.toBigInteger().pow(319) / x
  var total = precision
  var divisor = 1.toBigInteger()

  while(precision.abs() >= divisor) {
    precision = -precision / x.pow(2)
    divisor += 2
    total += precision / divisor
  }
  return total
}

fun arccot(x:Int) = arccot(x.toBigInteger())
operator fun Int.times(x: BigInteger) = this.toBigInteger() * x
operator fun BigInteger.plus(x: Int) = this + x.toBigInteger()

Currently it's longer than I'd like. I would like to shorten without making it less understandable. My vision is to have code that is readable enough it wouldn't be out of place in a production code base.

To give an idea, here's the significantly shorter Python version (which has been printed on a t-shirt and in my opinion looks good and is short enough but also quite readable). Can be run in browser here: https://repl.it/@sek/314-Digits - (there's also a link from there to the t-shirt if you are curious how that looks - the length in question isn't only the number of lines but also the width of the longest line as that determines the font size that can be used)

def pi():
  r = 4*(4*arccot(5) - arccot(239))
  return str(r)[0]+'.'+str(r)[1:-5]

def arccot(x):
  total = power = 10**319 // x
  divisor = 1
  while abs(power) >= divisor:
    power = -power // x**2
    divisor += 2
    total += power // divisor
  return total

print("314 digits of Pi " + pi())

I noticed that you are calculating to 314 decimal places instead of 314 digits, so drop 6 instead of 5.

You really don't need the extra functions. You can remove

fun arccot(x:Int) = arccot(x.toBigInteger())

if you convert Int to BigInteger inside your arctan method.
You can remove

operator fun Int.times(x: BigInteger) = this.toBigInteger() * x

if you shl(2) instead of * 4.
You can remove

operator fun BigInteger.plus(x: Int) = this + x.toBigInteger()

if you change

divisor += 2

to

divisor += BigInteger("2")

You can shorten

println("314 digits of Pi ${r[0]}.${r.substring(1).dropLast(5)}")

to

println("314 digits of Pi ${r[0]}.${r.substring(1,314)}")

Now, you can change the imports to

import java.math.*

You don't have to declare the type for val r.

The final code I came up with by doing that is:

import java.math.*
fun main(args: Array<String>) {
  val r = (acot(5).shl(2)-acot(239)).shl(2).toString()
  println("314 digits of Pi ${r[0]}.${r.substring(1,314)}")
}
fun acot(x:Int):BigInteger {
  var precision = BigInteger.TEN.pow(319)/x.toBigInteger()
  var total = precision
  var divisor = BigInteger.ONE;
  while(precision.abs() >= divisor) {
    precision = -precision/(x.toBigInteger().pow(2))
    divisor += BigInteger("2")
    total += precision / divisor
  }
  return total
}

I also came up with shorter code, with decreased width, by calculating a different way:

import java.math.*
fun main(args:Array<String>) {
  val b4 = BigDecimal(4)
  val r = ((atan(5)*b4-atan(239))*b4).toString()
  println("314 digits of Pi ${r.substring(0,315)}")
}
fun atan(xInv:Int):BigDecimal {
  var x = BigDecimal(1).divide(BigDecimal(xInv),330,3)
  var (numer, total) = arrayOf(x, x)
  for (i in 3..450 step 2) {
    numer = -numer * x * x
    total += numer / BigDecimal(i)
  }
  return total
}


Send messages to multiple passed recipients or a default recipient I have a function that accepts false as a default value but can be passed an array.

protected function sendMessages ($recipients = false) {

    //cast to an array
    $recipients = is_array($recipients) ? $recipients : [$recipients];

    // if no recipients are passed get email from current object (default)
    if (isset($recipients[0]) && $recipients[0] === false) {
        array_pop($recipients);
        array_push($recipients, 'email' => $this->author->email);
    }

    foreach ($recipients as $recipient) {
        //some logic.
    }
}

The whole, check if the first element is false, and if so swap it out seems a little convoluted / backwards to me. But I'm not sure of a cleaner method. The default value could be changed but is needed so that older calls to the method that specify no parameters don't choke.

protected function sendMessages (array $recipients = []) {

    // if no recipients are passed get email from current object (default)
    if ( ! $recipients) {
        array_push($recipients, 'email' => $this->author->email);
    }

    foreach ($recipients as $recipient) {
        //some logic.
    }
}

Assuming you can change the default value, an empty array evaluates as false: https://3v4l.org/Onpcv (alternatively if you don’t like using it like that, do a check on the count, or empty etc)


Python sprinting game using strings, loops and statements Here is my code for a sprinting game in Python. You have to tap the 'a' and 'd' keys as fast as you can to run 100 meters. Any improvements?

import msvcrt
import time
high_score = 50
name = "no-one"
while True:
    distance = int(0)
    print("\n--------------------------------------------------------------")
    print('\n\nWelcome to the 100m sprint, tap a and d rapidly to move!')
    print('* = 10m')
    print("\n**Current record: " + str(high_score) + "s, by: " + name)
    print('\nPress enter to start')
    input()
    print('Ready...')
    time.sleep(1)
    print('GO!')
start_time = time.time()
while distance < 100:
    k1 = msvcrt.getch().decode('ASCII')
    if k1 == 'a':
        k2 = msvcrt.getch().decode('ASCII')
        if k2 == 'd':
            distance += 1
            if distance == 50:
                print("* You're halfway there!")
                elif distance % 10 == 0:
                    print('*')
fin_time = time.time() - start_time
fin_time = round(fin_time,2)
print('Well done you did it in...'+str(fin_time))
if fin_time < high_score:
    print("Well done you've got a new high score ")
    name = input("Please enter your name : ")

Your code is very straightforward, which is good.

Use a conditional for printing values that don't exist yet

At the very top of your code, you are setting these two variables to default values:

high_score = 50
name = "no-one"

To me, it seems that your reason for this is so that, a few lines later when you are printing these values, you have some data to print.

However, since this is not actual data that means nothing for the user and you are using it as a placeholder, you can use a conditional when printing these messages:

if name == "":
    ... print high score ...

Then, you can set the default values to:

high_score = 0
name = ""

However, this would require later that you ensure that the user does not enter their username as "".

Store high score

Along with that above tip, what you could also try doing is storing the high score data. You don't need some file format. In fact, you could handle your own special type.

Here could be an example high score save/data file:

50
D.C.

where the first line has the completed time, and the second line has the user who achieved that time.

Then, in your script, you can simply read from the high score file.

Remove nesting

In your while distance < 100 loop, you have quite a few levels of nesting. While the code in there isn't too complicated, it still is a bit harder on the eye.

To improve readability, try to keep everything on the same level (without breaking indentation rules):

while distance < 100:
    k = msvcrt.getch().decode('ASCII')
    if k != 'a':
        continue

    k = msvcrt.getch().decode('ASCII')
    if k != 'd':
        continue

    distance += 1
    if distance == 50:
        print("* You're halfway there!")
    elif distance % 10 == 0:
        print('*')

Note: I merged the two k1 and k2 variables into one since the k1 one is no longer needed when k2 is being checked.


Simple local cache that checks by date else get from db I'm looking for feedback on how this can be improved. Basically what I'm trying to do is check if the cache has the object, else check from the database.

I have a list of ids, then I separate them based on whether they already exist in the cache or not, based on the date as well. One list fetches from the cache, the other from the db. Then return the list. That way I don't have to make multiple calls to cache/db.

The alternative is to implement a compound-key, but I would rather not go that route.

https://github.com/mel3kings/taxi-cab-api/blob/master/src/main/java/com/simple/controller/CabController.java

 private List<Cab> getFromCacheOrDb(CabTripsRequest request) {
        Map<Boolean, List<String>> cachedCabs = request.getMedallions().stream().collect(
                partitioningBy(key -> {
                    if (!cache.peek(key)) { return false; }
                    return cache.get(key).stream().anyMatch(a -> a.getPickupDateTime().equals(request.getDate()));
                }));
        List<Cab> response = cachedCabs.get(true).stream().map(k -> cache.get(k)).flatMap(List::stream)
                .filter(cab -> cab.getPickupDateTime().equals(request.getDate())).collect(toList());
        if (cachedCabs.get(false).size() > 0) {
            List<Cab> database = storage.fetch(cachedCabs.get(false), request.getDate());
            response.addAll(database);
            cache.save(database);
        }
        log.info("response from cache/db size:" + response.size());
        return response;
    }

One approach you could pick here is to design your data stores in Storage interfaces in an increasing fashion.

private List<Cab> getCabs(Key key) {
    List<Cab> cabsByKey = this.cache.stream().filter(c -> c.equals(key)).anyMatch();
    if (cabsByKey.isEmpty()) {
       cabsByKey = this.higherStorage.getByKey(key);
    }

    return cabsByKey;
}

The idea is to store its higher level cache instance for each data storage and to propagate request up whenever nothing has been found.


Too many null checks in an XML-to-XML transformation I'm writing a conversion mechanism in C#. I deserialize a big XML document of one type and return a big XML document of another type (via serialization of another object).

While I'm doing the conversion, I find myself doing a TON of null checks (checking whether or not a certain property has a value, which then would need to be translated). Here's an example piece of code:

if (MyPatientCareReport.eHistory != null && // Check if eHistory field exists
    MyPatientCareReport.eHistory.eHistoryCurrentMedsGroup != null && // Check if group exists
    MyPatientCareReport.eHistory.eHistoryCurrentMedsGroup.Count > 0 && // Make sure items exist
    MyPatientCareReport.eHistory.eHistoryCurrentMedsGroup[0].eHistory12 != null && // Null check
    MyPatientCareReport.eHistory.eHistoryCurrentMedsGroup[0].eHistory12.PN != "8801015") // None reported value (NEMSIS)
{
    string PertinantNegatives = MyPatientCareReport.eHistory.eHistoryCurrentMedsGroup[0].eHistory12.PN;
    if (PertinantNegatives == "8801019" || PertinantNegatives == "8801023")
    {
        // Add a new XML serializable boolean to the CDA object
        BL MyBL = new BL();
        MyBL.nullFlavor = "NI";
        MyCurrentlyOnMedicationObservation.value.Add(MyBL);
    }
    else
    {
        MyCurrentlyOnMedicationObservation.value.Add(new BL(true));
    }
}
else
{
    MyCurrentlyOnMedicationObservation.value.Add(new BL(false));
}

To me this look ugly. Should I be doing this a different way? If so, how?

Your problem isn't null checks, it's a lack of encapsulation. This code is rifling through the metaphorical pockets of myPatientCareReport, like an overly familiar paperboy [PDF].

Let's look at what it knows:

myPatientCareReport has a field eHistory
eHistory has a field eHistoryCurrentMedsGroup
eHistoryCurrentMedsGroup is an array (or at least has an indexer)
the order of elements in eHistoryCurrentMedsGroup is meaningful
the elements of eHistoryCurrentMedsGroup have a field eHistory12
eHistory12 has a field PN
PN is a string with some opaque meaning (see also: stringly typed)

If any of these implementation details were to change, you would end up rewriting a lot of code.

  In object-oriented programming, encapsulation is the term for not
  only grouping data and behavior, but also hiding data and behavior
  implementation details within a class ... so that the inner workings
  of a class are not exposed. This reduces the chances that callers will
  modify the data inappropriately or program according to the
  implementation, only to have it change in the future. -- Essential C#
  5.0

Perhaps it's time for a Demeter Transmogrifier?

  Code that violates the Law of Demeter is a candidate for Hide
  Delegate, e.g. manager = john.getDepartment().getManager() can be
  refactored to manager = john.getManager(), where the Employee class
  gets a new getManager() method. However, not all such refactorings
  make as much sense. Consider, for example, someone who’s trying to
  kiss up to his boss: sendFlowers(john.getManager().getSpouse()).
  Applying Hide Delegate here would yield a getManagersSpouse() method
  in Employee. Yuck.

Now it's hard to suggest what this could look like because I'm not familiar with the problem domain, and a lot of the names here are meaningless to me (eHistory12? is there an eHistory13?). But you would probably want the code to end up looking something like this:

if (patientReport == null)
{
    ...
    return;
}

var currentMedicationGroup = patientReport.GetCurrentMedicationGroup(0);
currentOnMedicationObservation.Value.Add(GetBoolean(currentMedicationGroup));

private static BooleanValue GetBoolean(MedicationGroup medicationGroup)
{
    if (medicationGroup != null && medicationGroup.HasPertinentNegatives)
    {
        var pertinentNegatives = medicationGroup.GetPertinentNegatives();
        if (pertinentNegatives == PertinentNegatives.Refused ||
            pertinentNegatives == PertinentNegatives.UnableToComplete)
        {
            return new BooleanValue { NullFlavor = NullFlavor.NoInformation };
        }
        else
        {
            return BooleanValue.True;
        }
    }
    else
    {
        return BooleanValue.False;
    }
}


Counting persons in each age range I am calculating the number of people within a given age range. This is my current approach.

Note:

The persons is a List<Person>
The getDiffYears() returns the number of years between now and DOB of the person. It includes the month and day as well.
The method will be returning a HashMap<String, Integer>() (Can be changed if there is a better alternative.)

The code has been tested and is working. I wanted to know if there is a more efficient approach without using this many if statements?

for (Person p: persons) {
 Date dob = p.getDob();
 if (dob != null) {
  years = getDiffYears(p.getDob());
  if (years < 10) {
   a++;
  } else if (years < 20) {
   b++;
  } else if (years < 30) {
   c++;
  } else if (years < 40) {
   d++;
  } else if (years < 50) {
   e++;
  } else if (years < 60) {
   f++;
  } else if (years < 70) {
   g++;
  } else if (years < 80) {
   h++;
  } else if (years < 90) {
   i++;
  } else if (years < 100) {
   j++;
  } else {
   k++;
  }
 } else {
  k++;
 }
}
map.put("00 - 09", a);
map.put("10 - 19", a);
map.put("20 - 29", b);
map.put("30 - 39", c);
map.put("40 - 49", e);
map.put("50 - 59", f);
map.put("60 - 69", g);
map.put("70 - 79", h);
map.put("80 - 89", i);
map.put("90 - 99", j);
map.put("Unknown", k);

Java 8 streams

Aside from the typos pointed out by @SuperBiasedMan, you can use Java 8's stream-based processing as an alternative for generating your Map result.

private static final int UPPER_LIMIT = 100;
private static final String[] KEYS = new String[]{ "00 - 09", /* ... */ "Unknown" };

private static int categorize(int year) {
    return Math.min(year, UPPER_LIMIT) / 10;
}

Map<String, Long> results = persons.stream()
                    .map(Person::getDob)
                    .filter(Objects::nonNull)
                    .map(ThisClass::getDiffYears)
                    .map(ThisClass::categorize)
                    .collect(Collectors.groupingBy(ageCategory -> KEYS[ageCategory],
                                Collectors.counting()));

Define the UPPER_LIMIT of your age categorization and the desired category names.
From a Stream of your Person objects, map() to their birthdays by calling Person::getDob as method reference.
filter() null birthdays by using Objects::nonNull.
map() birthdays to ages by calling your getDiffYears(Date) method. Note that if it is not a static method, the method reference then becomes this::getDiffYears.
map() ages to the age category by calling categorize(int) to get the desired array indices for KEYS. If you have not realized yet, ThisClass is just a placeholder for the actual class name.
collect() the age categories into the desired Map result groupingBy() our categorization and counting() the elements per category.

Other advice

Your method should return a Map instead of HashMap so that callers of this method only need to deal with the interface, and not the implementation.
You may want to consider how to handle for null inputs to getDiffYear(Date). Should it return a negative value?
You can also experiment with deriving the category names in a programmatic manner, instead of using literal Strings.
You can even think about making your age categorization depend on a configurable upper limit (100) and band (10). For example, how easy will it be to treat the upper limit as 101 (UPPER_LIMIT = 101) and a band of 11 years per category (hint: take a closer look at the suggested code...)?


User Confirmation using a Userform Modeless and DoEvents Can this be done more efficiently?

The code does the following:

Do Stuff
Show UserForm and ask user to confirm the shown data is ok
User can manipulate and controll excel as usual
As soon as user clicks the "yes" button "do some other stuff" is called

The Module

Sub ControlDataUI()
Dim Ui As New UserForm1
Dim IsConfirmed As Boolean

Debug.Print "dostuff"

Application.ScreenUpdating=True
With Ui
    .Show (0)
    While Not .IsHiden
        DoEvents
    Wend
    If .IsCancelled Then Exit Sub
    IsConfirmed = .Confirmed
End With

Debug.Print "Do some more stuff!"

If IsConfirmed Then
    Debug.Print "SaveStuff"
End If

Debug.Print "I will die!!"
End Sub

The UserForm

Private Type TView
    IsCancelled As Boolean
    Confirmed As Boolean
    IsHiden As Boolean
End Type

Private this As TView
Public Property Get IsCancelled() As Boolean
    IsCancelled = this.IsCancelled
End Property
Public Property Get Confirmed() As Boolean
    Confirmed = this.Confirmed
End Property
Public Property Get IsHiden() As Boolean
    IsHiden = this.IsHiden
End Property

Private Sub CommandButton1_Click()
Debug.Print "YES!!!!"
this.Confirmed = True
this.IsHiden = True
Me.Hide
End Sub

Private Sub CommandButton2_Click()
Debug.Print "NO?!?!"
this.Confirmed = False
this.IsHiden = True
Me.Hide
End Sub

Private Sub UserForm_Click()

End Sub

Private Sub UserForm_QueryClose(Cancel As Integer, CloseMode As Integer)
this.IsCancelled = True
Me.Hide
End Sub

Private Sub UserForm_Terminate()
Debug.Print "Murder! I was killed!"
End Sub

So the idea was in the right direction but the while loop had to contain a DoEvents in order to process the events comming from the user.

Summary:

1. Modeless Userform

2. While loop with DoEvents until user confirmes

This allows for the user controll of the sheet and a confirmation at any time. The code will run after the user confirms the data is ok.

Yup, it's wrong1 :)

Forms (and literally everything in Windows) already run their own message loop: whenever you feel the need to wrap a form with a no-op "wait until user does something" DoEvents loop, you are essentially forcing your form into a procedural dialog paradigm, keeping the VBA runtime much, much more busy than it needs to be.

Embrace the objects!

The idiomatic way to handle this in VBA is to adopt an event-driven paradigm: instead of wait-looping for a state change and essentially "poll" your form periodically until its .IsHiden (typo?) state changes, have a presenter class that's responsible for dealing with the state of the form, by responding to its events.

Dim Ui As New UserForm1

Instead of a local (auto-instantiated?) object variable, make that a WithEvents variable at module level (that's why you need a class: only class modules can have WithEvents variables).

Option Explicit
Private WithEvents UI As UserForm1

Private Sub Class_Initialize()
    Set UI = New UserForm1
End Sub

Now this class can handle any of the UI events - whether they're inherited from the UserForm base class, or custom-defined on the UserForm1 default interface, like this:

Public Event ByeBye()

Private Sub CommandButton1_Click()
    Debug.Print "YES!!!!"
    this.Confirmed = True
    this.IsHiden = True
    Me.Hide
    RaiseEvent ByeBye
End Sub

Note the indentation is made more consistent (and arguably better, IMO) by adding an indent level for all members.

Doing this allows the presenter class (the one that declares a WithEvents instance of the form) to handle this ByeBye event:

Private Sub UI_ByeBye()
    'form is no longer displayed
End Sub

Note that this is an entirely different paradigm: you can't do stuff in the same procedure that's responsible for showing the form anymore - it forces you to separate the responsibilies!

1 Only because the form isn't modal though: a modal form suspends execution, which makes the explicit loop useless; the form's state can be read on the statement that immediately follows the call to .Show, since that next instruction will only run after the form is dismissed. That isn't the case for a non-modal form, so instead of loop-waiting, we need to embrace the asynchronous nature of the non-modal messenging, with events.


HackerRank (Stacks) - Maximum element solution gets a timeout Apparently my solution is really slow. Can someone help me with why my solution passes a little over half the test cases for this question?

Here is my solution:

function processData(input) {
    //Enter your code here

    let inputArr = input.split("\n");
    let newArr = inputArr.map(x => x.split(" ").map(p => parseInt(p, 10)));
    let stack = [];
    let len = newArr.length;
    for (let i = 0; i < len; i++){
        if (newArr[i][0] === 1){
            stack.push(newArr[i][1]);
        } else if (newArr[i][0] === 2){

            stack.pop();

        } else if (newArr[i][0] === 3){
            console.log(stack.reduce(maxNum));
        }
    }

}

let maxNum = (acc, curr) => Math.max(acc, curr);

Performance review
These code challenges have tight constraints (a very good thing in my view) so you you must be aware of the actual load on the CPU for every line you write.
Also these code challenges run on V8, and sometimes that version is not up to date. It helps to have some knowledge of some of V8's oddities, past and present.
Looking at your code.
The first problem is the parsing of the input. Using Array.map is convenient but slow.
Rather than parse the input before you process it, combine the processing with the parsing. This will save a significant amount of time.
Don't slice arrays if you don't need to, there are better ways to access data in arrays if speed is important.
Same with Array.reduced, there is no need for it in this example.
Use Number("42") to convert to number rather than parseInt as it is quicker. There really is only one use for parseInt and that is to convert numbers from bases other than 10.
The commands (first value per line after first line) are not used as numbers, you don't need to convert them.
RegExps are very quick compared to loops, iterators, and many of the array functions. Its well worth learning RegExps to get the most out of your code.
You can use a RegExp to split the input. input.split(/[^0-9]/g); will split on each non digit and a zillion times faster than using String.split and array.map
Function calls add overhead that is not related to the algorithm. Avoid needless function calls if you can. That does not mean flatten out your code that means dont create indirect function calls if there is no need.
// completely redundant and only adds overhead.
let maxNum = (acc, curr) => Math.max(acc, curr);

V8 specific
Watch out when using let. Some of these code challenge sites are still running on older versions of V8 and let used to add a big overhead especially inside loops.
Careful with indexing indexing. Some JS engines are not as efficient as others when it comes to array indexing and property referencing.
For example V8 will be slower when you reference an array elements using multiple indexes
var c = 0;
for(var i = 0; i < 10; i ++){
   for(var j = 0; j < 100; j ++){
      c += myArray[i][j];
   }
}

You can improve the performance by staying as close to the required index as possible.
var c = 0;
for(var i = 0; i < 10; i ++){
   const innerArray = myArray[i]; // only need to calculate i index 10 times
                                  // rather than 1000 times.

   for(var j = 0; j < 100; j ++){
      c += innerArray[j];
   }
}

Note V8 has made some amazing improvements over the past months. It always pays to test performance when in doubt.
Rewrite
The following will be about the fastest possible. the only performance increase you could get is using a typed array and implementing your own stack (which is worth the effort for larger array sizes)
/* Commands first item is command count
1 x  -Push the element x into the stack.
2    -Delete the element present at the top of the stack.
3    -Print the maximum element in the stack. */
function processData(data) {
    const stack = [];
    const input = data.split(/[^0-9]/g); // RegExp to split the data
    var max, i, inPos = 0; // inPos is data read pos
    var count = Number(input[inPos++]); // You could get away without Number
                                        // but using a number type is faster
    while (count--) { //simplify the loop
       const command = input[inPos++];  // The command is not used as number
       if (command === "1") { // compare to the string not number as its quicker
                            // in this situation
            // Older V8 the following is the fastest way to push to an array
            stack[stack.length] = Number(input[inPos++]);
       } else if(command === "2") {
            stack.pop();
       } else {
            // for and while loops are faster than array methods.
            i = stack.length;
            // you dont know if length could be zero
            // but it says nothing about what to do for no value so I am
            // assuming it safe to get max from the top.
            max = stack[--i];
            while (i--) {
                // there may be a slight advantage to using the next line
                // but cant remember how much to, indexing stack twice may
                // swing the balance
                // max = max < stack[i] ? stack[i] : max;
                // over calling Math.max
                max = Math.max(max, stack[i]);
            }
            console.log(max);
       }
    }
}

I find expanded and commented code hard to read so below is a neater version in my prefered style.
function processData(data) {
    const stack = [];
    const input = data.split(/[^0-9]/g);
    var count, max, i, inPos = 0;
    count = Number(input[inPos++]);
    while (count--) {
       const command = input[inPos++];
       if (command === "1") { stack[stack.length] = Number(input[inPos++]) }
       else if (command === "2") { stack.pop() }
       else {
            i = stack.length;
            max = stack[--i];
            while (i--) { max = Math.max(max, stack[i]) }
            console.log(max);
       }
    }
}


Sorting an object by value w/ `sort` and `reduce` Given an object with unsorted numeric values, such as:
const unsorted = {
  green: 80,
  blue: 90,
  red: 30,
  yellow: 100,
}

We want a data structure with the same key/value pairs sorted in descending order of the values, e.g.:
const sorted = {
  yellow: 100,
  blue: 90,
  green: 80,
  red: 30,
}

My solution:
function sortObjectByValue (obj) {
  const sorted = Object.keys(obj)
    .sort((a, b) => obj[b] - obj[a])
    .reduce((acc, cur) => {
      acc[cur] = obj[cur]
      return acc
    }, {})

  return new Map(Object.entries(sorted))
}

What I hope could be improved:

Two iterations, first sort then reduce; this seems inefficient.
Using Map. I wish we could just use objects, it would make using this elsewhere much easier. I doubt this can be avoided though, as we need to ensure that the order is preserved.

Here's my proposal:
function sortObjectByValue (obj) {
  const map = new Map()

  Object.entries(obj)
    .sort((a, b) => obj[b][1] - obj[a][1])
    .forEach(([key, value]) => {
      map.set(key, value)
    });

  return map;
}

You don't need to create a sorted object unless you want to use it, I understand that you are using a Map so that you can preserve the order of the elements, but keep in mind this:

Note: Since ECMAScript 2015, objects do preserve creation order for
string and Symbol keys. In JavaScript engines that comply with the
ECMAScript 2015 spec, iterating over an object with only string keys
will yield the keys in order of insertion.

Source: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map
So with that in mind we can skip the Map and we have something like this:
function sortObjectByValue2 (obj) {
  return Object.keys(obj)
    .sort((a, b) => obj[b] - obj[a])
    .reduce((acc, cur) => {
      acc[cur] = obj[cur]
      return acc
    }, {})
}


ASCII Random Walk Algorithm I would like some advice on improving/optimizing a small program I wrote. What it does is run a random walk on a torus manifold with 8 colors that loop around.
//RTM Test Using Random Walk Fractals, WIP
//Created by delta23
//This Code works on Minix, Unix, MacOSX, Linux
//This Code does not work on DOS or Windows (yet...)
//LICENSE OF CODE: CC0 - Public Domain

#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#define TAPE_SIZE_X 672
#define TAPE_SIZE_Y 135
//Q!><^V became ><^V : move and add
char inst [ 4 ] = { '>', '<', '^', 'V' } ; //can be used for debugging, and for inputting code later on, when it starts in a conditional loop perhaps
unsigned char tape [ TAPE_SIZE_X ] [ TAPE_SIZE_Y ] ;
int x = 336 ;
int y = 67 ;

void main ( void ) {
    system ( "clear" ) ; //clear screen
    srand ( time ( NULL ) ) ; //initialize random generator; TODO: port to C++ and use TRNG instead
    printf ( "\e[?25l" ) ; //disable cursor
    //TODO: Implement program input and conditional commands
    while ( 1 ) {
        switch ( inst [ rand ( ) % 4 ] ) { //MOVE
        case '>' :
            x = x + 1 ;
        break ;
        case '<' :
            x = x - 1 ;
        break ;
        case '^' :
            y = y + 1 ;
        break ;
        case 'V' :
            y = y - 1 ;
        break ;
        }
        if ( x >= TAPE_SIZE_X ) x = 0 ; //TOROID CODES FOR OOB CURSOR
        else if ( x < 0 ) x = TAPE_SIZE_X - 1 ;
        if ( y >= TAPE_SIZE_Y ) y = 0 ;
        else if ( y < 0 ) y = TAPE_SIZE_Y - 1 ;
        tape [ x ] [ y ] = tape [ x ] [ y ] + 1 ; // ADD 1
        if ( tape [ x ] [ y ] == 8 ) tape [ x ] [ y ] = 0 ;
        usleep ( 16667 ) ; // CAP SPEED TO 60FPS : 1/60 seconds = 16666.666... microseconds
        printf ( "\33[%d;%dH", y, x ) ; //print @ cursor position
        switch ( tape [ x ] [ y ] ) { //Color Code for Mem
        case 0 :
            printf ( "\x1B[40m " ) ;
        break ;
        case 1 :
            printf ( "\x1B[41m " ) ;
        break ;
        case 2 :
            printf ( "\x1B[43m " ) ;
        break ;
        case 3 :
            printf ( "\x1B[42m " ) ;
        break ;
        case 4 :
            printf ( "\x1B[46m " ) ;
        break ;
        case 5 :
            printf ( "\x1B[44m " ) ;
        break ;
        case 6 :
            printf ( "\x1B[45m " ) ;
        break ;
        case 7 :
            printf ( "\x1B[47m " ) ;
        break ;
        }
        fflush ( stdout ) ; //flush stdout to prevent abnormal lag in printing to screen
    }
}
```

General Observations
Welcome to code review, nice first question. I would leave the licensing information out since stack exchange uses the Creative Commons Attribution-ShareAlike license.
The comment block at the top is rather helpful otherwise. FYI, this compiles fine on Windows 10 in Visual Studio 2019 Professional, but doesn't link (usleep() is undefined).
Some of the code is a little difficult to read as there is vertical crowding, you might want to insert a few blank lines inside main() between logic blocks.
Avoid Global Variables
It is very difficult to read, write, debug and maintain programs that use global variables. Global variables can be modified by any function within the program and therefore require each function to be examined before making changes in the code. In C and C++ global variables impact the namespace and they can cause linking errors if they are defined in multiple files. The answers in this stackoverflow question provide a fuller explanation.
Declare and Initialize the Variables Where They are Needed
In the original version of C variables needed to be declared at the top of the logic block where they were used, this is no longer true, declare them as necessary.
void main(void) {
    char inst[] =
    {
        '>',
        '<',
        '^',
        'V'
    };

    system("clear"); //clear screen
    srand(time(NULL)); //initialize random generator; TODO: port to C++ and use TRNG instead
    printf("\e[?25l"); //disable cursor
    //TODO: Implement program input and conditional commands

    unsigned char tape[TAPE_SIZE_X][TAPE_SIZE_Y];
    int x = 336;
    int y = 67;

    while (1) {
        ...
    }
}

Let the Compiler Do More of the Work
In the code example above there is no size defined for the array inst[], the compiler will fill this in based on the number of initialization values in the array. This makes it easier to write and maintain the code since the size doesn't need to be modified each time an instruction is added.
Magic Numbers
There are Magic Numbers in the main() function (336 and 67), it might be better to create symbolic constants for them to make the code more readable and easier to maintain. You already define symbolic constants for TAPE_SIZE_X and TAPE_SIZE_Y the code would be more readable and understandable if the initialization values for x and y used symbolic constants, in this case you could possibly make the initialization half of the tape size for x and y.
Numeric constants in code are sometimes referred to as Magic Numbers, because there is no obvious meaning for them. There is a discussion of this on stackoverflow.
Complexity
The function main() is too complex (does too much). As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.
There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

There are at least multiple functions in main().

//TODO: Implement program input and conditional commands
Set up the screen
The contents of the while (1) loop also seem to be multiple functions

Default Cases for switch Statements
While it isn't necessary in the current code, to prevent undefined behavior it is best to have a default case for every switch statement:
        switch (inst[rand() % 4]) { //MOVE
        case '>':
            x = x + 1;
            break;
        case '<':
            x = x - 1;
            break;
        case '^':
            y = y + 1;
            break;
        case 'V':
            y = y - 1;
            break;
        default :
            printf("Unknown instruction in switch\n");
            return 1;
        }

Possible Optimization
The second switch statement could be replaced with a table of values and that would improve performance.
        char* control_sequence[] =
        {
            "\x1B[40m ",
            "\x1B[41m ",
            "\x1B[43m ",
            "\x1B[42m ",
            "\x1B[46m ",
            "\x1B[44m ",
            "\x1B[45m ",
            "\x1B[47m ",
        };

        printf("%s", control_sequence[tape[x][y]]);
        fflush(stdout); //flush stdout to prevent abnormal lag in printing to screen

Readability and Maintainability
As mentioned in the general observations, this section of code is very hard to read and even harder to maintain:
        if (x >= TAPE_SIZE_X) x = 0; //TOROID CODES FOR OOB CURSOR
        else if (x < 0) x = TAPE_SIZE_X - 1;
        if (y >= TAPE_SIZE_Y) y = 0;
        else if (y < 0) y = TAPE_SIZE_Y - 1;
        tape[x][y] = tape[x][y] + 1; // ADD 1
        if (tape[x][y] == 8) tape[x][y] = 0;
        usleep(16667); // CAP SPEED TO 60FPS : 1/60 seconds = 16666.666... microseconds
        printf("\33[%d;%dH", y, x); //print @ cursor position

In languages like C, C++ or Java put action code such as x = 0; on its own line, preferably in braces, that makes the action easier to find, and with the braces easier to maintain by adding more statements later:
        if (x >= TAPE_SIZE_X)
        {
            x = 0;
        }
        else if (x < 0)
        {
            x = TAPE_SIZE_X - 1;
        }
        if (y >= TAPE_SIZE_Y)
        {
            y = 0;
        }
        else if (y < 0)
        {
            y = TAPE_SIZE_Y - 1;
        }

        tape[x][y] = tape[x][y] + 1; // ADD 1
        if (tape[x][y] == 8)
        {
            tape[x][y] = 0;
        }

        usleep(16667); // CAP SPEED TO 60FPS : 1/60 seconds = 16666.666... microseconds
        printf("\33[%d;%dH", y, x); //print @ cursor position

An example of how hard the original code is to read is that I missed the magic number 8 in the discussion of magic numbers above.


Binary search function in C# I was wondering if I can make this better:

public int BinarySearchIterative(int[]inputArray, int Key) {
        int min = 0;
        Array.Sort(inputArray);
        int max = inputArray.Length;
        return BinarySearchIterative(inputArray,Key,min,max,0);
    }

    public int BinarySearchIterative(int[] inputArray, int key, int min, int max,int LoopCount) {
        while(min <= max) {
            int mid = (min + max) / 2;
            if(key == inputArray[mid]) {
                return ++mid;
            } else if(key < inputArray[mid]) {
                max = mid - 1;
            } else {
                min = mid + 1;
            }
            LoopCount++;
        }
        return LoopCount;
    }

This code is wrong in a great many ways.

public int BinarySearchIterative(int[]inputArray, int Key) {

The name describes how the code is implemented, not what it does. The caller doesn't care how your search is implemented. The caller cares about what the contract of the method is: what goes in, what comes out, and what the relationship between those two things is.

Non-standard spacing.

Formals should start with a lower-cased letter.

Why an array?  What if we want to binary search a list?  Why ints?  What if we want to binary search an array of doubles?  This signature is needlessly specific.

Why return int?  What if the value is not found; what value will you return then?  Are you returning the value, or the index?  This is nowhere clear in the signature, and therefore it needs a comment.

    int min = 0;

What is the purpose of this assignment to a variable that is only read once?  You could say const int min = 0; or simply pass zero. Or not pass it at all; if you always pass zero then the callee doesn't need to have it passed, does it?

    Array.Sort(inputArray);

Someone handed you an array and you sorted it for them. What if it was already in an order they liked? You just wrecked their order. That's rude.  What if it was already sorted? Then you just spent time sorting an array that was already sorted.

Consider what the contract of this method should be; should it be that the array is already sorted?

    int max = inputArray.Length;

The code crashes if the input array is null.  You should detect that situation early and throw an invalid argument exception.

Again giving a name to a value that you only use once. Seems unnecessary.

    return BinarySearchIterative(inputArray,Key,min,max,0);

Non-standard spacing.

Why do min and max get names but whatever that zero is does not?

public int BinarySearchIterative(int[] inputArray, int key, int min, int max,int LoopCount) {

Why is this public? It appears to be a private implementation detail of the other method.

Again with the bad signature.

And again with the nonstandard spacing choices.

    while(min <= max) {
        int mid = (min + max) / 2;

A problem that is in practice a non-problem, but still deserves to be called out.  If min + max is greater than int.MaxValue, this wraps around to a negative number, which you then divide by 2.

This is a non-problem in practice because in C# an array is pretty much never observed to have more than a billion items. But to signal to the reader that you have thought this sort of thing over, you might want to use a different expression that gives the midpoint without overflowing. Can you devise such an expression?

        if(key == inputArray[mid]) {
            return ++mid;

So there you are debugging your program and stepping through the code, and you're watching the value of mid and suddenly it changes.  By doing this you make the person doing the debugging -- you -- have to remember whether they're watching the original value or the new value.  Don't make yourself think.  Don't mutate variables unnecessarily.  Either return mid; or return mid + 1;, whichever one is correct. Don't mutate a variable you're never going to read again in order to save a couple of keystrokes.

        } else if(key < inputArray[mid]) {
            max = mid - 1;

Again, mutating your formals is a bad practice that makes it hard to debug your program. Don't do it.

        } else {
            min = mid + 1;
        }
        LoopCount++;
    }
    return LoopCount;
}

What the heck is "loopcount"?

Why is it capitalized?

Why is this the return value of the method?

What connection does it have with the problem of binary searching?

Suppose the search fails after ten loops. So you return 10.  Now suppose the search succeeds and finds the element 10 is the key, so you return 10. How does the caller know whether the search succeeded or not?

Why is the count passed in at all, if you're always going to pass zero? If you want to count loops then make a local variable.

For that matter, why do you pass in min, max, etc, if you could simply compute them?  Why do you have two methods, one of which just calls the other, passing things that the inner method could have computed itself?

Start over.

Start with a clear statement of the problem.

My method takes as it argument an IList<int> and a target int. The list must be sorted and non-null. It returns null if the target is not in the list; it returns the index of the target if it is in the list.

Now that you have a statement of the problem, write the signature:

public static int? SearchSortedList(IList<int> list, int target)

Now you have everything you need right in this method already to implement binary search, so take it from here. There's no need for an auxiliary method, and there's no need for a loop count. Can you write the body of this method correctly?


Rename your variables and functions to be more specific and do not keep unneeded array data, like so:

def createDigits():
    global digits
    digits = []
    for i in n:
        digits.append(int(i))
    return digits

def calculateProduct(startPosition):
   global digits
   global windowSize
   factors = digits[startPosition: startPosition+windowSize]
   product = 1
   for digit in digits:
       product = product * digit
   return product

createDigits()
maxProduct = 1
windowSize = 13
for i in xrange(0, len(digits - windowSize))
   product = calculateProduct(i)
   if product > maxProduct:
      maxProduct = product

EDIT: Also, might as well not scan products at the end when the window starts to shrink, since they are guaranteed to be less than the last complete window.


Dictionary Parser Class I'm working on a parser library for substituting strings in dictionaries, with things like globals etc. Wanting to get opinions on the direction I'm taking and where I can improve the script. This currently works

class DictParser(object):
    def __init__(self, dictionary):
        self.working_dict = dictionary

    def __traverse(self, obj, callback=None):
        if isinstance(obj, dict):
            return {k: self.__traverse(v, callback) for k, v in obj.items()}
        elif isinstance(obj, list):
            return [self.__traverse(elem, callback) for elem in obj]
        else:
            if not callback:
                return obj
            else:
                return callback(obj)

    def replace_with_global_var(self, pattern):
        pattern_variable = re.compile(pattern)

        def substitute(x):
            if x.group(1) in globals():
                return globals()[x.group(1)]
            else:
                raise IndexError('Missing Global: %s' % x.group(1))

        def transformer(item):
            if isinstance(item, str):
                return pattern_variable.sub(substitute, item)
            else:
                return item

        self.working_dict = self.__traverse(self.working_dict, callback=transformer)

    def retrieve(self):
        return self.working_dict

Usage:

a = 'blah'

my_dict = {
    'test': '((a))',
    'another': 'my ((a)) test',
    'nested': {
        'child': 'here is ((a)) ((a))'
    },
    'boo': 1,
    'test': False
}

d = DictParser(my_dict)
d.replace_with_global_var(r'\(\((.+?)\)\)')
print(d.retrieve())

This was a reasonably quick hack together, tried using repl so I could print the d variable directly but didn't quite work out.

I would have used str.format together with a transparent dictionary to achieve this result. It gives you the familiarity of the format syntax and not having to come up with the regex on your own. This comes at the cost of not being able to specify the regex anymore at all.

The transparent dict is taken from this answer and is needed because otherwise format raises a KeyError when trying to do a partial replace.

from collections import defaultdict

class SafeDict(dict):
    def __missing__(self, key):
        return '{' + key + '}'

def parse(val, vars, callback=None):
    if callback:
        val = callback(val)
    if isinstance(val, dict):
        return parse_dict(val, vars, callback)
    elif isinstance(val, list):
        return parse_list(val, vars, callback)
    try:
        return val.format(**SafeDict(vars))
    except Exception:
        return val

def parse_list(l, vars, callback=None):
    parsed = type(l)()
    for val in l:
        parsed.append(parse(val, vars, callback))
    return parsed

def parse_dict(d, vars, callback=None):
    parsed = type(d)()
    for key, val in d.items():
        parsed[key] = parse(val, vars, callback)
    return parsed

parse_dict and parse_list could be replaced by the one-liners you use. This way, however, they return the same data type as was originally there (this is relevant for sub-classes of list or dict). If you don't need this, just replace them with what you've got.

usage:

a = 'blah'
my_dict = {'test': '{a}',
           'another': 'my {a} test',
           'nested': {'child': 'here is {a} {a}'},
           'boo': 1,
           'test2': False}
print(parse(my_dict, globals()))

When adding more parsers for other data types than list or dict (their sub-classes are already included), you would probably define a mapping from type to parsers to avoid repetition in parse.

As an alternative you should have a look at f-strings, which will be included in Python 3.6, scheduled to be released soon.

With this you could do:

a = 'blah'
table = "Users"
row = 42
parsed_dict = {'test': f'{a}',
               'another': f'my {a} test',
               'nested': {'child': f'here is {a} {a}'},
               'boo': 1,
               'test2': False,
               'complex': f'result: {db_lookup(table, row))}'}


WebApi action that updates entity using stored procedure I have ASP.NET WebApi project. Some of controller has method SetAsMain(int id) that get information about people then use stored procedure update people. How can I improve it? Should I do check people.IsMain == true in stored procedure?

        public HttpResponseMessage SetAsMain(int id)
        {
            People people = repository.GetById(id);

            if (people == null)
            {
                return ErrorMsg(HttpStatusCode.NotFound, string.Format("No people with ID = {1}", id));
            }

            if (people.IsMain == true)
            {
                return ErrorMsg(HttpStatusCode.OK, string.Format("{0} is already set as main", people.Houses.ToString()));
            }

            try
            {
                var parameters = new Dictionary<string, string>();
                parameters.Add("PeopleID", id.ToString());

                repository.ExecProcedure("usp_PeopleSetMain", parameters);
                return Request.CreateResponse(HttpStatusCode.OK, repository.GetById(id)); // some problem here
            }
            catch (Exception ex)
            {
                return ErrorMsg(HttpStatusCode.InternalServerError, ex.Message);
            }
        }

You don't have a people, you have a person.
Saying if (person.IsMain == true) is exactly equivalent to if (person.IsMain)

Updated code below.

public HttpResponseMessage SetAsMain(int id)
{
    People person = repository.GetById(id);

    if (people == null)
    {
        return ErrorMsg(HttpStatusCode.NotFound, string.Format("No people with ID = {1}", id));
    }

    if (person.IsMain)
    {
        return ErrorMsg(HttpStatusCode.OK, string.Format("{0} is already set as main", person.Houses.ToString()));
    }

    try
    {
        var parameters = new Dictionary<string, string>();
        parameters.Add("PeopleID", id.ToString());

        repository.ExecProcedure("usp_PeopleSetMain", parameters);
        return Request.CreateResponse(HttpStatusCode.OK, repository.GetById(id));
    }
    catch (Exception ex)
    {
        return ErrorMsg(HttpStatusCode.InternalServerError, ex.Message);
    }
}

I would also recommend changing the name of the repository method that returns a person from GetById to GetPersonById. As it is currently, the method name gives us no clue what's going on. I had to figure it out from context.


GLSL atomic float add for architectures with out atomic float add //MEM = initial memory, INDATA, the actual floating point value
#define atomicAddFloatX(MEM, INDATA)\
{\
    float data = INDATA;\
    float prev_mem = uintBitsToFloat(MEM);\
    float exchange_mem = prev_mem;\
    float input_mem = (data + prev_mem);\  //initially this is what we assume we want to put in here.
    while (true){\
        uint exchange_memuint = atomicExchange(MEM, floatBitsToUint(input_mem));\ //exchange with memory location, assume other threads are doing the same exact thing. atomic exchange returns the value that was already there.
        exchange_mem = uintBitsToFloat(exchange_memuint);\ //get the floating point value from returned value.
        if (prev_mem != exchange_mem){\  //if it was equal to what we initially suspected, we can exit, our data is guaranteed to be in this memory location for all other threads trying to perform this function.
            input_mem = exchange_mem + data;\ //otherwise we need to update our input. Another thread completed their atomic exchange before us.
            prev_mem = exchange_mem;\ // update our expectation of return now.
        } else {\
            break;\
        }\
    }\
}

//example
atomicAddFloatX(output[thread_id].x,  3.0);

Vulkan supports atomic float, but only for Nvidia at the moment.  It's possible however that other vendors will never support atomic float. In order to provide atomic float functionality, which is absolutely required for some algorithms, I tried my best to try to create something that would have the same effect, if less performant.
I have access to  #extension GL_KHR_memory_scope_semantics, but it is unclear if that can help me or not. I think I have bugs in how I'm dealing with loading the initial memory logically, though through internal tests in projects where this is used this seems to not have errors, but that may be a fluke of the Nvidia memory model.  I'm currently using this for the immersed boundary method for fluid solid coupling, its used to accumulate forces in a part of the algorithm.
I'm also not sure if there's other things I can do to speed this up, though I think this has serialized performance characteristics, which should be equivalent to atomic float normally.

I did not realize what atomicCompSwap did nor that it was common for this kind of usecase: https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/atomicCompSwap.xhtml .  This function handles the case of the loaded value for MEM being read incorrectly, this check is automatically handled via the function itself, and simplifies the code.  Using the original value of memory we can verify if our assumptions about what we should be adding to are correct:
#define atomicAddFloatX(MEM, INDATA)      \
{                                         \
    uint expected_mem = MEM;              \
    float input_mem = (uintBitsToFloat(MEM) + INDATA); \  //initially this is what we assume we want to put in here.
    uint returned_mem = atomicCompSwap(MEM, expected_mem, floatBitsToUint(input_mem)); \ //if data returned is what we expected it to be, we're good, we added to it successfully
    while(returned_mem != expected_mem){ \ // if the data returned is something different, we know another thread completed its transaction, so we'll have to add to that instead.
        expected_mem = returned_mem; \
        input_mem = (uintBitsToFloat(expected_mem) + INDATA) \
        returned_mem = atomicCompSwap(MEM, expected_mem, floatBitsToUint(input_mem)); \
    } \
}

Basically the code I wrote before was attempting to do something like this, but more confusing and a much less safe way.