GroupBy over a clustered iterator in Scala Here's a solution for a lazy groupBy when the iterator contents are assumed to be clustered (repeated elements next to each other) over the key:

def clusteredGroupBy[B](h: Iterator[B])(f: B => _): Stream[Iterator[B]] = {
  if (h.hasNext) {
    val firstValue = h.next()
    val projection = f(firstValue)
    val (head, tail) = h.span(f(_) == projection)

    (Iterator(firstValue) ++ head) #:: clusteredGroupBy[B](tail)(f)
  } else Stream.empty
}

Any suggestions on how to improve this? Is there anything more native to Scala that does the same?

I'd like to add a completely different answer. The task can be very well solved with a concept called pipes or conduits (also iteratees).

I'll give a solution using my experimental library called scala-conduit (disclaimer: I'm the author).

A Pipe[I,O,R] is a thing that can read input elements of type I from one side (request) and emit output elements of type O on another side (respond). When it finishes, it produces a final result of type R. It decides on its own when it requests and when it responds. This is ideal for the task - we receive inputs and decide when to emit the cluster of equal elements. A pipe also knows when the input finishes. This was problematic with using Streams or other native Scala collections, because folding, mapping, etc. don't notify the function that it hit the end. We didn't know easily when to output the last accumulated buffer. With conduit it is easy, our pipe will get notified when it runs out of input. The solution would look like this:

import conduit._
import conduit.Pipe._
import conduit.Util._

def cluster[A](implicit eq: Equiv[A]): Pipe[A,Seq[A],Unit] = {
  // no finalizers used here, import the empty implicit
  implicit val fin = Finalizer.empty;

  def loop(xs: Seq[A]): Pipe[A,Seq[A],Unit] =
    request(
      y => xs match {
        case Seq(x, _*) if eq.equiv(x, y) => loop(xs :+ y);
        case Seq()                        => loop(Seq(y));
        case _                            => respond(xs, loop(Seq(y)))
      },
      // When upstream finishes producing output,
      // we just respond the buffer and finish.
      _ => if (xs.isEmpty) done else respond(xs)
    )

  loop(Seq.empty);
}

The elements accumulated so far are kept in loop's argument xs. When we hit an non-equal element, we respond the buffer and start with a new one. When we hit the end, we respond the buffer (if it's non-empty).

Then we can construct a source pipe from an interator and combine the result into a collection:

def runCluster[A](xs: Iterator[A])(implicit eq: Equiv[A]): Seq[Seq[A]] =
  runPipe(fromIterator(xs) >-> cluster >-> toCol);

println(runCluster(Stream(1,1,2,2,2,3,4,4,4,5).iterator));

Processing of pipes is single-threaded and fully lazy. A pipe is processed only if its downstream pipe requests an input and only until it produces its output, then it's suspended again. And if a pipe terminates (means stops receiving input), its upstream pipes are notified and any registered finalizers are executed.

For example we could read the source values from a file, process it with cluster and do something with the clustered output. And if the right-most (sink) pipe decided to terminate early, the file would be automatically closed, without reading unneeded input.

(We can't convert a pipe to an Iterator or to a lazy Stream. We must run it and extract its final, definite output. The reason is that if extracted values lazily, we wouldn't be able to detect when no more values are requested and finalizers should be run. However this is not a big problem, we simply build the whole computation from Pipes.)


Python 3 - Calculator This is my second Python beginner program done without help. I would like to receive comments on how clean does it look and if it can be done more efficiently.

def user_operation():
    return input('Type operation: ')

def user_number():
    """Return numbers used in operations"""
    n1 = input('First number: ')
    n2 = input('Second number: ')
    return (int(n1), int(n2))

#Operations
def add(x, y):
    a = x + y
    print ('%d + %d is %d' % (x, y, a))
def sub(x, y):
    s = x - y
    print ('%d - %d = %d' % (x, y, s))
def mult(x, y):
    m = x * y
    print('%d * %d is %d' % (x, y, m))
def div(x, y):
    d = x / y
    print ('%d / %d is %d' % (x, y, d))

def calculate():
    while True:
        n1, n2 = user_number()
        operation = user_operation()
        if operation.lower() == 'quit':
            return False

        elif operation == '1':
            add(n1, n2)
        elif operation == '2':
            sub(n1, n2)
        elif operation == '3':
            mult(n1, n2)
        elif operation == '4':
            div(n1, n2)
        else:
            print ("That is not an operation!!")

def main():
    print ("Calculator program by Jfelix\n"
           "Available operations:\n1.Addition\n2.Subtraction\n3.Multiplication\n4.Division\n"
           "Type their respective number to perform the selected operation\n"
           "If done with an operation, type 'back'\n"
           "Type 'quit' to end the program.\n"

        )
    calculate()

if __name__ == "__main__":
    main()

Nice first project! It is generally better practice to have functions return values instead of printing them. I added a power function to yours and made it more user friendly by having them type the symbol instead of a number.also you should add square, cube and other roots making the symbol '//'. otherwise it looks good, your code is nicely formatted and looks great.

def user_operation():
    return input('Type operation: ')

def user_number():
    #Return numbers used in operations
    n1 = int(input('First number: '))
    n2 = int(input('Second number: '))
    return (n1, n2)

#Operations
def add(x, y):
    a = x + y
    return ('%d + %d is %d' % (x, y, a))
def sub(x, y):
    s = x - y
    return ('%d - %d = %d' % (x, y, s))
def mult(x, y):
    m = x * y
    return ('%d * %d is %d' % (x, y, m))
def div(x, y):
    d = x / y
    return ('%d / %d is %d' % (x, y, d))
def sqr(x,y):
    d = x ** y
    return ('%d ** %d is %d' % (x, y, d))
def calculate():
    while True:
        n1, n2 = user_number()
        operation = user_operation()
        if operation.lower() == 'quit':
            break
        elif operation == '+':
            print (add(n1, n2))
        elif operation == '-':
            print (sub(n1, n2))
        elif operation == '*':
            print (mult(n1, n2))
        elif operation == '/':
            print (div(n1, n2))
        elif operation == '**':
            print (sqr(n1, n2))
        else:
            print ("That is not a valid operation!!")

def main():
    print ("Calculator program by Jfelix\n"
           "Available operations:\n1. Addition (+)\n2. Subtraction (-)\n3. Multiplication(*)\n4. Division (/)\n5. Power (**)"
           "Type their respective character(s) to perform the selected operation\n"
           "Type 'quit' to end the program.\n")

    calculate()

if __name__ == "__main__":
    main()


Resistance is futile, Resistors have been dispatched to model your Resistance Since it was getting boring to always do OOP Languages I decided to dabble in the functional realm of programming.

For that I chose the narrowly missed community-challenge by Edward: Resistor Mania, since it really is a quick-to solve using recursive and functional style.

Challenge description:

  In electronics, two resistors in series have a combined resistance
  \$R_1+R_2\$, and two resistors in parallel have combined resistance of
  \$\displaystyle \frac{R_1 R_2}{R_1 + R_2}\$.  Given an infinite supply
  of \$270\Omega\$ resistors with \$5\%\$ tolerance, write a program to
  describe how to combine them into any arbitrary resistance value.

To make things a little easier for me, I decided to ditch the tolerance for this first prototype. Instead of \$270\Omega\$ I also ask the user to provide the values for our resistors.

The code works as expected for rather simple inputs, and I didn't check for more complicated stuff yet.

The ouptut comes as a wiring help that uses simple "equations" to describe parallel and combining wiring.

Output of some manual tests:

*Main> resistor_mania 60 120
"60.0+60.0"
*Main> resistor_mania 120 60
"120.0||(120.0)"
*Main> resistor_mania 120 120
"120.0"

Code:

module Main where

import System.IO

main = do
    hSetBuffering stdin LineBuffering;
    putStrLn "Please enter the value of resistors we use";
    res_input <- getLine;
    putStrLn "Please enter the value you want to model with these resistors";
    target_value <- getLine;
    putStrLn (show (resistor_mania (read res_input) (read target_value)));

{-
Calculate the remaining resistor-strenght necessary to get to a given target
 when going in parallel to the given existing resistor.
-}
p_resistor :: Fractional a => a -> a -> a
p_resistor = (\existing target -> (1 / ((1 / target) - (1 / existing))));

{-
Resistor_Mania: Create a wiring schema for a given target resistor value
with one single available kind of resistor
-}
resistor_mania :: (Show a, Ord a, Fractional a) => a -> a -> String
resistor_mania resistor target =
    if resistor == target;
    then (show resistor);
    else if resistor < target;
    then (show resistor) ++ "+" ++ (resistor_mania resistor (target - resistor));
    else (show resistor) ++ "||(" ++ (resistor_mania resistor (p_resistor resistor target)) ++ ")";

There's quite a few things that I hope can be improved here. For one I don't like how p_resistor is declared as a lambda, and resistor_mania isn't.
Another thing I don't like is the concatenation of Strings in the control structure and the type-signature of resistor_mania makes me uneasy :/

Guards

Using so many if then and else is really weird in Haskell, I suggest using the guards:

resistor_mania resistor target
    | resistor == target = (show resistor)
    | resistor < target = (show resistor) ++ "+" ++ (resistor_mania resistor (target - resistor))
    | otherwise = (show resistor) ++ "||(" ++ (resistor_mania resistor (p_resistor resistor target)) ++ ")"

This is the standard way of listing mutually exclusive conditions in a function definition.

Semicolons

Haskell uses a two-dimensional syntax, semicolons are not needed and should be omitted as they convey no additional information, just noise.

Prefer mainstream function definitions

p_resistor = (\existing target -> (1 / ((1 / target) - (1 / existing))))

Becomes:

p_resistor existing target = (1 / ((1 / target) - (1 / existing))))

That looks maybe less cool, but surely faster to understand.

Types

  the type-signature of resistor_mania makes me uneasy :/

Haskell is type-centric, so it is good that you are worrying about types.

I deleted your types and asked the compiler what types it would like for your functions.

*Main> :t p_resistor
p_resistor :: Double -> Double -> Double
*Main> :t resistor_mania
resistor_mania :: Double -> Double -> String

I think these suggested types are simpler, and I would use them instead of your bulky type declarations.

Non-closing parenthesis

$ is like a parenthesis but you do not need to close it:

putStrLn $ show $ resistor_mania (read res_input) (read target_value)

This is somewhat subjective though...


Title capitalization exercise from The Odin Project I just finished the book title class exercise for The Odin Project and I would appreciate some honest feedback for my code. The code is intended to be "test-driven learning", and therefore emulates test driven development

Here is the test spec:

require 'book'

describe Book do

  before do
    @book = Book.new
  end

  describe 'title' do
    it 'should capitalize the first letter' do
      @book.title = "inferno"
      expect(@book.title).to eq("Inferno")
    end

    it 'should capitalize every word' do
      @book.title = "stuart little"
      expect(@book.title).to eq("Stuart Little")
    end

    describe 'should capitalize every word except...' do
      describe 'articles' do
        specify 'the' do
          @book.title = "alexander the great"
          expect(@book.title).to eq("Alexander the Great")
        end

        specify 'a' do
          @book.title = "to kill a mockingbird"
          expect(@book.title).to eq("To Kill a Mockingbird")
        end

        specify 'an' do
          @book.title = "to eat an apple a day"
          expect(@book.title).to eq("To Eat an Apple a Day")
        end
      end

      specify 'conjunctions' do
        @book.title = "war and peace"
        expect(@book.title).to eq("War and Peace")
      end

      specify 'prepositions' do
        @book.title = "love in the time of cholera"
        expect(@book.title).to eq("Love in the Time of Cholera")
      end
    end

    describe 'should always capitalize...' do
      specify 'I' do
        @book.title = "what i wish i knew when i was 20"
        expect(@book.title).to eq("What I Wish I Knew When I Was 20")
      end

      specify 'the first word' do
        @book.title = "the man in the iron mask"
        expect(@book.title).to eq("The Man in the Iron Mask")
      end
    end
  end
end

Here if my code:

class Book
  attr_accessor :title

  def title=(book)
    lowercase = ["and", "or", "the", "of", "in", "a", "an"]
    @title = book.capitalize.split(" ").map do |word|
      if lowercase.include? word
        word
      else
        word.capitalize
      end
    end.join(" ")
  end

end

Your code looks good. But I have some suggestions. lowercase array will be created on each method call, so we can just declare it as class level constant (and use %w() syntax):

LOWERCASE = %w(and or the of in a an).freeze

Try to use single quotes if possible.

It is possible to use next inside map block to avoid if/else. And split(' ') is the same as split.

Regarding specs. There's neat RSpec feature called shared_examples(RSpec shared_example).

Here's my suggested refactoring:

class Book
  attr_accessor :title

  LOWERCASE = %w(and or the of in a an).freeze

  def title=(book)
    @title = book.capitalize.split.map do |word|
      next word if LOWERCASE.include?(word)
      word.capitalize
    end.join(' ')
  end

end

RSpec.shared_examples 'capitalize' do |message, text, title|
  let!(:book) { described_class.new }
  let!(:assign_title) { book.title = text }

  it message do
    expect(subject).to eq(title)
  end
end

RSpec.describe Book do
  context 'title' do
    subject { book.title }

    it_behaves_like 'capitalize', 'first letter', 'inferno', 'Inferno'
    it_behaves_like 'capitalize', 'every word', 'stuart little', 'Stuart Little'

    context 'articles' do
      it_behaves_like 'capitalize', 'except "the"', 'alexander the great', 'Alexander the Great'
      it_behaves_like 'capitalize', 'except "a"', 'to kill a mockingbird', 'To Kill a Mockingbird'
      it_behaves_like 'capitalize', 'except "an"', 'to eat an apple a day', 'To Eat an Apple a Day'
    end

    it_behaves_like 'capitalize', 'except conjunctions', 'war and peace', 'War and Peace'
    it_behaves_like 'capitalize', 'except prepositions', 'love in the time of cholera', 'Love in the Time of Cholera'
    it_behaves_like 'capitalize', 'always for "I"', 'what i wish i knew when i was 20', 'What I Wish I Knew When I Was 20'
    it_behaves_like 'capitalize', 'always for first word', 'the man in the iron mask', 'The Man in the Iron Mask'
  end
end


How can I make handling multiple {}.get('value', {}.get('foo')) pretty? I ran into an issue where I was doing this:

dictionary1 = {}
dictionary2 = {}
foo = dictionary1.get('foo', dictionary1.get('bar', dictionary2.get('baz')))

I.e. if the value isn't supplied in one dict, it would fall back on the next and then on the next. In this case dictionary1 has a key that is deprecated that I'm getting in case the new style on isn't there, dictionary2 is completely different. It looks pretty ugly though, the alternative seem to be to break it up across multiple lines like:

foo = dictionary2.get('baz')
foo = dictionary1.get('bar', foo)
foo = dictionary1.get('foo', foo)

Which might be better, though it seems like it could be confusing to read the code in the reverse order. I wonder if there's an alternative more straight forward and prettier solution?

Your code attempts to fetch the values of all keys when it probably only really needs one.  You do not want to be doing this.

I think it would be better to create a list representing the priorities of what values you wish to retrieve and attempt to retrieve them.

def get_value(priorities):
    for key, dictionary in priorities:
        try:
            return dictionary[key]
        except KeyError:
            pass
    return None

priorities = [
    ('foo', dictionary1),
    ('bar', dictionary1),
    ('baz', dictionary2),
]
foo = get_value(priorities)


PHP api design - anything done badly? I have created an api in php and I was hoping some clever individuals would mind reviewing it to see what I could do better. I have commented it heavily to explain my reasoning. please feel free to pick holes wherever you can!

    //On my page 'localhost/api'

    if (!$_POST) {
        die('No direct access allowed');
    }

    include('/api/gateway.php'); //Pull in my Gateway class

    try {
        $api = new Gateway($_POST);
    }
    catch (Exception $e) {
        //Catch any exceptions thrown in the api and return them as a bad request
        echo json_encode( array(
            'status'=> 400,
            'params'=> $_POST,
            'data'=> array(
                'message'=> $e->getMessage(),
                'file'=> $e->getFile(),
                'line'=> $e->getLine()
            )
        ));
    }

My Gateway class then deals with the api call.

class Gateway {

    function __construct($params) {
        $this->params = (isset($params) ? $params : array());

        if ( !isset($params['method']) ) {
            throw new Exception('No method passed to API');
        }

        $method = explode('.', $params['method']);

        if ( !class_exists($method[0]) ) {
            throw new Exception('No class by the name of ' . $method[0]);
        }
        if ( !method_exists($method[0], $method[1]) ) {
            throw new Exception('No method in ' . $method[0] . ' called ' . $method[1]);
        }

        //Call my class and method without instantiating it.
        $result = $method[0]::$method[1]($params);

        if (!$result) {
            //This shouldn't really fire as it should be handled in the method.
            $this->error();
        }
        else {
            $this->send(200, $result, $params);
        }

    }

    public $params; //$_POST parameters stored here to return if an error is thrown

    public function error($code = 400, $data = array(), $params = array()) {
        echo json_encode( array(
            'status'=> 400,
            'params'=> $this->$params,
            'data'=> $data
        ));
    }

    public function send($code = 200, $data = array(), $params = array()) {
        echo json_encode( array(
            'status'=> 200,
            'params'=> $this->params,
            'data'=> $data
        ));
    }
}

Example of my api classes. These are currently in the same file as my Gateway class. Eventually these will be moved into a separate folder and pulled in dynamically.

class Sites {
    public function getSites($params) {
        return array(
            'gotyou'=> 'message'
        );
    }
}

What you are really trying to accomplish here is taking http requests and dispatching them to the correct class. This is much better handled by one of many mvc frameworks in existence. That said, I see several problems with what you have here.

Improper check for POST requests. $_POST is always available
No validation of input. Do you want to allow unrestricted access to classes and methods?
Gateway class (which seems more like a dispatcher) is concerned with too much. Not only is it dispatching the request, it is also formatting responses.
No use of http header to properly instruct the client. Put the status codes (200, 404, etc) in the response header, not in the payload.
You are potentially exposing sensitive information by passing back the Exception object's message. When things go wrong, YOU need to know what and why, but the client does not.

Here is a rough improvement:

GatewayExeption.php

<?php
//Use your own exception to distinguish from others
class GatewayException extends Exception {}

Gateway.php

<?php
class Gateway
{
    /**
     * @var array
     */
    private $apis;

    /**
     * Pass an array of pre-configured api class and actions
     *
     * array(
     *   'Foo' => array('bar', 'baz')
     * )
     *
     * @param array $apis
     */
    public function __construct($apis)
    {
        $this->apis = $apis;
    }

    /**
     * Invoke a pre-configured api
     *
     * @param string $apiName
     * @param string $action
     * @param null|array $args
     * @throws GatewayException
     */
    public function invokeApi($apiName, $action, $args = array())
    {
        if (! array_key_exists($apiName, $this->apis)) {
            throw new GatewayException('Invalid api name');
        }

        if (! in_array($action, $this->apis[$apiName])) {
            throw new GatewayException('Invalid action');
        }

        try {
            $obj = new $apiName;
            $result = call_user_func(array($obj, $action), $args);
            return $result;
        } catch (Exception $e) {
            throw new GatewayException('api error', null, $e);
        }
    }
}

Sites.php

<?php
class Sites
{
    public function getSites($params)
    {
        return array(
            'gotyou' => 'message'
        );
    }
}

index.php

<?php
require 'GatewayException.php'
require 'Gateway.php'

if ( $_SERVER['REQUEST_METHOD'] !== 'POST') {
    header('HTTP/1.0 405 Method Not Allowed');
    exit;
}

//Provide an explicit list of available apis
$validApis = array(
    'Sites' => array('getSites')
);
$gateway = new Gateway($validApis);

//initialize request parameters with defaults
$api = array_key_exists('api', $_POST) ? $_POST['api'] : '';
$action = array_key_exists('action', $_POST) ? $_POST['action'] : '';
$params = array_key_exists('params', $_POST) ? $_POST['params'] : null;

//all responses at this point will be json
header('Content-Type: application/json');

try {
    $result = $gateway->invokeApi($api, $action, $params);
    echo json_encode($result);
} catch (GatewayException $e) {
    header('HTTP/1.0 400 Bad Request');
    echo json_encode( array(
        'status'=> $e->getMessage()
    ));
} catch (Exception $e) {
    header('HTTP/1.0 500 Internal Server Error');
    //you should log $e->getMessage()
    //you probably do not want to reveal the Exception's message
    echo json_encode( array(
        'status'=> 'A system error occurred'
    ));
}


Print all permutations from a given list of strings such that each set of strings is completely unique Problem : Given a list of strings, some of which may be anagrams amongst themselves, print the permutations which can be so constructed so that each permutation has set of strings which is completely unique.

input :  “asda”, “daas”, “dand”, “nadd”

output :  {“asda”, “dand”}, {“daas”, “dand”}, {“asda”, “nadd”}, {“daas”, “nadd”}

input  :  “laptop”, “toplap”, “loptap”, “mouse”

output :  {“laptop”, “mouse”}, {“toplap”, “mouse”}, {“loptap”, “mouse”}

Code :

def is_anagram(value_1, value_2):
    """checks whether the two given strings are anagrams """
    if sorted(value_1) == sorted(value_2):
        return True

def random_set(input_value):
    """returns the set of strings which are completely unique"""
    if len(input_value) <= 2:
        output = input_value
    else:
        output = []
        for i in range(len(input_value)):
            for j in range(len(input_value)):
                if not is_anagram(input_value[i], input_value[j]):
                    output.append( {input_value[i], input_value[j]})

    return output

print random_set(["asda", "daas" , "dand" , "nadd"])
print random_set(["laptop", "toplap", "loptap", "mouse"])

Though the code gives the correct output but it makes duplicate copy of the sets. How can i remove this flaw?

I don't think your current algorithm is that good.

You loop through all input_values twice, not even removing clearly know duplicates.

If you instead used say a half_product function, then you'd remove a lot of checks.
This is as you know if you've already visited the value with i, then you won't need to visit again with j.

This is as set('ab') == set('ba').
You can group the anagrams, to reduce the code complexity.

Currently you have two \$O(n)\$ loops - running in \$O(n^2)\$ time - where \$n\$ is the size of input_values.
After this, you then run in \$O(v\log{v})\$ - where \$v\$ is the max size of the values.
And so is \$O(n^2 v\log{v})\$.

However you can group all the values in \$O(n v\log{v})\$ time.
This means you now have \$g\$ groups, of \$a\$ anagrams. Being roughly \$ag = n\$.
From this you have to loop all groups and anagrams leaving \$O(g^2 a^2)\$ time.
And so is \$O(n v\log{v} + g^2 a^2)\$. Which is better in all cases, except where \$g\$ or \$a\$ are \$1\$.

And so I'd use:

def half_product(it):
    it = list(it)
    for i, value in enumerate(it):
        for j in range(i+1, len(it)):
            yield value, it[j]

def random_set(values):
    d = {}
    for value in values:
        d.setdefault(tuple(sorted(value)), set()).add(value)

    for anagrams in half_product(d.values()):
        yield from itertools.product(*anagrams)


Speech Synthesis to showcase how various voices sound with System.Speech.Synthesis I was wondering if you would be willing to give me some suggestions on shortening this code. I feel as if the amount of if statements I have is a bit much.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Speech.AudioFormat;
using System.Speech.Synthesis;
using System.Text;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;

namespace SpeechTutorial
{
    /// <summary>
    /// Interaction logic for MainWindow.xaml
    /// </summary>
    public partial class MainWindow : Window
    {
        SpeechSynthesizer synth = new SpeechSynthesizer();

        public MainWindow()
        {
            InitializeComponent();
            comboItems();
        }

        private void comboItems()
        {
                foreach (InstalledVoice voice in synth.GetInstalledVoices())
                {
                    // loop through each voice installed on machine
                    VoiceInfo info = voice.VoiceInfo;
                    // adds each installed voice's Name and location to the combobox's list
                    comboBox.Items.Add(info.Description);
                }
            }

        private void button_Click(object sender, RoutedEventArgs e)
        {
            // set the synthesizer to speak
            synth.Speak(textBox.Text);
        }

        private void comboBox_SelectionChanged(object sender, SelectionChangedEventArgs e)
        {
            // here we are setting the voices for each selected item in the combobox

            try
            {
                if (comboBox.SelectedIndex == 0)
                {
                    synth.SelectVoice("Microsoft Hazel Desktop");
                }

                if (comboBox.SelectedIndex == 1)
                {
                    synth.SelectVoice("Microsoft Heera Desktop");
                }

                if (comboBox.SelectedIndex == 2)
                {
                    synth.SelectVoice("Microsoft David Desktop");
                }

                if (comboBox.SelectedIndex == 3)
                {
                    synth.SelectVoice("Microsoft Zira Desktop");
                }

                if (comboBox.SelectedIndex == 4)
                {
                    synth.SelectVoice("Microsoft Haruka Desktop");
                }

                if (comboBox.SelectedIndex == 5)
                {
                    synth.SelectVoice("Microsoft Heami Desktop");
                }

                if (comboBox.SelectedIndex == 6)
                {
                    synth.SelectVoice("Microsoft Huihui Desktop");
                }

                if (comboBox.SelectedIndex == 7)
                {
                    synth.SelectVoice("Microsoft Tracy Desktop");
                }

                if (comboBox.SelectedIndex == 8)
                {
                    synth.SelectVoice("Microsoft Hanhan Desktop");
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }

        }
    }
}

Take a look at arrays. It will allow you to quickly add and remove future options without needing to change the logic of the code. It will also allow you to possibly add more complex logic in a single place, instead of having to add code to every if/switch statements.

string[] options = { "Microsoft Hazel Desktop",
                     "Microsoft Heera Desktop",
                     "Microsoft David Desktop",
                     "Microsoft Zira Desktop",
                     "Microsoft Haruka Desktop",
                     "Microsoft Heami Desktop",
                     "Microsoft Huihui Desktop",
                     "Microsoft Tracy Desktop",
                     "Microsoft Hanhan Desktop"};

try
{
    synth.SelectVoice(options[comboBox.SelectedIndex]);
}
catch (Exception ex)
{
    MessageBox.Show(ex.Message);
}


Scale Numpy array to certain range As I've described in a StackOverflow question, I'm trying to fit a NumPy array into a certain range.

Here is the solution I currently use:

import numpy as np

def scale_array(dat, out_range=(-1, 1)):
    domain = [np.min(dat, axis=0), np.max(dat, axis=0)]

    def interp(x):
        return out_range[0] * (1.0 - x) + out_range[1] * x

    def uninterp(x):
        b = 0
        if (domain[1] - domain[0]) != 0:
            b = domain[1] - domain[0]
        else:
            b =  1.0 / domain[1]
        return (x - domain[0]) / b

    return interp(uninterp(dat))

print(scale_array(np.array([-2, 0, 2], dtype=np.float)))
# Gives: [-1., 0., 1.]
print(scale_array(np.array([-3, -2, -1], dtype=np.float)))
# Gives: [-1., 0., 1.]

Is there a way to make this code cleaner? Is there a built-in function in NumPy or scikit-learn? This feels like a really common data pre-processing step and it feels weird that I keep re-implementing it.

NumPy provides numpy.interp for 1-dimensional linear interpolation. In this case, where you want to map the minimum element of the array to −1 and the maximum to +1, and other elements linearly in-between, you can write:

np.interp(a, (a.min(), a.max()), (-1, +1))

For more advanced kinds of interpolation, there's scipy.interpolate.


Checking if a string is a number with STL Recently me and colleague had a discussion about the following piece of code (simple bool function that checks if string is a number, +1000 not allowed, -1000, 1234 ... allowed).

He felt that it was hackish, while I thought it was nice, clean and elegant (since it uses STL, not hand made loops).
So is this a judgement call or one of us is wrong? Is this code elegant or hack?

 bool validate(const std::string& m)
 {
   if (m.empty())
     return false;
   return m.end() == find_if(*m.begin() == '-'? ++m.begin() : m.begin(),
                                    m.end(), not1(ptr_fun(isdigit)));
 }

I had to really look at your code and think about what it was doing before I figured it out.  That tells me that this code is not elegant by my definition of "elegant".  My definition of elegant is something like,

  Easy to understand, easy to maintain, efficient in execution, robust.

I would suggest something like this as a start:

if( m.empty() )
  return false;

string::iterator start = m.begin();
if( *start == '-' )
  ++start;

if( m.end() == find_if( start, m_end(), not1(ptr_fun(isdigit)) ))
  return true;
else
  return false;

Not much caring for the use of find_if, I'd refine it to:

if( m.empty() )
  return false;

string::size_type pos = 0;
if( m[0] == '-' )
  pos = 1;

if( m.find_first_not_of("0123456789", pos) == m.end() )
  return true;
else
  return false;

I find this to be much more elegant than the 1-liner you wrote.  It's more code, sure.  But it is exceedingly simple to understand and maintain.  You found a bug yourself just by brain-compiling it.  That's what code should look like.


Random value generation for uncertainty analysis I am a beginner in coding, writing my first bigger C++ project (aircraft initial sizing program). This code should be able to analyze the design in a probabilistic manner by using Monte Carlo Simulation. Hence, I have to generate random values for several independent random variables.

As I want to learn, how to program good code, I would be thankful for reviewing the simplified 'random number generation'-part of my code:

settings.h

#ifndef SETTINGS_H
#define SETTINGS_H

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

class settings
{

public:
    struct distInput
    {
        std::string distName;
        double paramA;
        double paramB;
        double paramC;
    };

    std::vector<distInput> distInputs;

    settings();
};

settings.cpp

#include "settings.h"

settings::settings()
{
    //Read input from file, here simplified
    distInput input1 =
    {
        "Normal Distribution",
        100.,
        1.,
        0.
    };

    distInput input2 =
    {
        "Triangular Distribution",
        80.,
        100.,
        109.
    };

    distInputs.push_back(input1);
    distInputs.push_back(input2);
}
//here are more functions

uncertainty.h

#ifndef UNCERTAINTY_H
#define UNCERTAINTY_H

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

class uncertainty
{
public:
    void generateRN();

    std::vector<std::mt19937_64> randomNumberGenerators;
    std::vector<std::vector<double>> randomNumbers;
    settings *mySettingsPt;
    std::vector<settings::distInput> *inputData;

    uncertainty(settings &mySettings, double numbersRV);
};

uncertainty.cpp

#include "uncertainty.h

uncertainty::uncertainty(settings &mySettings, double numbersRV)
    :
    mySettingsPt(&mySettings),
    inputData(&mySettingsPt->distInputs)
{
    for(unsigned int i = 0; i < numbersRV; ++i)
    {
        std::random_device rd;
        std::mt19937_64 mt_tmp(rd());
        randomNumberGenerators.push_back(mt_tmp);
        randomNumbers.resize(numbersRV);
    }
}

void uncertainty::generateRN()
{
    unsigned int numRV = inputData->size();

    for(unsigned int i = 0; i < numRV; ++i)
    {
        std::string name = inputData->at(i).distName;
        double a = inputData->at(i).paramA;
        double b = inputData->at(i).paramB;
        double c = inputData->at(i).paramC;

        if(name == "Normal Distribution")
        {
            std::normal_distribution<> dist(a, b);
            randomNumbers.at(i).push_back(dist(randomNumberGenerators.at(i)));
        }
        else if(name == "Triangular Distribution")
        {
            std::uniform_real_distribution<> dist(0, 1);
            double F = (b - a) / (c - a);
            double U = dist(randomNumberGenerators.at(i));
            if (U <= F)
                randomNumbers.at(i).push_back(a + sqrt(U * (c - a) * (b - a)));
            else
                randomNumbers.at(i).push_back(c - sqrt((1 - U) * (c - a) * (c - b)));
        }
    }
};

main.cpp

#include "settings.h"
#include "uncertainty.h"
#include <iostream>
#include <string>
#include <vector>
#include <random>

int main()
{
    // Initialize objects for computation
    settings mySettings;
    uncertainty myUncertainty(mySettings, mySettings.distInputs.size());

    // Start Computation
    for(unsigned int i = 0; i < 100; ++i)
    {
        myUncertainty.generateRN();
        // Other code, that uses the random values
    }
}

The code works fine, but I never had somebody reviewing my code skills. Improvement suggestions of any kind (How to structure .cpp and h. files, how to write that code more efficient, use of data structures and objects, etc.) are very welcome!

You've clearly spent some time looking into best practices; your code is well separated, readable, logical. Keep up the formatting, and people will be happy to read over your code.

This did jump out at me though: you have two specific string literals "Normal Distribution" and "Triangular Distribution" that are repeated in separate files.. Advisable to avoid multiple instances of a string literal in future, especially in separate files; when code gets large, approaches like that can be time consuming and erroneous.

Think: constants and enums. Your compiler won't pick up on string literal typos like that, which could lead to irritating runtime errors or unexpected behaviour. On the other hand, it will of course alert you to undeclared (mistyped) handles in your code. Much more efficient bug removal.


A program which adds 2 integers in the range of 0 to 10^100 I wrote this program to help myself learn C. It is the result of a few days of hacking things together until it worked.

This program takes 2 integers as inputs in the range of 0 to 10100 and outputs the sum of the inputs.

The code used to generate the final result doesn't really feel that elegant so it would be great to see what could have been done better.

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

int main()
{
    char input[2][100];

    // Reset arrays to 0
    for (int i = 0; i < 100; i++)
    {
        input[0][i] = 0;
        input[1][i] = 0;
    }

    int length[2];

    scanf ("%s%n", &input[0][0], &length[0]);
    scanf (" "); // Catch the newline
    scanf ("%s%n", &input[1][0], &length[1]);

    // Format user input
    for (int i = 0; i < 2; i++)
    {
        // Convert chars to ints
        for (int b = 0; b < length[i]; b++)
        {
            input[i][b] = input[i][b] - '0';
        }
        // Shift input to end of array and fill start with 0
        for (int a = length[i]; a > 0; a--)
        {
            input[i][(99 - length[i]) + a] = input[i][a - 1];
            input[i][a-1] = 0;
        }
    }

    int longest = length[0] > length[1] ? length[0] : length[1];
    char* output = (char*)malloc(sizeof(char) * longest + 1);

    int carry = 0;
    for (int i = 99; i >= (100 - longest); i--)
    {
        int result = input[0][i] + input[1][i] + carry;
        int remainder = result % 10;
        carry = result / 10;
        output[longest - (99 - i)] = remainder;
    }
    output[0] = carry;

    // Don't output a 0 at the start of output
    int output_offset = 0;
    if (output[0] == 0)
        output_offset = 1;

    for (int i = output_offset; i < longest + 1; i++)
        printf("%d",output[i]);

    printf("\n");
    return 0;
}

Here are some things that may help you improve your code.

Break up the code into smaller functions

Right now, everything is done in main. Rather than having everything in one long function, it would be easier to read and maintain if each discrete step were its own function.  Specifically you could divide into input, output and calculation.

Add error checking

If we enter "alpha" and "edward" as integers, the result is 6453621.  That doesn't really make much sense.  Better might be to screen the inputs to make sure they're really decimal digits.

Think of the user

The user runs the program and sees... nothing.  It might be nice to show a user prompt.  Even better, in my view, would be to allow the user to enter the numbers as command line parameters.

Don't leak memory

There is a malloc but no matching free which means that this leaks memory.  The operating system will reclaim that memory when the program ends, but it's best to get into good habits now before you tackle more complex programs.

Eliminate "magic numbers"

This code is littered with "magic numbers," that is, unnamed constants such as 10, 99, 100, etc.  Generally it's better to avoid that and give such constants meaningful names.  That way, if anything ever needs to be changed, you won't have to go hunting through the code for all instances of "99" and then trying to determine if this particular 99 is related to what you're trying to change or not.

Avoid scanf if you can

There are so many well known problems with scanf that you're usually better off avoiding it.  The usual approach is to read in user input into a string using something like fgets to allow specifying the input buffer size (and therefore avoiding buffer overrun vulnerabilities).


Baby Step Giant Step Discrete Log Solver I've made a concurrent implementation of the Big Step Giant Step discrete log solver in Go. I'm pretty new to this branch of math, but it seems slow. Any way to speed it up or is this the limit of this particular algorithm?

package main

import (
    "fmt"
    "math/big"
    "runtime"
)

const increment = 1000
var processors int

func main() {
    a := big.NewInt(7)
    b := big.NewInt(24190)
    m := big.NewInt(65537)

    processors = runtime.NumCPU()
    runtime.GOMAXPROCS(processors)
    fmt.Println("RESULT:", babyGiant(a, b, m))
}

func modInverse(a, b *big.Int) *big.Int {
    return new(big.Int).ModInverse(a, b)
}
func mod(a, b *big.Int) *big.Int {
    return new(big.Int).Mod(a, b)
}
func pow(a, b *big.Int) *big.Int {
    return new(big.Int).Exp(a, b, nil)
}
func mul(a, b *big.Int) *big.Int {
    return new(big.Int).Mul(a, b)
}
func sub(a, b *big.Int) *big.Int {
    return new(big.Int).Sub(a,b)
}

func babyGiant(a, b, m *big.Int) int64 {
    fmt.Println("Beginning Store Generation")
    var k int64 = 1000
    store := make(map[string]int64)
    var i int64
    previous := mod(big.NewInt(1), m)
    for i = 1; i < k; i++ {
        if i % 1e7 == 0 {
            fmt.Println("Storage of", i, "items completed.")
        }
        current := mod(mul(previous, a), m)
        currentString := current.String()
        if _, inMap := store[currentString]; inMap {
            k = i
            break
        }
        store[currentString] = i
        previous = current
    }
    fmt.Println("Store Generation Complete")

    var r int64 = 0 - increment
    rk := big.NewInt(r * k)
    receiver := make(chan int64)

    semiphoreStart := processors
    semiphore := semiphoreStart
    for rk.Cmp(m) <= 0 {
        if semiphore > 0 {
            semiphore -= 1
            r += increment
            go func (receiver chan int64, rGiven, k int64) {
                r := rGiven
                rk := big.NewInt(r * k)
                fmt.Println("Currently at", rGiven, "\nRemaining:", sub(m, rk), "\n")
                for ; r < rGiven + increment+1; r++ {
                    rk = big.NewInt(r * k)
                    current := mod(mul(b, modInverse(pow(a, rk), m)), m)
                    currentString := current.String()
                    val, inMap := store[currentString]
                    if inMap {
                        receiver <- (val + r*k)
                    }
                }
                receiver <- -1
            }(receiver, r, k)
        } else {
            result := <- receiver
            if result != -1 {
                return result
            }
            semiphore += 1
        }
    }
    for ; semiphore < semiphoreStart; semiphore++ {
        if result := <- receiver; result != -1 {
            return result
        }
    }
    return -1
}

The most expensive operation you're performing is calculating a^(rk) every iteration. Since you are doing this in a loop and increasing the exponent by a constant amount each iteration (k), you can replace the exponentiation with a multiplication. This makes your concurrent task function look like this:

        go func(receiver chan int64, rGiven, k int64) {
            r := rGiven
            ark := pow(a, big.NewInt(r*k))
            ak := pow(a, big.NewInt(k))
            // fmt.Println("Currently at", rGiven, "\nRemaining:", sub(m, rk), "\n")
            for ; r < rGiven+increment+1; r++ {
                current := mod(mul(b, modInverse(ark, m)), m)
                currentString := current.String()
                val, inMap := store[currentString]
                if inMap {
                    receiver <- (val + r*k)
                }
                ark.Mul(ark, ak) // a^(rk) * a^(k) = a^((r+1)k)
            }
            receiver <- -1
        }(receiver, r, k)

To test this out, I commented out all the prints (they slow things down a lot) and made a test file named main_test.go containing:

package main

import (
    "math/big"
    "runtime"
    "testing"
)

func BenchmarkBabyGiant(b *testing.B) {
    processors = runtime.NumCPU()
    runtime.GOMAXPROCS(processors)

    b.ResetTimer()
    b.ReportAllocs()

    for i := 0; i < b.N; i++ {
        a := big.NewInt(7)
        b := big.NewInt(24190)
        m := big.NewInt(65537)

        babyGiant(a, b, m)
    }
}

Running this with go test ./ -bench=. before and after shows:

Before:
BenchmarkBabyGiant-8          50     663145338 ns/op    812469942 B/op    213165 allocs/op

After:
BenchmarkBabyGiant-8         100     335030538 ns/op    425100448 B/op     85983 allocs/op

This shows that this technique causes it to finish in just over half the time of the original. Using ReportAllocs also shows that this function is rather memory allocation heavy, so you could likely squeeze out another 10-30% by reusing big.Int instances instead of allocating a new one for every operation, and using something other than a string map for the lookup table (a sorted list that you binary search using sort.Search will likely be significantly faster and will avoid the overhead of converting to a base 10 string).


Project Euler Problem #5 Solution in C++ Project Euler #5: Smallest multiple
2520 is the smallest number that can be divided by each of the numbers from 1 to 10 without any remainder.
What is the smallest positive number that is evenly divisible by all of the numbers from 1 to 20?"

And I'm wondering if my code to solve this problem is good and if there are ways to make it better.
#include "stdafx.h"
#include <iostream>
int main(){
int smallestMultiple = 40;
int i = 10;

while (i<20){
    if (smallestMultiple%i == 0){
        i++;
        continue;
    }
    else{
        i = 10;
        smallestMultiple += 20;
    }
    }
std::cout << smallestMultiple;
}

Code Review
Despite its name, "stdafx.h" isn't a standard header.  It doesn't seem to be needed, as the code compiles and runs without it.
Your indentation is off - perhaps that's a result of how you inserted the code into the question?
The continue is redundant - there's no more code within the loop after the if/else, so it can be omitted.
The code assumes that a result will be found before int overflows.  Remember that INT_MAX may be as small as 32767; perhaps unsigned long may be a better choice, or one of the fixed-width types specified in <cstdint>, perhaps?
Your loop structure would be clearer if you separate out the test from incrementing smallest multiple.  That could look something like this (still using int as our type):
#include <iostream>
#include <climits>

static bool is_multiple_of_all(int n, int max)
{
    for (int i = 10;  i <= max;  ++i) {
        if (n % i != 0) {
            return false;
        }
    }
    return true;
}

int main()
{
    const int target = 20;
    for (int candidate = 2 * target;
         candidate <= INT_MAX - target;
         candidate += target)
    {
        if (is_multiple_of_all(candidate, target)) {
            std::cout << candidate;
            return 0;
        }
    }
    // not found
    return 1;
}

Algorithm
Project Euler problems tend to exercise your mathematical reasoning.  You're expected to find better methods than brute force search to find the solution.  (Hint: think about prime factors).


Converting the first and last name to pig latin The following question was taken from Absolute Java 5th ed. by Walter Savitch:

  Write a program that starts with the string variable first set to your first name and the string variable last  set to your last name. Both names should be all lower- case. Your program should then create a new string that contains your full name in pig latin with the first letter capitalized for the first and last name. Use only the pig latin rule of moving the first letter to the end of the word and adding “ay.” Output the pig latin name to the screen. Use the substring  and  toUpperCase  methods to construct the new name.
  For example, given
  first = "walt";
  last = "savitch";
  the program should create a new string with the text “Altway Avitchsay” and print it.

This is the code that I have written:

    public class Question3 {

    private static String first;
    private static String last;
    private static String newName;

    public static void main(String[] args) {

        Question3 name = new Question3("walt", "savitch");

        System.out.println(name.convertName(first) + " "
                + name.convertName(last));

    }

    public Question3(String lowerCaseFirstName, String lowerCaseLastName) {
        first = lowerCaseFirstName;
        last = lowerCaseLastName;
    }

    private String convertName(String originalName) {
        String firstLetter = originalName.substring(0, 1);
        newName = nameWithoutFirstLetter(originalName) + firstLetter; // move first letter to the end
        newName = capitalizeFirstLetter() + nameWithoutFirstLetter(newName) // capitalize first letter and add "ay" to the end
                + "ay";
        return newName;
    }

    private String capitalizeFirstLetter() {
        String capitalFirstLetter = newName.substring(0, 1).toUpperCase();
        return capitalFirstLetter;
    }

    private String nameWithoutFirstLetter(String name) {
        String restOfName = name.substring(1);
        return restOfName;
    }
}

Structure

Try not to use static variables. static is for when the value of a field is the same across all instances of your object. But this isn't the case with peoples names.

What if you have two people? You would create your people:

    Question3 name = new Question3("walt", "savitch");
    Question3 anotherName = new Question3("john", "smith");

But then, you cannot use them the way you want.

If instead you would rewrite your code like this:

public class Question3 {

    private String first;
    private String last;
    private String newName;

    public static void main(String[] args) {

        Question3 name = new Question3("walt", "savitch");
        Question3 anotherName = new Question3("john", "smith");
        System.out.println(name.convertFullName());
        System.out.println(anotherName.convertFullName());

    }

    public Question3(String lowerCaseFirstName, String lowerCaseLastName) {
        first = lowerCaseFirstName;
        last = lowerCaseLastName;
    }

    public String convertFullName() {
        newName = convertName(first) + " " + convertName(last);
        return newName;
    }

    [...]
}

You can see that you can create however many people you want.

Temporary variables

If you don't need variables, you shouldn't create them. For example:

    String restOfName = name.substring(1);
    return restOfName;

could just be

    return name.substring(1);

Functions

You have a function which removes the first letter from a string, but you do not have one for getting the first letter of a string, but instead write that functionality directly in your convertName function. This seems inconsistent. I would either have a function for both or neither.

I also wouldn't operate on newName directly in capitalizeFirstLetter. None of your other functions (for example nameWithoutFirstLetter) do this, so it is inconsistent and can be confusing.

Also, right now, newName is just a temporary variable, which isn't ideal. You can never be sure what state it has. I would either change the code so it always holds the complete new name (for example Altway Avitchsay), or just get rid of newName completely.


For loop step including last value I've got the following code. I want to loop through my dataset (which is a matrix) in steps of let's say 50.000 and perform biglm on it.

lengthdata <- 100340
stepsize <- 50000

for (i in seq(1, lengthdata, stepsize)){
  if(i+stepsize > lengthdata){
    ## call function (i, lengthdata)
    print(paste(i, " - ", lengthdata))
    }
  else {
   ## call function (i, i+stepsize)
   print(paste(i, " - ", i + stepsize))
    }
}

This is my output:

[1] "1  -  50001"
[1] "50001  -  100001"
[1] "100001  -  100340"

Is there a more effective or cleaner way of coding this? I'm quite unexperienced using R!

You can save some typing if you declare a variable for where your range ends:

for (start.i in seq(1, lengthdata, stepsize)){
  end.i <- if (i + stepsize > lengthdata) lengthdata else i + stepsize
  ## call function (start.i, end.i)
  print(paste(start.i, " - ", end.i))
}

Another improvement would be to compute end.i using the min function (less typing!)

end.i <- min(start.i + stepsize, lengthdata)

As you become an advance user, you might like working with vectors directly:

start.i <- seq(1, lengthdata-1, stepsize)
end.i   <- pmin(start.i + stepsize, lengthdata)

and call your function(s) (e.g. paste) on all using the Map (or mapply)

output <- Map(paste, start.i, " - ", end.i)

rather than use a for loop (even less typing). This is equivalent to doing:

output <- list()
for (i in 1:length(start.i)) {
   output[[i]] <- paste(start.i[i], " - ", end.i[i])
}


smallest number divisible by all numbers from 1 to 20? Project Euler question 5 I'm currently working my way through the questions on Project Euler and I am on question 5. Is this the best possible solution? Any suggestions welcomed!

        List<int> divisors = new List<int>{ 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 };

        int n = 1;

        while (true)
        {
            n++;
            foreach (int d in divisors)
            {
                if (n % d != 0) { break; }
                if (d==20)
                {
                    Console.Write(n);
                    Console.ReadLine();
                }
            }
        }

The most optimal solution would be to calculate the LCM of 1 to 20 directly using the standard Euclidean algorithm.

// calculate GCD using Euclidean algorithm
public long GCD(long a, long b) {
    if (b == 0)
        return a;
    else
        return GCD(b, a % b);
}

// calculate LCM using simple formula based on GCD
public long LCM(long a, long b) {
    var gcd = GCD(a, b);
    return a * b / gcd;
}

// iteratively calculate LCM of 1 through 20
public static void Main(string args[]) {
    long result = 1;

    // loop through each value 1 to 20, and LCM with previous result
    for (long n = 2; n <= 20; n++) {
        result = LCM(result, n);
    }

    // print out the result
    Console.WriteLine(result);
}

In terms of complexity, this algorithm is roughly O(n) for small n.


Multiple for loops in Sudoku solver I have an assignment for writing a Sudoku solver and have achieved the functionality required but a lot of the code is "ugly" and I'm wanting to simplify it but unsure how to.

def hiddenSingleCol(snapshot, cell):
    j = cell.getCol()                           # Get column number for cell
    colCells = snapshot.cellsByCol(j)           # Get all cells in that column
    for num in cell.getAvailNum():              # For each available number for the cell we are looking at
        hidden = True                           # Is it a hidden single? (set hidden as True to start)
        for tempCell in colCells:               # For each cell in the list of cells
            if tempCell == cell:                # If it is our cell, ignore
                pass
            elif num in tempCell.getAvailNum(): # If one of the available num is the same
                hidden = False                  # Set hidden to false
                break
        if hidden:
            cell.setAvailNum(num)               # If hidden is still true, set available num to num
            break

More code snippets:

    for cell in cUnsolved:                          # we're only interested in cells without assigned values
    updateAvailNums(snapshot, cell)             # update the list of potential answers for each cell

for cell in cUnsolved:
    hiddenSingleCol(snapshot, cell)
    hiddenSingleRow(snapshot, cell)
    hiddenSingleBlock(snapshot, cell)

for cell in cUnsolved:
    if (len(cell.getAvailNum()) == 0):          # and if any have no possible numbers, this is a failed solution
        return None
    if (len(cell.getAvailNum()) == 1):          # and if there are any with only one possibility, we do this first
        x = cell.getRow()
        y = cell.getCol()
        break
    elif (len(cell.getAvailNum()) < minNum):    # otherwise we are interested in the cell with the least possible numbers so far
        x = cell.getRow()
        y = cell.getCol()
        minNum = len(cell.getAvailNum())

Not only are the multiple for-loops in here ugly, but I have 2 other versions, slightly altered, for the row and 3x3 blocks. I've tried looking at other code snippets on this but I seem incapable of applying it to my own work.

A for statement can have an else clause which suits your needs perfectly and allows you to eliminate the variable hidden. I would also prefer a simple if instead of the if pass elif construct:

for num in cell.getAvailNum():              # For each available number for the cell we are looking at
    for tempCell in colCells:               # For each cell in the list of cells
        if tempCell != cell and num in tempCell.getAvailNum(): # If one of the available num is the same
            break
    else:
        cell.setAvailNum(num)               # If hidden is still true, set available num to num
        break


Algorithm to Iterate All Possible Strings in Clojure I'm fairly new to Clojure and I struggled a bit coming up with a nice Clojure-y solution for implementing an algorithm for one of my applications. The real issue I'm trying to solve would require a large writeup in order to understand what it's about, so I've come up with a contrived problem statement that's much easier to understand which is quite similar to my original problem.

  Assume you have mapped numbers to letters like so: 1->a, 2->b, 3->c, ... , 26->z

  If you receive input like "4 1 25", you can easily convert this to the string "day" (because 'd' is the 4th letter in the alphabet, 'a' is the 1st, and 'y' is the 25th).

  Now assume you don't have spaces as delimiters. So the input is "4125". This introduces ambiguity. "4125" can be interpreted as "4 1 25", "4 12 5", and "4 1 2 5".

  Write a function that takes a number, and returns all possible strings from these letter codes.

  So if you receive "4125" (the example above), the function would return: dle, day, dabe

Here is what I came up with:

; Generates map {1->a, 2->b, ... , 26->z}
(def code->letter
  (into {}
        (for [x (range 26)]
          [(str (inc x))
           (str (char (+ x (int \a))))])))

(defn possible-strings
  ([number]
   (possible-strings (str number) 0 ""))
  ([number index acc]
   (if (>= index (count number))
     acc
     (flatten (for [[k v] code->letter
                    :when (.startsWith number k index)]
                (possible-strings number (+ index (count k)) (str acc v)))))))

; Print all the possible strings from the input "4125"
(doseq [s (possible-strings 4125)]
  (println s))

So I believe this is basically a backtracking algorithm. And because it's a backtracking algorithm, it's impossible to make this tail-recursive, correct? Or maybe I'm wrong that this is a backtracking algorithm. If so, what would this kind of solution be classified as?

Anyway, the algorithm seems to work, but I'm not sure this solution is very idiomatic. The part in particular that I really don't like is having to call flatten for each call. I can't imagine that being very efficient either for large inputs. It seems like there must be a better way to do handle that.

Like I said, I'm new to Clojure and functional programming in general, so if there are improvements elsewhere, I'd be happy to hear about them as well. A completely different approach to solving the problem is welcome as well.

I figured out a faster, more functional programming-oriented algorithm for this problem -- see below. Besides that, your code overall looks idiomatic and was easy for me to understand, so I have no real criticisms in that area.

zipmap

zipmap is ideal for concisely constructing maps like your code->letter map:

(def code->letter
  (zipmap (map str (iterate inc 1)) (map char (range 97 123))))

;=> {"1" \a, "2" \b, "3" \c, ... "26" \z}

One way to avoid using flatten

Personally, I think using flatten is fine, but some view it as being somewhat hacky, and I've heard it said that if you find yourself using flatten, there is usually a better way to do what you're trying to do. In this case, I would use mapcat -- it's like map, but it handily concatenates the results together into a single collection.

EDIT: This doesn't work! Please disregard :)

...
(mapcat (fn [[k v]]
          (when (.startsWith number k index)
            (possible-strings number (+ index (count k)) (str acc v))))
        code->letter))))

or,

...
(mapcat (fn [[k v]] (possible-strings number (+ index (count k)) (str acc v)))
        (filter #(.startsWith number (key %) index) code->letter)))))

Functional programming

You're correct in that your algorithm is not (and cannot be) tail-recursive. This is because you're wrapping the recursive call to possible-strings within a filtering and mapping operation. I'm not familiar with the term "backtracking algorithm," but it does seem appropriate in this case. This initially struck me as the kind of thing you would do with a reduce, building up a string from scratch, however, you do have to keep "backtracking" to build up new strings using different groupings of digits. I might be wrong about this, but I don't think this is something you can do with tail-recursion, which means you can't use recur and there would be performance implications, especially for large inputs.

Here's another approach: write a function that takes a number and returns a list of possible arrangements of numbers from 1-26. Then just map the numbers to letters and you'll have your possible strings.

This function, which I'll call possible-arrangements, is a little complicated to write. My approach is to walk through the number string from the beginning and form a tree, looking at numbers 2 at a time and branching off for each possibility, each branch terminating once it reaches a list of numbers that can't be broken down any further. For example:

; *starred numbers are ones that are evaluated to see if they can be broken
; down further

                  *4125    ; 41 is not in range, so only one possibility...
                    |
                  4 *125   ; 12 can be 1 2 or 12, so we branch, etc.
                    /  \
                 1 *25  12 5
                   /\     |
                  /  \    |
      (4 1 ...) 2 5  25  12 5   = 4 1 2 5, 4 1 25, 4 1 12 5

; the result would be [["4" "1" "2" "5"] ["4" "1" "25"] ["4" "12" "5"]]

.

(defn possible-arrangements [^String n]
  (case (count n)
    1 [[n]]
    2 (let [each-digit (mapv str n)]
        (if (contains? code->letter n)
          [[n] each-digit]
          [each-digit]))
    (let [a       (str (first n))
          ab      (subs n 0 2)
          prepend (fn [x offset]
                    (map (partial cons x) (possible-arrangements (subs n offset))))]
      (if-not (contains? code->letter ab)
        (prepend a 1)
        (concat (prepend a 1) (prepend ab 2))))))

(defn possible-strings [^String n]
  (for [nums (possible-arrangements n)
        :let [chars (map code->letter nums)]]
    (apply str chars)))

This is still not a tail-recursive solution, but it's about 4 times faster, I think because it isn't having to perform 26 string operations for each digit of the input number.


Encrypting and decrypting strings using Caesar cipher I was inspired by a solution in Think Python: How to Think Like a Computer Scientist. I structured mine (into two functions) according to it, and used the technique used in
rotate_letter. It's here if you're interested.

How can I make this better in all aspects (documentation, algorithm, variable names, and etc.)? Is memoizing rotated_string worth it?

def rotated_letter(letter, key):
    """Returns a single-letter string representing
       letter rotated by key, and returns letter if
       it doesn't belong to any of the English alphabet.

       letter: a single-letter string.

       key: an integer.
       """
    if letter.isupper():
        start = ord('A')

    elif letter.islower():
        start = ord('a')

    else:
        return letter

    diff = ord(letter) - start
    position = (diff + key) % 26 + start

    return chr(position)

def rotated_string(string, key, encrypt=True):
    """Returns string rotated by key.

       string: a string.

       key: an integer.

       encrypt: a boolean value.
       """
    result = ''
    memo = {}

    if not encrypt:
        key = -key

    for character in string:
        if character in memo:
            # rot_cahr denotes rotated character
            rot_char = memo[character]
        else:
            rot_char = memo.setdefault(character, rotated_letter(character, key))

        result += rot_char

    return result

def main():
    print(rotated_string('Hello!', 3))
    print(rotated_string('Khoor!', 3, encrypt=False))

if __name__ == '__main__':
    main()

Docstrings:
If using (and only targeting) python 3.5 or later, you could remove the type hints from the docstring and include them as real type hints.
You could also extend the docstrings to contain a (doctest-conforming) example usage.

Algorithm:
Unicode is HARD. Python3 strings are unicode. The caesar cipher only works for letters that are in ascii range. Example:

>>> rotated_letter("Á", 13)
'L'
>>> rotated_letter("L", -13)
'Y'

So your test by isupper / islower is not enough. You could do 'a' <= letter <= 'z' for an ascii lowercase test.

Variable names:
Just a gut feeling: functions do stuff. So a good name for a function is an imperative: I'd use rotate_letter instead of rotated_letter.
Also, once you name the functions rotate_xxx, the concept of a key is not valid anymore (this belongs into functions like encrypt and decrypt). When speaking of rotation, I'd call the argument offset or something like that.

Memoization / Performance:
The letter encryption function does not do much, so memoization is probably not necessary.
You are concatenating strings for building the result. This is slow. If this slowness doesn't matter to you (because of your small-input use case), then memoization is overkill. If you want better performance, you would (in the general case) append your string parts onto a list, and join the list when the calculation is done. In this special case, I would even opt for a preallocated array, as the output size is known.

PS:
I learned something, too, i.e. that there is setdefault() for dicts. Thanks!


Read file directly - simplified In order to read a whole file as a string using standard libraries you have to allocate memory, terminate the string, determine size of the file and size of each element.
This could happen with fread until I decided to automatize it using this:

char *readFile (FILE* fp, char *source)
{
    unsigned long long int size;
    unsigned long long int i = 0;
    int chr;

    fseek(fp, 0L, SEEK_END);
    size = ftell(fp);

    source = calloc(size + 1, sizeof(char));

    fseek(fp, 0, SEEK_SET); // rewind

    while(chr != EOF)
        source[i++] = (chr = fgetc(fp));

    source[size] = '\0';    // Terminate string

    return(source);
}

You let the user pass in a char* but you don't use it. you might as well remove it and declare it as a local variable.

char *readFile (FILE* fp)
{
    char* source;

Some FILE* won't let you seek to the end (stdin and socket streams for example). You should check the return value of fseek and fall back to the classic fread then realloc, rinse and repeat method on error.

A single fread is more efficient than the repeated fgetc; so you can replace the while with:

int readBytes = fread(source, size, 1, fp);

string[readBytes] = '\0';


Improve Performance of Comparing two Numpy Arrays I had a code challenge for a class I'm taking that built a NN algorithm.  I got it to work but I used really basic methods for solving it.  There are two 1D NP Arrays that have values 0-2 in them, both equal length. They represent two different trains and test data  The output is a confusion matrix that shows which received the right predictions and which received the wrong (doesn't matter ;).

This code is correct - I just feel I took the lazy way out working with lists and then turning those lists into a ndarray.  I would love to see if people have some tips on maybe utilizing Numpy for this? Anything Clever?

import numpy as np

x = [0, 0, 0, 0, 1, 0, 1, 0, 0, 1, 0, 1, 0, 1, 1, 1, 1, 0, 1, 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
     2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 1, 0, 2, 0, 0, 0, 0, 0, 1, 0]
y = [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
     2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]

testy = np.array(x)
testy_fit = np.array(y)

row_no = [0,0,0]
row_dh = [0,0,0]
row_sl = [0,0,0]

# Code for the first row - NO
for i in range(len(testy)):
    if testy.item(i) == 0 and testy_fit.item(i) == 0:
        row_no[0] += 1
    elif testy.item(i) == 0 and testy_fit.item(i) == 1:
        row_no[1] += 1
    elif testy.item(i) == 0 and testy_fit.item(i) == 2:
        row_no[2] += 1

# Code for the second row - DH
for i in range(len(testy)):
    if testy.item(i) == 1 and testy_fit.item(i) == 0:
        row_dh[0] += 1
    elif testy.item(i) == 1 and testy_fit.item(i) == 1:
        row_dh[1] += 1
    elif testy.item(i) == 1 and testy_fit.item(i) == 2:
        row_dh[2] += 1

# Code for the third row - SL
for i in range(len(testy)):
    if testy.item(i) == 2 and testy_fit.item(i) == 0:
        row_sl[0] += 1
    elif testy.item(i) == 2 and testy_fit.item(i) == 1:
        row_sl[1] += 1
    elif testy.item(i) == 2 and testy_fit.item(i) == 2:
        row_sl[2] += 1

confusion = np.array([row_no,row_dh,row_sl])

print(confusion)

the result of the print is correct as follow:

[[16 10  0]
 [ 2 10  0]
 [ 2  0 22]]

This can be implemented concisely by using numpy.add.at:

In [2]: c = np.zeros((3, 3), dtype=int)

In [3]: np.add.at(c, (x, y), 1)

In [4]: c
Out[4]:
array([[16, 10,  0],
       [ 2, 10,  0],
       [ 2,  0, 22]])


Having less code for the current program I am was writing a practice program in javascript and it turned out to work quite alright, however I am not entirely happy with the code. I can't shake the feeling that there is a lot of code which doesn't have to be there, I feel that it can be improved.

The idea of the program is basically, you have a selector, each time you make a choice and hit submit a new set of choices are offered. The choices of the user are stored in an object.

I am interested to know if there is a way to make this code more efficient.

Here is the entire source code (+ the html part)

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8" />
    <title>Dynamic Selector</title>
</head>
<body>
    <select id="select">
</select>
    <button onclick="updateObject()">Submit</button>
</body>
</html>
<script>
    var select = document.getElementById("select");

    var choice = { time: "", category: "", complexity: "" };
    var time = ["15", "30", "60", "60+"];
    var category = ["knowedge", "experimenting", "self-iprovement", "cooking", "health"];
    var complexity = [1, 2, 3, 4];

    var coef = 0;

    //creates the children for the time choices
    function setChoices() {
        if (coef === 0) {
            for (var i = 0; i < time.length; i++) {
                var node = document.createElement('option');
                node.value = time[i].toString();
                node.innerHTML = time[i];
                select.appendChild(node);
            }
        } else if (coef === 1) {
            for (var i = 0; i < category.length; i++) {
                var node = document.createElement('option');
                node.value = category[i].toString();
                node.innerHTML = category[i];
                select.appendChild(node);
            }
        } else if (coef === 2) {
            for (var i = 0; i < complexity.length; i++) {
                var node = document.createElement('option');
                node.value = complexity[i].toString();
                node.innerHTML = complexity[i];
                select.appendChild(node);
            }
        }
    }

        function updateObject() {
            if (coef === 0) {
                choice.time = select.value;
                console.log(choice.time);
                coef++;
                removeChildren();
                setChoices();
            } else if (coef === 1) {
                choice.category = select.value;
                coef++;
                removeChildren();
                setChoices();
            } else if (coef === 2) {
                choice.complexity === select.value;
                printObject();
                setChoices();
            }
        }

        //removes the children of the "select" element
        function removeChildren() {
            while (select.firstChild) {
                select.removeChild(select.firstChild);
            }
        }

        function printObject() {
            console.log("foobarr");
        }

        removeChildren();
        setChoices();
</script>

Applying the good ol' DRY principle and by adding coef as a parameter, we'd get something like :

function setChoices(coef) {
    var relevantArray = (coef === 0) ? time :
                        (coef === 1) ? category :
                                       complexity;
    for (var i = 0; i < relevantArray.length; i++) {
        var node = document.createElement('option');
        node.value = relevantArray[i].toString();
        node.innerHTML = relevantArray[i];
        select.appendChild(node);
    }
}

function updateObject() {
       for (var coef = 0; coef<3; coef++)
       {
           ((coef === 0) ? choice.time :
            (coef === 1) ? choice.category :
                           choice.complexity) =  select.value;
            removeChildren();
            setChoices(coef);
       }
}

(Also, I have changed a few things which I assumed to be wrong, for instance, updateObject did not call setChoices with coef=0.


Pig Latin exercise in Rust The title says it all. I'm reading "The Rust Programming Language" 2nd ed. right now, and just finished the "Collections" chapter. I know there's a ton of Pig Latin code here, but I think Rust deserves a separate question.

use std::io;
use std::iter::FromIterator;

fn main() {
    println!("Enter the string to be Pig-latinized");

    let mut input = String::new();

    io::stdin().read_line(&mut input)
        .expect("Failed to read from stdin");

    let s: Vec<char> = input.chars().collect();
    let mut new_s: Vec<char> = Vec::new();
    let consonants = ['b', 'c', 'd', 'f', 'g', 'h', 'j', 'k', 'l', 'm', 'n', 'p', 'q',
    't', 'r', 's', 'v', 'w', 'x', 'z', 'B', 'C', 'D', 'F', 'G', 'H', 'J', 'K', 'L', 'M', 'N', 'P', 'Q',
    'T', 'R', 'S', 'V', 'W', 'X', 'Z'];

    let mut i = 0;

    while i < s.len() {

        if !s[i].is_alphabetic() {
            new_s.push(s[i]);
            i += 1;
            continue;
        }

        let first_letter = s[i];
        let mut appendage: Vec<char> = "-hay".chars().collect();
        if consonants.contains(&first_letter) {
            appendage[1] = first_letter;
            i += 1;
        }

        while s[i].is_alphabetic() && i < s.len() {
            new_s.push(s[i]);
            i += 1;
            continue;
        }

        new_s.append(&mut appendage);
    }

    println!("{}", String::from_iter(new_s));
}

The code feels clunky, but I don't really know how to make it better. I'd love to see an idiomatic Rust solution!

For overall issues, these jump out:

s[i].is_alphabetic() && i < s.len() can panic, because you access the value before checking the length, it would have to be i < s.len() && s[i].is_alphabetic().
is_alphabetic considers lots of things alphabetic that you probably don't want. If you look at the docs the examples include assert!('京'.is_alphabetic()); which probably isn't what you want.

For Rust itself:

Rather than converting the string to a vector of chars, it's easier to use an iterator. In your case it needs to be an iterator that can .peek so you can stop before consuming the character.
Pattern-matching can make the readability of checking the vowels vs others easier, and it allows you to collapse the logic in your first if. The matches also allow you to cleanly create and define the suffix value earlier, and avoid the usage of the problematic is_alphabetic function.

Here's an updated implementation:

let mut chars = input.chars().peekable();
let mut new_s = String::new();
while let Some(c) = chars.next() {
    let suffix = match c {
        'a' | 'e' | 'i' | 'o' | 'u' => {
            new_s.push(c);
            String::from("-hay")
        }
        'a'...'z' | 'A'...'Z' => {
            format!("-{}ay", c)
        }
        _ => {
            new_s.push(c);
            continue;
        }
    };

    while let Some(&c) = chars.peek() {
        match c {
            'a'...'z' | 'A'...'Z' => {
                chars.next();
                new_s.push(c);
            }
            _ => break,
        }
    }

    new_s += &suffix;
}
println!("{}", new_s);


Generic data interface I currently have the following working code. I am looking for some suggestion with best practices and perhaps a better way to accomplish my goal.

Goal: - in short - Have a generic data interface as a single data access point to different data sources.

Concern: My current code is working fine but I have to call GetRepository() in each method inside my test class to access the desire instance of the data source repository.

Code Snippet:

public class GenericRepository : IGenericRepository
{
   public T GetRepository<T>()
    {
        string typeName = typeof(T).ToString();
        if (string.IsNullOrEmpty(typeName))
            return default(T);

        Type repoType = Type.GetType(typeName);
        object repoInstance = Activator.CreateInstance(repoType);
        return (T)(object)repoInstance;
    }
    // more code...
}

Usage:

public class MyTestClass
{
    IGenericRepository repo = new GenericRepository();

    private void DoThis()
    {
       IFileRepository repoA = repo.GetRepository<FileRepository>();
       // now I can call repository A interface methods like this repoA.SomeMethod()
    }

    private void DoThat()
    {
        ISQLRepository repoB = repo.GetRepository<SQLRepository>();
        // now do something with repository B interface methods like this repoB.SomeMethod()
    }
}

I thought of an alternative way to do this but it has its own drawbacks as well.

Implement the interface methods in both IGenericeRepository and as well as the interface I inherited.
Implement the methods for inherited interface inside IGenericRepository and put it in each different region and end up with a massive page of code

Thoughts? Comments? Suggestion?
-- I personally think the first option is a better choice but wonder if there are better ideas or maybe there is something wrong with my implementation.

Alternative Code

public class GenericRepository : IGenericRepository, IFileRepository, ISQLRepository
{
   public FileRepository fileRepo { get; set; }
   public SQLRepository sqlRepo { get; set; }

   public GenericRepository()
   {
       fileRepo = new FileRepository();
       sqlRepo = new SQLRepository();
   }
    // more code...
}

Alternative Usage

public class MyTestClass
{
    IGenericRepository repo = new GenericRepository();

    private void DoThis()
    {
         // now I can access file repository like this: repo.fileRepo.SomeMethod();
         // now I can access sql repository methods like this: repo.sqlRepo.SomeMethod();
    }

    private void DoThat()
    {
         // now I can access file repository like this: repo.fileRepo.SomeMethod();
         // now I can access sql repository methods like this: repo.sqlRepo.SomeMethod();
    }
}

======
updated:

To provide more context. Below is the solution structure

-- Solution

---- Repository Project (GenericRepository, FileRepository, SQLRepository)

---- IRepository Project (IGenericRepository, IFileRepository, ISQLRepository)

---- MyTestProject Project (MyTestClass)

Your current implementation is somewhat flawed as relying on ToString to get the type name is dangerous. ToString is meant largely to yield or short meaningful description of the object for debugging, logging etc.
typeof(T) already yields a Type - it is totally unnecessary to convert it to a string just to then turn it back into a type again.
If your repositories all have a parameterless default constructor you can avoid calling CreateInstance entirely.

Your current implementation can be shortened to:

public T GetRepository<T>()
{
    object repoInstance = Activator.CreateInstance(typeof(T));
    return (T)(object)repoInstance;
}

or possibly even to:

public T GetRepository<T>() where T : new()
{
    return new T();
}

In addition to that your GenericRepository seems more like a repository factory than a repository in itself hence you should possibly rename it to RepositoryFactory and the method to CreateRepository.


Python persistant cache for non-pythonic objects I am trying to implement a cache which can be persisted. Also, some of the functions that I am trying to cache takes arguments that are not python objects (external library objects). But they seem to have a hash value. My first try was to use lru_cache but it didn't have an option to persist across sessions. Then I stumbled upon this answer which explained a way to create a custom decorator to cache the function results. I made some minor changes to it and came up with this decorator (which can take arguments) now.
def cached(maxsize=None, hashkey=True, persist=False):
    """Decorator method to cache an expensive function

    Args:
        maxsize (int): The maximum number of cached results to store
            if None, the size is indefinite.
        hashkey (bool): If the arguments of the function are objects
            which cannot be pickled, use the hashkey option which will
            hash the arguments.
        persist (bool): Write the cached results to the disk so that
            it can be read in even after the session has ended.

    Returns:
        Decorator function
    """

    def outer(func):
        func.cache = {}
        func.cache_file = f"{func.__name__}.cache"

        @wraps(func)
        def wrapper(*args):
            if hashkey:
                key = hash(args)
            else:
                key = args
            try:
                return func.cache[key]
            except KeyError:
                try:
                    if persist:
                        with open(func.cache_file, "rb") as cachefile:
                            func.cache = pickle.load(cachefile)
                    return func.cache[key]
                except (KeyError, FileNotFound):
                    func.cache[key] = result = func(*args)
                    if maxsize and len(func.cache) > maxsize:
                        func.cache.pop(next(iter(func.cache))) # removes the first item
                    if persist:
                        with open(func.cache_file, "wb") as cachefile:
                            pickle.dump(func.cache, cachefile)
                    return result

        return wrapper

    return outer

The functions that I am planning to cache may be called at most 10000 times with a given cache file.
Here are my questions:

Is there anything obviously wrong with this code?
Is hashing the arguments a bad idea?
Are there any obvious places where I can store the cache file?

It seems rather inefficient to reload the cache file on every cache miss. The last part of wrapper() saves the file whenever the cache is updated, so the cache and file should always be in synch.  Loading the file could be moved into outer().


SQL Query over ODBC is slow with DISTINCT Is there any way to simplify this query? It's being run against a Progress Database via an ODBC connection.

The CustomerSalesperson table has a CustomerID, a SalespersonID, and a CreateDate. Each CustomerID could have many records, each with a different SalespersonID. I'd like to get a list of the SalespersonIDs with the most recent CreateDate for each CustomerID. This is what I'm doing currently:

SELECT DISTINCT cs.CustomerID, cs.SalespersonID
FROM PUB.CustomerSalesperson cs
INNER JOIN
  (SELECT CustomerID, MAX(CreateDate) AS MaxDate
  FROM PUB.CustomerSalesperson
  GROUP BY CustomerID) csmax
ON cs.CustomerID = csmax.CustomerID
WHERE csmax.MaxDate = cs.CreateDate

It take a little longer to return than I would expect. When I remove the DISTINCT, it goes a lot faster, but it returns many duplicates as a result of other fields in the table not which aren't relevant to the query.

This is a messy query, I personally don't like nesting a select statement in my joins, I would rather use a Common Table Expression like this

WITH csmax AS (
    SELECT CustomerID, MAX(CreateDate) AS MaxDate
        FROM PUB.CustomerSalesperson
        GROUP BY CustomerID
)
SELECT DISTINCT cs.CustomerID, cs.SalespersonID
    FROM PUB.CustomerSalesperson cs
    INNER JOIN csmax ON cs.CustomerID = csmax.CustomerID
    WHERE csmax.MaxDate = cs.CreateDate

This should help the DB run the query more efficiently as well.  using the with statement should allow the SQL Engine to create a table and then compare the tables the way they were meant to be compared.  The CTE actually prepares the table before the outer query is run, so we aren't doing this on the fly.

Sounds like you can't run this through the ODBC connection, I looked at the documentation that you linked me to and found a CREATE VIEW command that would probably have similar efficiency to using a CTE.

CREATE VIEW csmax (CustomerID, CreateDate) AS
    SELECT CustomerID, MAX(CreateDate) AS MaxDate
        FROM PUB.CustomerSalesperson
        GROUP BY CustomerID ;

SELECT DISTINCT cs.CustomerID, cs.SalespersonID
    FROM PUB.CustomerSalesperson cs
    INNER JOIN csmax ON cs.CustomerID = csmax.CustomerID
    WHERE csmax.MaxDate = cs.CreateDate

DROP VIEW csmax

This code is more readable than the code that you wrote, it is easier to follow.

First thing you want to do is get a list of all the customers and the most recent date they did whatever.

Then, you want to know which Sales Person is associated with the "whatever" that customer did at their most recent visit.


Usage of Expression> I am developing data-interfacing that converts between two different data models.
However, I must be sure that all required fields exist. Therefore I have written this utility class that I can easily use to verify required fields.

However I am unsure whether this is the best way because of the expression that needs to be compiled and the usage of reflection. Any feedback is welcome!

Usage

    public OutputDataElement DetermineRetailTransactionShopperType(IHeaderEntity headerEntity)
    {
        Ensure.IsNotNull(() => headerEntity);
        Ensure.IsNotNull(() => headerEntity.ShopId, "headerEntity");

        // Some mapping logic removed from the example
    }

Utility

/// <summary>
/// Helper class able to ensure expectations
/// </summary>
public static class Ensure
{
    public static void IsNotNull<T>(Expression<Func<T>> property) where T : class
    {
        IsNotNullImpl(property, null);
    }

    public static void IsNotNull<T>(Expression<Func<T>> property, string paramName) where T : class
    {
        IsNotNullImpl(property, paramName);
    }

    private static void IsNotNullImpl<T>(Expression<Func<T>> property, string paramName) where T : class
    {
        // Compile the linq expression
        Func<T> compiledFunc = property.Compile();

        // Invoke the linq expression to get the value
        T fieldValue = compiledFunc.Invoke();

        // Check whether we have a value
        if ((fieldValue is string && fieldValue.ToString() == string.Empty) || (fieldValue == null))
        {
            // We have no value. Get the initial expression
            var expression = (MemberExpression)property.Body;

            // log information about the expression that failed
            throw new ArgumentException(string.Format("Missing required field '{0}'", expression.Member.Name), string.IsNullOrEmpty(paramName) ? null : paramName);
        }
    }
}

I don't see any unnecessary reflection in your code.

One think that could be simplified is your check for string.Empty:

if (fieldValue == null || fieldValue as string == string.Empty)


Efficiency of binary search and improvements Below you can see my binary search method, and as a relatively new programmer I am looking for any suggestions on how it could be improved.

bool found = false;
int min = 0;
int max = 9;
int avg;
int anArray[] = {1,2,3,4,5,6,7,8,9,10};

int userNumber;

printf("Enter a number between 1 and 10:\n");
scanf("%d", &userNumber);

while(!found){
    avg = (min+max)/2;
    if(anArray[avg]==userNumber){
        found = true;
        break;
    }
    if(anArray[avg]<userNumber)
        min = avg+1;
    else if(anArray[avg]>userNumber)
        max = avg-1;
    if( (min==max) && (anArray[max]!=userNumber) )
        break;
}
if(found)
    printf("The number is in index [%d]\n", avg);
else
    printf("Number not found\n");

While loop termination

The while loop condition is:

while(!found) {

But when you find your answer, you already do a break.  Therefore, the while loop condition could just be:

while (1) {

But there's more.  This statement here is not quite right:

if( (min==max) && (anArray[max]!=userNumber) )
    break;

First of all, it is possible that min could actually go beyond max.  For example, if originally min is 6, max is 7, and avg is 6, then it could be the case that max becomes 5 afterwards.  So you really need to check if min > max and break when that happens.

Secondly, if min == max, there's no need to do an extra equality check and break.  You can just loop around one more time.

Putting that all together, I would actually remove that whole if statement and change your while loop to:

while (min <= max) {
    avg = (min+max) / 2;
    if (anArray[avg] == userNumber) {
        found = true;
        break;
    }
    if (anArray[avg] < userNumber)
        min = avg+1;
    else
        max = avg-1;
}

Unnecessary if

Notice I also removed the check for anArray[avg] > userNumber because at that point, it wasn't equal or less than, so it had to be greater than.


Shortening Custom Document Property additions Context
I have blocks of if statements that check if a certain cell contains a particular string and to add to the counter of the respective custom property. Originally I was using global variables, but those are zeroed out when excel crashes. I learned of custom document properties and their non-volatility and made the switch. However, adding to the counter is a long, barely readable statement.
With global variables I had a statement like:
If cell = "Heavy Duty" Then numHD = numHD + 4

But now statements look like:
If cell = "Heavy Duty" Then ThisWorkbook.CustomDocumentProperties("numHD").Value = ThisWorkbook.CustomDocumentProperties("numHD").Value + 4

It's immensely longer and with 27 other statements doing the same thing for other strings, the code becomes taxing to read.
What I would like
Is there a way that I can shorten the custom document property edits to look more in line with what I had before with global variables? I had tried setting up a global variable as a properties type and setting to the property, then just using the variable in the actual sub:
Public numHD As Properties
Set numHD = ThisWorkbook.CustomDocumentProperties("numHD")

Sub count()
    If cell = "Heavy Duty" Then numHD = numHD + 4
End Sub

But Set cannot be used outside of a sub or function apparently. Any advice?

Writing to a document property is a concern in its own right, and thus deserves its own scope. Write a procedure that's responsible only for this!

Public Sub IncrementHeavyDutyCount(ByVal increment As Long)
    With ThisWorkbook.CustomDocumentProperties.Item("numHD")
        .Value = .Value + increment
    End With
End Sub

And now you can increment your counter with a simple, self-explanatory procedure call:

If cell = "Heavy Duty" Then IncrementHeavyDutyCount 4


Equals and Contains for text search I am trying to implement a search routine for an application written in C#. The requirements are as follows:

The user can select one of the following options for search:

Property Search(Exact): the text entered in the search box has to match exactly with the results, e.g. searching for PL should only show the results whose properties contain PL.
Property Search(Contains): the text entered in the search box can be part of the results, e.g. searching for PL should show all the results whose properties contain PL, e.g. PLApp, Planning, playing, etc.

I have implemented it in my code by passing a boolean flag to my method, and using if..else to perform the searches accordingly.

    public override bool PropertySearch(string value, bool searchPartial = false)
    {
        if(searchPartial)
        {
            if (dataValue.IndexOf(value, StringComparison.OrdinalIgnoreCase) >= 0)
                return true;
        } else
        {
            if (dataValue.Equals(value, StringComparison.OrdinalIgnoreCase))
                return true;
        }
        return false;
    }

This was my first implementation, and I have a feeling there might be a better and concise way to achieve this. Any thoughts?

The flow control seems excessively verbose.  This would do the same:

public override bool PropertySearch(string value, bool searchPartial = false)
{
    return searchPartial ? (dataValue.IndexOf(value, StringComparison.OrdinalIgnoreCase) >= 0)
                         :  dataValue.Equals(value, StringComparison.OrdinalIgnoreCase);
}

I would also question your decision to lump these two operations together into the same method.  The code would likely be clearer if you wrote two separate methods.  But you haven't shown the superclass or rest of this class, so I can't say for sure.