Python Binary Search My take on a binary search algorithm without research or knowledge of an efficient one. It would be appreciated if one could show me an efficient way to do it using recursion or a more pythonic approach.

from math import *

numList = [i for i in range(11)]

while True:
     try:
           beginning = 0
           end = len(numList) - 1
           mid = floor(((end-beginning)/2) + beginning)

           value = int(input("What value would you like to search for?: "))

           notFound = True

           while notFound:
               if value > end or value < beginning:
                   print("The value is not in the list")
                   break
               elif value == mid:
                   print("The value is in the list")
                   print("The index is " + str(mid))
                   notFound = False
               elif value == end:
                   print("The value is in the list")
                   print("The index is " + str(end))
                   notFound = False
               elif value > mid:
                  beginning = mid
                  mid = floor(((end-beginning)/2) + beginning)
               elif value < mid:
                  end = mid
                  mid = floor(((end-beginning)/2) + beginning)
     except ValueError:
          print("Invalid characters")

```

numList is not used (this is "ok" as numList[i] == i). There is value == mid or value == end several times. But this just means you are comparing a value to be found with an index. The correct way would be value == numList[mid].

Consider using a function that does the binsearch for you.

You may or may not use recursion. There is a nice way to do binsearch without recursion.

I have also incorporated some naming conventions of python. If you are interested try programs like pylint. I have also added a bit nicer string formatting and some other python features you can live without but can come handy.

"""Binary search."""
from random import randint

def bin_search(sorted_list, to_find):
    """Searches the sorted_list for a to_find.
    If the to_find is found return (True, index)
    Otherwise return (False, 0)
    """
    beg = 0
    end = len(sorted_list) - 1
    while beg <= end:
        mid = (end + beg) // 2  # integer division
        if sorted_list[mid] == to_find:
            return (True, mid)
        if sorted_list[mid] < to_find:
            (beg, end) = (mid + 1, end)
        else:  # sorted_list[mid] > to_find
            (beg, end) = (beg, mid - 1)
    return (False, 0)

# Create a sorted list of random numbers
MY_LIST = sorted(randint(0, 100) for _ in range(20))
print(MY_LIST)

while True:
    try:
        TO_FIND = int(input('Enter value you want to find: '))
        (FOUND, INDEX) = bin_search(sorted_list=MY_LIST, to_find=TO_FIND)
        if FOUND:
            print(f'Found {TO_FIND} at index: {INDEX}!')
        else:
            print(f'Value {TO_FIND} not found!')
    except ValueError:
        print('Invalid number to find')
```


Any side effects from this Python logging code? def _getCallerLogger():
    caller = inspect.currentframe().f_back.f_back
    name = "{}-{}".format(caller.f_globals['__name__'], caller.f_code.co_name )
    logger = logging.getLogger(name)
    return logger

log = lambda msg : _getCallerLogger().debug(msg)
info = lambda msg: _getCallerLogger().info(msg)
warning = lambda msg: _getCallerLogger().warning(msg)
error = lambda msg: _getCallerLogger().error(msg)
critical = lambda msg: _getCallerLogger().critical(msg)

I have the above logging code in my application so I can just call log from anywhere and it will print the file and where the log method was called from.

Would there be anything wrong with this in the long run?

The logging module already supplies a function name to log messages.  If you want to include the function name in your log message just set the appropriate formatter with %(funcName)s in the format string on your log handler with: http://docs.python.org/2/library/logging.html#logging.Handler.setFormatter

See also http://docs.python.org/2/library/logging.html#logrecord-attributes

The typical usage is to create a global log variable in your module like

logger = logging.getLogger(__name__)

and use that logger within functions in that module.  But creating a separate logger for every single function is excessive.

I (just now) realized this is a pretty old question, but if you're still interested I can provide a full working example.


Unlocking spreadsheet files quickly I've created a program for my job that unlocks spreadsheet files quickly and efficiently. What we use to have to do before this program is we'd have to go into the system, search for the file, kill the users process containing the file, unlock the file, then the user can finally get back into their spreadsheet. With my new program it first cd's into the directory containing the files, you enter the file number "folio", it searches through a list of locked files using shell commands, then unlocks the specific one for the user.

I'm looking for some critique on my work, I would really like to know what I can do better what I did wrong, etc..

Source:

#!/usr/local/bin/ruby

require 'fileutils'

module Kernel
    def ls_grep
        `cd '/my/dir'`
        `ls -la|grep -i .lock`
        puts "Enter Folio #"
        @input_folio = gets.chomp
        lockfile = "/my/dir.~lock.#{@input_folio}.ods#"
        if @input_folio =~ /^\d{7}/
            if File.exist?( lockfile )
                puts "Unlock file?"
                input = gets.chomp.upcase
                if input == 'Y'
                     FileUtils.rm( lockfile )
                     puts "File unlocked."
                else
                    puts "You went through that trouble for no reason..."
                end
            else
                puts <<-EDE.gsub(/^\s*>/, ' ')
                >
                >Lockfile not found for Folio # #{@input_folio}"
                >
                >If the file name doesn't match the folio number, get the file name
                >and use that instead of the Foilio number...
                >
                EDE
                ls_grep
            end
        else
            puts <<-EDF.gsub(/^\s*>/, ' ')
            >
            >What part of 'Folio #' is hard to understand? 7 DIGITS"
            >
            >If the file name doesn't match the folio number, get the file name
            >and use that instead of the Folio number...
            >
            EDF
            ls_grep
        end
    end
end
ls_grep

Example usage:

Enter Folio #
1234567

 Lockfile not found for Folio # 1234567"

 If the file name doesn't match the folio number, get the file name
 and use that instead of the Foilio number...

Enter Folio #
erwe

 What part of 'Folio #' is hard to understand? 7 DIGITS"

 If the file name doesn't match the folio number, get the file name
 and use that instead of the Folio number...

Enter Folio #

Don't put your method inside Kernel.
Rename ls_grep to something more descriptive, e.g. unlock_spreadsheet.
The two shell commands (cd and ls|grep) aren't needed. The code doesn't depend on the current directory, and you're not using the commands' output.
Don't use an instance variable (@input_folio) as a temporary variable inside a method. Make it a local variable.
The regex matches anything that begins with sevent digits. Use /\A\d{7}\z/ instead in order to match against the whole string.
Don't check if the file exists before deleting it, someone might delete it after you check and before you try to delete it. Instead, try to delete it and catch the possible exception.
Don't use recursion to repeat the method. Use a loop.

                puts <<-EDE.gsub(/^\s*>/, ' ')
                >
                >Lockfile not found for Folio # #{@input_folio}"
                >
                >If the file name doesn't match the folio number, get the file name
                >and use that instead of the Foilio number...
                >
                EDE

Remove the call to gsub:

                puts <<-EDE

 Lockfile not found for Folio # #{@input_folio}"

 If the file name doesn't match the folio number, get the file name
 and use that instead of the Foilio number...

                EDE

You may want to put those strings in global constants to avoid this decrease in indentation in the code.


Remove specific CSS rules from WYSIWYG text I have created a working function for cleaning some specific styles within the text from a WYSIWYG editor.

  var rulesText = textArea.value;

  var selector = "a:link, span.MsoHyperlink";
  var pattern = new RegExp(selector.replace(/\./g, "\\.") + "\\s*{[^}]*?}", "gim");

  rulesText = rulesText.replace(pattern,"");

  var nextSelector = "a:visited, span.MsoHyperlinkFollowed";
  var nextPattern = new RegExp(nextSelector.replace(/\./g, "\\.") + "\\s*{[^}]*?}", "gim");

  rulesText = rulesText.replace(nextPattern,"");

  textArea.value = rulesText;

  console.log("FIRED!");

Here are the styles being cleaned:

a:link, span.MsoHyperlink
{mso-style-unhide:no;
color:blue;
text-decoration:underline;
text-underline:single;}

a:visited, span.MsoHyperlinkFollowed
{mso-style-noshow:yes;
mso-style-priority:99;
color:purple;
mso-themecolor:followedhyperlink;
text-decoration:underline;
text-underline:single;}

This solution seems a bit wonky to me... And I feel like there is definitely more room for elegance. How can improve on this, possibly combine the two RegExp instances, and make sure that multiple instances of this are being removed?

As selectors are static(known beforehand) and are not going to change, there is no need to use RegExp constructor syntax to create regex. By saying that,

var selector = "a:link, span.MsoHyperlink";
var pattern = new RegExp(selector.replace(/\./g, "\\.") + "\\s*{[^}]*?}", "gim");

seems unnecessary. With these two statements, you're creating two extra variables, an additional replace and then creating a regular expression from string.
Same is for other selector too.

What if there are 10 more CSS rules to be removed? Will you create 10 more strings, 10 more regex and 10 replacements? And what if there are 100 such rules?

The point is, this is not maintainable solution and the code written is way too much for string removals.

  possibly combine the two RegExp instances, and make sure that multiple instances of this are being removed?

The two regex can be combined using OR condition in regex with g flag to replace all occurrences.

First, using regex literal syntax to create regex

var linksRegex = /a:link, span\.MsoHyperlink\s*{[^}]*?}/gim; // First selector regex

and

var visitedLinksRegex = /a:visited, span\.MsoHyperlinkFollowed\s*{[^}]*?}/gim; // Second selector regex

Using OR condition(|), the two regex can be combined as

/a:link, span\.MsoHyperlink\s*{[^}]*?}|a:visited, span\.MsoHyperlinkFollowed\s*{[^}]*?}/

This works. But, there is duplication of the part \s*{[^}]*?} which can be removed using groups

/(a:link, span\.MsoHyperlink|a:visited, span\.MsoHyperlinkFollowed)\s*{[^}]*?}/

I'll suggest to use \s* instead of space between two selectors which will match any number of any space characters(space, tab, vertical tabs, etc.)

/(a:link,\s*span\.MsoHyperlink|a:visited,\s*span\.MsoHyperlinkFollowed)\s*{[^}]*?}/

To match the CSS rule properties of a selector use [\s\S]*?} where [\s\S] will match any character(space characters and non-space characters) thus no need of m flag anymore, while *? following it will make the regex lazy. See http://www.regular-expressions.info/repeat.html. Lazy repetition will match fewer characters to satisfy the following condition in regex. So, [\s\S]*?} will match anything until } character.

Note: . does not match new-line character even with m-multiline flag Demo. Work-around is to use [\s\S] which matches any character. See Javascript regex multiline flag doesn't work

With these changes the regex will become

/(a:link,\s*span\.MsoHyperlink|a:visited,\s*span\.MsoHyperlinkFollowed)[\s\S]*?}/gi

Here's demo.

Final Code:

var regex = /(a:link,\s*span\.MsoHyperlink|a:visited,\s*span\.MsoHyperlinkFollowed)[\s\S]*?}/gi;

textArea.value = textArea.value.replace(regex, '');

Here's working demo in action to try it out.

function replaceRules() {
  var regex = /(a:link,\s*span\.MsoHyperlink|a:visited,\s*span\.MsoHyperlinkFollowed)[\s\S]*?}/gi;

  var textArea = document.querySelector('textarea');
  textArea.value = textArea.value.replace(regex, '');
}
input {
  margin-bottom: 5px;
  color: green;
  padding: 5px;
}

textarea {
  width: 100%;
}
<input type="button" value="Remove problematic rules" onclick="replaceRules()" />

<textarea cols="40" rows="14">
body {
    background: red;
}

a:link, span.MsoHyperlink {
  mso-style-unhide: no;
  color: blue;
  text-decoration: underline;
  text-underline: single;
}

a:visited, span.MsoHyperlinkFollowed
{mso-style-noshow:yes;
mso-style-priority:99;
color:purple;
mso-themecolor:followedhyperlink;
text-decoration:underline;
text-underline:single;}

a {
    color: gray;
}
</textarea>


DIV mask implemented in JavaScript/CSS Mask.html

        <link rel="stylesheet" type="text/css" href="mask.css">

Mask.js

(function ($) {
    $.fn.Mask = function () {

    if ($(this).find('.Mask').length > 0) return null;
        var __ctrl = $(this)[0];
        if (__ctrl.tagName != 'DIV') return null;

        var containerCssPaddingTop = $(__ctrl).css('padding-top');
        var containerCssPaddingRight = $(__ctrl).css('padding-right');
        var containerCssPaddingBottom = $(__ctrl).css('padding-bottom');
        var containerCssPaddingLeft = $(__ctrl).css('padding-left');

        $(__ctrl).css('position', 'relative');
        $(__ctrl).css('overflow', 'hidden');

        var mask = '

';
        $(__ctrl).prepend(mask);

        var m = $(this).find('.Mask');
        var mc = $(this).find('.MaskContent');

        m.css('margin-top', '-' + containerCssPaddingTop);
        m.css('margin-right', '-' + containerCssPaddingRight);
        m.css('margin-bottom', '-' + containerCssPaddingBottom);
        m.css('margin-left', '-' + containerCssPaddingLeft);

        // The 16 just comes from the fact that the image displayed is 32x32
        mc.css('left', m.width()/2 - 16 + "px");

        var toReturn = {
            RemoveMask: function () {
                m.remove();
            }
        };

        return toReturn;
    };
})(jQuery);

mask.css

.Mask {
    background-image: -webkit-radial-gradient(center, ellipse farthest-side, #FFFFFF 0%, #EEEEEE 100%);
    height: 100%;
    opacity: 0.8;
    position: absolute;
    width: 100%;
    z-index: 4;
}

.MaskContent {
    background: none;
    position: absolute;
    top: 30%;
    z-index: 5;
}

The real code I would like reviewed/criticized is in Mask.js and Mask.css

The idea is that this extends jQuery so you can block a div while it is being updated.

You would use it by running

v = $('#div1').Mask()

then, to remove it, one would do

v.RemoveMask();

The code looks fine to me. Just a couple minor points:

Why do you change the div's position to relative? If the function is called on a div with absolute positioning, it would mess up the layout. And you should revert the CSS changes on RemoveMask() to what they were when Mask() was called because the div's content might depend on a certain overflow value.
If the div changes size while masked, the image will no longer be horizontally centered. I would suggest using CSS instead of JS to center it:

.MaskContent {
  background: none;
  position: absolute;
  width: 32px;
  height: 32px;
  top: 30%;
  left: 50%;
  margin-left: -16px;
  z-index: 5;
}

Your mask background currently only supports webkit browsers; you should add the CSS3 variations for the other browsers.


Copy an array using pointers Is this a correct way to copy elements from array origin to array location?
  #include <stdio.h>
   void copy(const int *origin, int *location, int n){
   int i;
   for(i=0;i<n;i++){
    location[i]=origin[i];
    }
  }

#include <stdio.h>

This function doesn't use anything from <stdio.h>, so don't waste the compiler's time by including it.

   void copy(const int *origin, int *location, int n){

I would put the destination argument first, to be consistent with Standard Library functions such as memcpy.  And change n to be a size_t, so it will work with any array.

   int i;
   for(i=0;i<n;i++){

We can reduce the scope of i, by declaring it in the control expression: for (int i = 0;  i < n;  ++i).

     location[i]=origin[i];

We ought to tell the compiler (with restrict) that the arrays don't overlap.

I think it's simpler just to forward to memcpy():
#include <string.h>
void copy(int *restrict dest, int const *restrict origin, size_t count)
{
    memcpy(dest, origin, sizeof *dest * count);
}

But that's so simple that I don't think we want a separate function for it (particularly with such a broad name).


Create a sorted-map of coordinates I wrote a little function to create a sorted-map with vectors of coordinates as keys and an empty map as a default value:

(defn empty-board [rows cols]
  (into (sorted-map)
        (for [x (range cols) y (range rows)]
            [[x y] {}])))

Is there is a more idiomatic way of achieving this?

Yes, this is the right way to do it.

Many Clojure programs do not bother with creating empty maps. The absence of a value or nil behave like an empty map. For example (assoc nil :foo :bar) returns {:foo :bar}. You may be able to initialize to an empty sorted-map, and only assoc-in novelty as you need it ... depending on your logic.


Convert Foreach Loop into Linq in C# I want to convert these loops into simple loop. I want the code to look neat and short. I saw answers related to LINQ but couldn't make out from that. Are there any other possible ways for this snippet?

XmlDocument manifestXmlFile = new XmlDocument();
manifestXmlFile.Load(manifestFileName);

foreach (XmlNode rules in manifestXmlFile.DocumentElement.ChildNodes)
{
    foreach (XmlNode ruleNode in rules)
    {
        foreach (XmlNode childNodeAttributes in ruleNode)
        {
            foreach (XmlNode childNodeAttrib in childNodeAttributes.ChildNodes)
            {
                XmlElement ruleElement = (XmlElement)ruleNode;
                foreach (XmlNode childNodeConditions in childNodeAttrib.ChildNodes)
                {
                    foreach (XmlNode childNodeCond in childNodeConditions.ChildNodes)
                    {
                        if (childNodeCond.Name.ToUpper() == "CONDITION")
                        {
                            if (childNodeCond.Attributes["type"].Value.ToUpper() == "HEALTHY")
                            {
                                string ruleId = ruleElement.Attributes["ruleid"].Value;
                                string attributeName = childNodeAttrib.Attributes["name"].Value;
                                string attributeType = childNodeAttrib.Attributes["type"].Value;
                                string condTypeValue = childNodeCond.Attributes["type"].Value;
                                string operatorValue = childNodeCond.Attributes["operator"].Value;
                                string healthyConditionValue = childNodeCond.FirstChild.InnerText;

                                var guid = new Guid(ruleId);

                                //Conversion of enum types
                                PsmsOperator psmsOperator = (PsmsOperator)Enum.Parse(typeof(PsmsOperator), operatorValue, true);
                                TypeCode psmsAttributeType = (TypeCode)Enum.Parse(typeof(TypeCode), attributeType, true);

                                Rule rule = new Rule(guid, attributeName, healthyConditionValue, psmsOperator);

                                Rule(attributes, guid);
                           }
                        }
                    }
                }
            }
        }
    }
}

Is there any other better way than this or LINQ?

I think you can extract small methods to simplify this code. Also you can try XPath to get specified sub-nodes and attributes.

And, yes, you can simplify this code with LINQ:

var ruleList = from rules in manifestXmlFile.DocumentElement.ChildNodes.Cast<XmlNode>()
               from ruleNode in rules
               from childNodeAttributes in ruleNode
               from childNodeAttrib in childNodeAttributes.ChildNodes.Cast<XmlNode>()
               from childNodeConditions in childNodeAttrib.ChildNodes.Cast<XmlNode>()
               from childNodeCond in childNodeConditions.ChildNodes.Cast<XmlNode>()
               where childNodeCond.Name.ToUpper() == "CONDITION"
               where childNodeCond.Attributes["type"].Value.ToUpper() == "HEALTHY"
               select new
               {
                   Id = new Guid(ruleElement.Attributes["ruleid"].Value),
                   AttributeName = childNodeAttrib.Attributes["name"].Value,
                   AttributeType = childNodeAttrib.Attributes["type"].Value,
                   CondTypeValud = condTypeValue = childNodeCond.Attributes["type"].Value,
                   OperatorValue = childNodeCond.Attributes["operator"].Value,
                   HealthyConditionValue = childNodeCond.FirstChild.InnerText,
               };

foreach (var r in ruleList)
{
    var psmsOperator = (PsmsOperator)Enum.Parse(typeof(PsmsOperator), r.OperatorValue, true);
    var psmsAttributeType = (TypeCode)Enum.Parse(typeof(TypeCode), r.AttributeType, true);

    var rule = new Rule(r.Id, r.AttributeName, r.HealthyConditionValue, psmsOperator);

    Rule(attributes, r.Id);
}


Creating a simple 52-card data structure for Blackjack in Python I am fairly new to Python so I am trying to learn more about its conventions and ways to make my codes more efficient.
I am making a Blackjack game and so far have this class that draws, organizes, and scores a hand of cards.

class Hand (object):
    def __init__(self, card=[], card2=[], score=0):
        self.card = card
        self.card2 = card2
        self.score = score
    def roll(self):
        # picks two cards and stores in card and card2 respectively as [0rank, 1suit]
        ranks = ["Ace", 2, 3, 4, 5, 6, 7, 8, 9, 10, "Jack", "Queen", "King"]
        suits = ["Hearts", "Diamonds", "Clubs", "Spades"]
        pick = []
        pick2 = []
        pick.append(ranks[randint(0, 12)])
        pick.append(suits[randint(0, 3)])
        pick2.append(ranks[randint(0, 12)])
        pick2.append(suits[randint(0, 3)])
        self.card = pick
        self.card2 = pick2
    def playerScore(self):
        # scores player's hand, prompting ace choice
        value = self.card[0]
        value = str(value)
        points = 0
        if value.isdigit():
            points += int(value)
        elif value != "Ace":
            points += 10
        else:
            aceChoice = input("Play ace as 11 or 1? ")
            while not aceChoice.isdigit():
                try:
                    aceChoice = input("Play ace as 11 or 1? ")
                except TypeError:
                    print("Invalid response.")
            if aceChoice == "11":
                points += 11
            else:
                points += 1
        self.score += points
    def dealerScore(self):
        # scores dealer's hand and automates ace choice
        value = self.card[0]
        value = str(value)
        points = 0
        if value.isdigit():
            points += int(value)
        elif value != "Ace":
            points += 10
        elif value == "Ace" and self.score >= 11:
            points += 1
        elif value == "Ace" and self.score < 11:
            points += 11
        self.score += points

I was wondering if I am using classes correctly, and if I should put the card list to draw from or .pop() in the class initializer, As well as the best type of data structure to store the unused cards in.
Ignore the fact that I called one method roll(). I just finished a Yahtzee game haha.

One thing that catches my eye:

Default list argument.

There is a bit of a footgun hiding in this method:

def __init__(self, card=[], card2=[], score=0):
    self.card = card
    self.card2 = card2
    self.score = score

Default arguments are only created once, when the method is defined.  The end result is that all Hands using default arguments will share the same lists. Observe:

>>> first = Hand()
>>> first.card
[]
>>> second = Hand()
>>> second.card.append(1)
>>> first.card
[1]

I mention this not because it currently causes problems in the code (it does not; card and card1 are never modified in-place), but because it is a problem you are bound to run into in the near future (or perhaps already have!).

There are a couple ways to get around this.

A good placeholder value is usually None.  The following lists are created each time the function is run:

def __init__(self, card=None, card2=None, score=0):
    if card is None: card = []
    if card2 is None: card2 = []
    self.card = card
    self.card2 = card2
    self.score = score

If you immediately take a copy of any mutable objects then they are harmless:

def __init__(self, card=[], card2=[], score=0):
    self.card = list(card) # copies the input list
    self.card2 = list(card2)
    self.score = score

("mutable" objects are those that can be modified in-place; list, dict, and set are mutable.  int, tuple, and str are immutable, and they are safe to use anywhere)

But I feel the most important alternative is probably this:

Simply require the cards to be passed in.

def __init__(self, card, card2, score=0):
    self.card = card
    self.card2 = card2
    self.score = score

Generating cards and remembering (i.e. storing) cards are two separate responsibilities.  It is clear (from the fact that playerScore and dealerScore both index into self.card[0]) that, in general, this class is expected to contain cards.  Therefore, a different one should generate them.

That said, it also has the dual responsibility of both tracking a score and making decisions based upon that score.  In fact, it would seem that these two are the only real reasons it needs to exist.  It feels more like the class is trying to be this:

class Player:
    def __init__(self):
        self.score = 0

    def playerScore(self, card):
        value = str(card)
        if value.isdigit():
            # yadda yadda yadda ...

    def dealerScore(self, card):
        value = str(card)
        if value.isdigit():
            # yadda yadda yadda ...

in which case we never even need to store the card in the class!

Duplicate logic

playerScore and dealerScore duplicate some logic which can be factored out with a flag:

def addCardScore(self, card):
    value = str(card)
    if value.isdigit():
        self.score += int(value)
    elif value == "Ace":
        if self.playerType == 'player':
            self.playerAceValue()
        else:
            self.dealerAceValue()
    else:
        self.score += 10

def playerAceValue(self):
    aceChoice = input("Play ace as 11 or 1? ")
    while not aceChoice.isdigit():
        try:
            aceChoice = input("Play ace as 11 or 1? ")
        except TypeError:
            print("Invalid response.")
    if aceChoice == "11":
        self.score += 11
    else:
        self.score += 1

def dealerAceValue(self):
    if self.score >= 11:
        self.score += 1
    else:
        self.score += 11

# ... somewhere else, not necessarily in a class ...
def playGame():
    player = Player('player')
    dealer = Player('dealer')
    while True:
        playerCard = drawCard()
        player.addCardScore(playerCard)

        dealerCard = drawCard()
        dealer.addCardScore(dealerCard)

This code has a new issue: looking at addCardScore, we see that two branches of the if modify self.score, but it is not immediately obvious that the middle branch (elif value == 'Ace') does the same.  Generally speaking, by rewriting the functions so that most of them simply return values instead of modifying members, it becomes generally easier to reason about when data is modified.  Here's an example version where only one function touches score:

def addCardScore(self, card):
    self.score += self.getCardScore(card)

def getCardScore(self, card):
    value = str(card)
    if value.isdigit():
        return int(value)
    elif value == "Ace":
        if self.playerType == 'player':
            return self.playerAceChoice()
        else:
            return self.dealerAceChoice()
    else:
        return 10

def playerAceChoice(self):
    aceChoice = input("Play ace as 11 or 1? ")
    while not aceChoice.isdigit():
        try:
            aceChoice = input("Play ace as 11 or 1? ")
        except TypeError:
            print("Invalid response.")
    if aceChoice == "11":
        return += 11
    else:
        return += 1

def dealerAceChoice(self):
    return 1 if self.score >= 11 else 11


Simplified three-body simulation The code below simulates a given Newtonian 3 body system. Each row per slice is supposed to represent a Cartesian component for the initial trajectory vector and for the distance vectors to the other two objects.

What are some possible improvements to this? For instance is there any benefit to implement something to cut the redundancy with the duplicate distance vectors? How about using something like NumPy instead of vanilla?

# GRAV_MASS_CONSTANT = 1
# masses = [1, 1, 1]

def updated_trajectory_component(row):
    # a = masses * GRAV_MASS_CONSTANT
    a = 1
    return row[0] + sum([a / i for i in row[1:]])

def updated_distance_component(row):
    return [i - row[0] for i in row[1:]]

def iterate_row(row):
    dist = updated_distance_component(row)
    return [updated_trajectory_component(row)] + dist

def iterate_slice(slice):
    return [iterate_row(i) for i in slice]

def iterate_cube(cube):
    return [iterate_slice(i) for i in cube]

def num_iter(cube, iter):
    print(cube)
    if iter == 1: return iterate_cube(cube)
    else: return num_iter(iterate_cube(cube), iter - 1)

cube = [
    [
        [0, 0.1, 1],
        [0, 1, 0.1],
        [0, 1, 1]
    ],
    [
        [0, 0.1, 1],
        [0, -1, 1],
        [0, -1, 0.1]
    ],
    [
        [0, -1, -1],
        [0, 0.1, -1],
        [0, -1, 0.1]
    ]
]

Docstrings: You should include a docstring at the beginning of every method, class, and module you write. This will allow documentation to identify what your program is supposed to do.
Constants Naming: Any constants that you have in your program (eg. cube) should be in UPPERCASE, like so: CUBE.
Main Guard: Any code that isn't inside a method/class should be inside an if __name__ == '__main__' guard. This will prevent this code from being executed if you decide to import this module from other programs. This StackOverflow answer provides more insight and explanation.
Unnecessary if/else with returns: It is unnecessary to have an else after an if, if you return something in the if. Simply put the else return statement outside the if statement. This return will be run if the if evaluates to be False, which removes the necessity for the else statement.
Reserved Names: You shouldn't use reserved names for variable names, such as iter and slice, to avoid confusion.
Variable/Parameter Naming: You have a parameter named iter which you use as the number of times you iterate over the cube. It took a bit to identify this, as I see iter as an iterator, not the number of times you iterate. Renaming it  iterations makes it clearer about what that parameter is supposed to be.

Updated Code

"""
Module Docstring
A description of your program goes here
"""

def updated_trajectory_component(row):
    """
    Updated the trajectory component of the passed row

    :param row: Row to be updated

    """
    num = 1
    return row[0] + sum([num / i for i in row[1:]])

def updated_distance_component(row):
    """
    Updated the distance component of the passed row

    :param row: Row to be updated

    """
    return [i - row[0] for i in row[1:]]

def iterate_row(row):
    """
    Iterates over the passed row

    :param row: Row to be iterated

    """
    dist = updated_distance_component(row)
    return [updated_trajectory_component(row)] + dist

def iterate_slice(_slice):
    """
    Iterates over the rows in the passed slice

    :param _slice: Slice to be iterated over

    """
    return [iterate_row(i) for i in _slice]

def iterate_cube(cube):
    """
    Iterates over the slices in the passed cube

    :param cube: Cube to be iterated over

    """
    return [iterate_slice(i) for i in cube]

def num_iterator(cube, iterations):
    """
    Recursive iterator, iterates over the cube `iterations` times

    :param cube: Cube to iterate
    :param iterations: Number of times to iterate
    """
    print(cube)
    if iterations == 1:
        return iterate_cube(cube)
    return num_iterator(iterate_cube(cube), iterations - 1)

if __name__ == '__main__':
    CUBE = [
        [
            [0, 0.1, 1],
            [0, 1, 0.1],
            [0, 1, 1]
        ],
        [
            [0, 0.1, 1],
            [0, -1, 1],
            [0, -1, 0.1]
        ],
        [
            [0, -1, -1],
            [0, 0.1, -1],
            [0, -1, 0.1]
        ]
    ]

    #Sample Cases
    num_iterator(CUBE, 5)
    num_iterator(CUBE, 2)
    num_iterator(CUBE, 7)
    num_iterator(CUBE, 4)


ListBox Update Handling I have UserGroups for Users to be assigned to in a ListBox in HTML form. Items from listbox can be multi-selected or completely unselected.

In summary currently, I am getting a int[] of GroupId from client into my action and I am pulling existing assigned usergroups for the user into the action as well. Then I am comparing the Length of both int[] to determine whether UserGroups for the User was added or removed which is depicted in the following flow chart:

Actual code:

var existUserGroups = db.Users.Where(u => u.ID == user.ID).Single().UserGroups;
var existUserGroupsIds = db.Users.Where(u => u.ID == user.ID).Single().UserGroups.Select(ug => ug.ID).ToArray<int>();
var newUserGroupsIds = user.GroupId ?? new int[0];

//No change made
if (!existUserGroupsIds.SequenceEqual(newUserGroupsIds))
{
    //Add
    if (newUserGroupsIds.Length > existUserGroupsIds.Length)
    {
        for (int i = 0; i < newUserGroupsIds.Length; i++)
        {
            if (!existUserGroupsIds.Any(id => id == newUserGroupsIds[i]))
            {
                int id = newUserGroupsIds[i];
                existUserGroups.Add(db.UserGroups.Where(ug => ug.ID == id).Single());
            }
        }
    }
    //Remove
    else if (newUserGroupsIds.Length < existUserGroupsIds.Length)
    {
        var diff = existUserGroupsIds.Except(newUserGroupsIds);
        if (diff != null && diff.Count() > 0)
        {
            for (int i = 0; i < diff.Count(); i++)
            {
                int id = diff.ElementAt(i);
                existUserGroups.Remove(db.UserGroups.Where(ug => ug.ID == id).Single());
            }
        }
    }
    //Update
    else
    {
        existUserGroups.Remove(existUserGroups.ElementAt(0));
        int id = newUserGroupsIds[0];
        existUserGroups.Add(db.UserGroups.Where(ug => ug.ID == id).Single());
    }

    db.SaveChanges();
}

This is working but the code seems to be very tangled. So, please suggest how I can improve this code or how devs usually handle such situations.

Small Code duplication

var existUserGroups = db.Users.Where(u => u.ID == user.ID).Single().UserGroups;
var existUserGroupsIds = db.Users.Where(u => u.ID == user.ID).Single().UserGroups.Select(ug => ug.ID).ToArray<int>();

can be reduced to

var existUserGroups = db.Users.Where(u => u.ID == user.ID).Single().UserGroups;
var existUserGroupsIds = existUserGroups.Select(ug => ug.ID).ToArray<int>();

The //Add part

Should be simplified using a foreach loop and made more readable by using the Contains() method

foreach (int newId in newUserGroupsIds)
{
    if (!existUserGroupsIds.Contains(newId))
    {
        existUserGroups.Add(db.UserGroups.Where(ug => ug.ID == newId).Single());
    }
}

The //Remove part

var diff = existUserGroupsIds.Except(newUserGroupsIds);

diff will never be null => the null check can be skipped
using a for..each instead of a for loop => the check for diff.Count() > 0 can be skipped

var diff = existUserGroupsIds.Except(newUserGroupsIds);
foreach (int id in diff)
{
    existUserGroups.Remove(db.UserGroups.Where(ug => ug.ID == id).Single());
}


Returning a description based on a minimum and a maximum value I need to return a "human readable" description based on a minimum and a maximum value. Note that the minimum is included in the condition, but the maximum is excluded -- so for instance if the parameters are 6 and 9, the valid values are 6, 7 and 8, but not 9. The maximum value is also nullable.

I started out with nested switch statements, but I quickly abandoned them because they were unreadable and would be hard to maintain. Instead I kept the original switch, but moved each nested switch to its own method.

Note that the code currently supports only a handful of cases, but these are the ones currently needed. Though it is possible others will need to be added in the future and thus I'd like to have something that would be easy to maintain.

The code as featured below doesn't feel "mature" to me, but I don't see another solution right now. Perhaps the Strategy Pattern could be a solution, but it feels a bit overkill for this purpose.

internal class InterpretationRetriever
{
    public static string Execute(int minimum, int? maximum)
    {
        switch (minimum)
        {
            case 0:
                return MinimumIsZero(maximum);
            case 1:
                return MinimumIsOne(maximum);
            case 3:
                return MinimumIsThree(maximum);
            case 5:
                return MinimumIsFive(maximum);
            case 6:
                return MinimumIsSix(maximum);
            case 9:
                return MinimumIsNine(maximum);
            case 10:
                return MinimumIsTen(maximum);
            default:
                return null;
        }
    }

    private static string MinimumIsZero(int? maximum)
    {
        if (!maximum.HasValue)
        {
            return "Zero or more";
        }

        switch (maximum)
        {
            case 1:
                return "Less than one";
            case 3:
                return "Less than three";
            case 5:
                return "Less than five";
            default:
                return null;
        }
    }

    private static string MinimumIsOne(int? maximum)
    {
        if (!maximum.HasValue)
        {
            return "One or more";
        }
        return null;
    }

    private static string MinimumIsThree(int? maximum)
    {
        if (!maximum.HasValue)
            return null;

        switch (maximum)
        {
            case 6:
                return "From three to five";
            default:
                return null;
        }
    }

    private static string MinimumIsFive(int? maximum)
    {
        if (!maximum.HasValue)
            return null;

        switch (maximum)
        {
            case 10:
                return "From five to nine";
            default:
                return null;
        }
    }

    private static string MinimumIsSix(int? maximum)
    {
        if (!maximum.HasValue)
            return null;

        switch (maximum)
        {
            case 9:
                return "From three to eight";
            default:
                return null;
        }
    }

    private static string MinimumIsNine(int? maximum)
    {
        if (!maximum.HasValue)
        {
            return "Nine or more";
        }
        return null;
    }

    private static string MinimumIsTen(int? maximum)
    {
        if (!maximum.HasValue)
        {
            return "Ten or more";
        }
        return null;
    }
}

Perhaps instead of this convoluted branching, you could simply handle the three cases: whether there is a maximum and a non-zero minimum, a maximum and a minimum of 0 or there is no maximum.

private static Dictionary<int, string> humanReadableNumbers = new Dictionary<int,string>()
    {
        {0, "zero"},
        {1, "one"},
        {2, "two"},
        //etc...
    };

public static string Execute(int minimum)
{
    return string.Format("{0} or more", CultureInfo.CurrentCulture.TextInfo.ToTitleCase(humanReadableNumbers[minimum]));
}

public static string Execute(int minimum, int maximum)
{
    return minimum == 0 ? string.Format("Less than {0}", humanReadableNumbers[maximum]) : string.Format("From {0} to {1}", humanReadableNumbers[minimum], humanReadableNumbers[maximum-1]);
}

With this method, adding extra numbers is very simple. I've additionally chosen to split the method into two methods, one for no maximum, and one for a maximum. I feel it shows the different use-cases better, but if it is too much work to refactor all uses of it, this will work instead:

public static string Execute(int minimum, int? maximum)
{
    if(maximum.HasValue())
    {
        return string.Format("{0} or more", CultureInfo.CurrentCulture.TextInfo.ToTitleCase(humanReadableNumbers[minimum]))
    }
    else
    {
        return minimum == 0 ?
            string.Format("Less than {0}", humanReadableNumbers[maximum]) :
            string.Format("From {0} to {1}", humanReadableNumbers[minimum], humanReadableNumbers[maximum-1]);
    }
}


Parallel Task Queue that runs in sequence I'm in need of a solution that runs constantly incoming requests in a per-resource sequence, but parallel in general.

The use-case:

Many clients connect to a server and start issuing work. The work of a single client needs to run in sequential order, so the downward code doesn't need to cope with concurrency, but in general all work should be run on multiple threads. I'm trusting the .NET framework a lot here, which I hope is a good thing.

I've also read into DataFlow and parallel Rx but could not find a general solution there. But hints into that direction are welcome!

class TaskGroup
{
    public int CurrentlyQueuedTasks { get { return _currentlyQueued; } }

    private readonly object _previousTaskMonitor;
    private Task _previousTask;
    private int _currentlyQueued;

    public TaskGroup()
    {
        _previousTaskMonitor = new object();
        _previousTask = Task.CompletedTask;
    }

    public void Append(Action action)
    {
        lock(_previousTaskMonitor)
        {
            Interlocked.Increment(ref _currentlyQueued);
            _previousTask = _previousTask.ContinueWith(task =>
            {
                try
                {
                    action();
                }catch(Exception)
                {
                    //TODO
                }
                finally
                {
                    Interlocked.Decrement(ref _currentlyQueued);
                }
            });
        }
    }
}

This is an interesting approach. I would have used a queue by default since it seems to express the semantics a bit clearer (the queuing is slightly more obvious). Also ContinueWith creates a Task wrapping the original task which I'm not sure if that has any form of performance downsides (it probably shouldn't). I hacked a quick benchmark together with the alternative being implemented using the BlockingCollection:

using System;
using System.Diagnostics;
using System.Threading.Tasks;
using System.Threading;
using System.Collections.Concurrent;

public interface IAppendable
{
    void Append(Action action);
}

public class TaskGroup : IAppendable
{
    public int CurrentlyQueuedTasks { get { return _currentlyQueued; } }

    private readonly object _previousTaskMonitor;
    private Task _previousTask;
    private int _currentlyQueued;

    public TaskGroup()
    {
        _previousTaskMonitor = new object();
        _previousTask = Task.FromResult(false);
    }

    public void Append(Action action)
    {
        lock(_previousTaskMonitor)
        {
            Interlocked.Increment(ref _currentlyQueued);
            _previousTask = _previousTask.ContinueWith(task =>
            {
                try
                {
                    action();
                }catch(Exception)
                {
                    //TODO
                }
                finally
                {
                    Interlocked.Decrement(ref _currentlyQueued);
                }
            });
        }
    }
}

public class QueueAppendable : IAppendable, IDisposable
{
    public int CurrentlyQueuedTasks { get { return _Queue.Count; } }

    BlockingCollection<Action> _Queue = new BlockingCollection<Action>();

    public QueueAppendable()
    {
        Task.Factory.StartNew(() =>
        {
            while (true)
            {
                try
                {
                    var action = _Queue.Take();
                    action();
                }
                catch (InvalidOperationException)
                {
                    break;
                }
                catch
                {
                    // TODO log me
                }
            }
        });
    }

    public void Append(Action action)
    {
        _Queue.Add(action);
    }

    public void Dispose()
    {
        _Queue.CompleteAdding();
    }
}

public class Test
{
    public static void TimeIt(string name, IAppendable appendable)
    {
        var finishEvent = new ManualResetEvent(false);
        var sw = new Stopwatch();
        sw.Start();
        for (int i = 0; i < 2000; ++i)
        {
            appendable.Append(() => { Thread.Sleep(1); });
        }
        appendable.Append(() => { finishEvent.Set(); });
        finishEvent.WaitOne();
        sw.Stop();
        Console.WriteLine("{0} elapsed time: {1}ms", name, sw.ElapsedMilliseconds);
        (appendable as IDisposable)?.Dispose();
    }

    public static void Main()
    {
        TimeIt("TaskGroup", new TaskGroup());
        TimeIt("Queue", new QueueAppendable());
    }
}

Output:

TaskGroup elapsed time: 2135ms
Queue elapsed time: 2121ms

So there is pretty much no performance difference between the two however I think the BlockingCollection approach has a few advantages:

Easier to debug. You can simply set a break point and peek the queue. This is quite difficult to do with the wrapped task approach.
No use use of lower level synchronization primitives. The first time I read your code I instinctively thought "Hang he's got a lock why the Interlocked calls" until I realized that the decrement happen in the async task outside of the lock. With the BlockingQueue you program against a slightly higher level of abstraction which is often a good thing.
Fewer class members which reduce the state complexity of the object (the queue is the only member).

Apart from that I think your approach should be fine. You may want to consider adding support for cancellation via CancellationToken


compare different pandas dataframes to find matching rows I have two dataframes: One contains of name and year.

**name** **year**

ram       1873

rob       1900

Second contains names and texts.

**name** **text**

ram      A good kid

ram     He was born on 1873

rob     He is tall

rob     He is 12 yrs old

rob     His father died at 1900

I want to find the indices of the rows of second dataframe where the name of second dataframe matches with name of the first df and the text in second df contains the year in first df.

The result should be indices 1,4

My Code:

ind_list = []
for ind1, old in enumerate(A.name):
    for ind2, new in enumerate(B.name):
        if A.name[ind1] == B.name[ind2]:
            if A.year[ind1] in B.text[ind2]:
                ind_list.append(ind2)

Any better way to write the above code?

Here is what we start with.

In [16]: df1
Out[16]:
  name  year
0  ram  1873
1  rob  1900

In [17]: df2
Out[17]:
  name                     text
0  ram               A good kid
1  ram      He was born on 1873
2  rob               He is tall
3  rob         He is 12 yrs old
4  rob  His father died at 1900

What you probably want to do is merge your two DataFrames.  If you're familiar with SQL, this is just like a table join.  The pd.merge step essentially "adds" the columns from df1 to df2 by checking where the two DataFrames match on the column "name".  Then, once you have the columns you want ("year" and "text") matching according to the "name" column, we apply the function lambda x: str(x.year) in x.text (which checks if the year is present in the text) across the rows (axis=1).

In [18]: cond = pd.merge(
    ...:     left=df2,
    ...:     right=df1,
    ...:     how="left",
    ...:     left_on="name",
    ...:     right_on="name",
    ...: ).apply(lambda x: str(x.year) in x.text, axis=1)

This gives us a Series which has the same index as your second DataFrame, and contains boolean values telling you if your desired condition is met or not.

In [19]: cond
Out[19]:
0    False
1     True
2    False
3    False
4     True
dtype: bool

Then, we filter your Series to where the condition is true, and give the index, optionally converting it to a list.

In [20]: cond[cond].index
Out[20]: Int64Index([1, 4], dtype='int64')
In [21]: cond[cond].index.tolist()
Out[21]: [1, 4]

If all you need later on is to iterate over the indices you've gotten, In [18] and In [20] will suffice.


Encryption scripts using the module cryptography Its meant to password protect python scripts by using encryption
It should use os.urandom(40)'s output in hex as the salt. The kdf (cryptography.io) is using Scrypt (cryptography.io) password input is using getpass.getpass. It's on my GitHub: Python Script Locker (GitHub), and is required to work with both Python 2 and 3.

#!/usr/bin/env python
import sys
def exitmsg(msg):
    print(msg)
    input("Press ENTER to exit the script")
    sys.exit()
if sys.version_info<(3,0,0):
    def input(string):
         return raw_input(string)
import base64
import binascii
import os
import getpass
try:
    from cryptography.fernet import Fernet
except ImportError:
    exitmsg("cryptography not installed install it with pip install cryptography via cmd or powershell (On Windows)")
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives.kdf.scrypt import Scrypt
print("PyLock beta v1.0.3 https://github.com/OfficialNoob/Python-Script-Locker")
salt = binascii.hexlify(os.urandom(40))
kdf = Scrypt(salt=salt,length=32,n=2**14,r=8,p=1,backend=default_backend())
loc = input("Script to use: ")
try:
    fscript = open(loc)
except IOError:
    exitmsg("Unable to read file")
script = fscript.read()
fscript.close
print("Can be used to overwrite your script")
sloc = input("Save as: ")
nc = '''#!/usr/bin/env python
#Made using PyLock https://github.com/OfficialNoob/Python-Script-Locker
import sys
def exitmsg(msg):
    print(msg)
    input("Press ENTER to exit")
    sys.exit()
if sys.version_info<(3,0,0):
    def input(string):
         return raw_input(string)
import getpass
import base64
try:
    from cryptography.fernet import Fernet
except ImportError:
    exitmsg("cryptography not installed install it with pip install cryptography via cmd or powershell (On Windows)")
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives.kdf.scrypt import Scrypt
kdf = Scrypt(salt=%s,length=32,n=2**14,r=8,p=1,backend=default_backend())
try:
    exec(Fernet(base64.urlsafe_b64encode(kdf.derive(getpass.getpass("Password to use: ").encode()))).decrypt(%s))
except Exception as ex:
    if(type(ex).__name__ == "InvalidToken"):
        exitmsg("Wrong password (-:")
    print(ex)''' % (salt, Fernet(base64.urlsafe_b64encode(kdf.derive(getpass.getpass("Password to use: ").encode()))).encrypt(script.encode()))
try:
    f = open(sloc,"w+")
    f.write(nc)
except IOError:
    exitmsg("Unable to write to file")
f.close
exitmsg("Your file has been created")

The good news is that as far as I can tell, the cryptography is correct (password-based key derivation function, random salt). The bad news is that your code is hard to read, and so it's hard to make sure that it's correct.

Please read PEP 8. It's good advice. The first thing one sees is 62 lines and not a single blank line. Use blank lines between function definitions. And on a related note, use auxiliary functions with a clearly-defined task. Avoid very long lines (80 columns maximum is a good idea), and break up long chains like Fernet(base64.urlsafe_b64encode(kdf.derive(getpass.getpass("Password to use: ").encode()))).encrypt(script.encode())) into manageable pieces. Because the code is hard to read, don't expect a complete review.

def exitmsg(msg):
    print(msg)
    input("Press ENTER to exit")
    sys.exit()

Don't require the user to press a key to exit the program. And don't print anything when you don't have anything to say. Standard output is for program output and standard error is for useful messages. This function doesn't do anything useful. Replace exitmsg(msg) by sys.exit().

    def input(string):

Don't override standard functions.

    exitmsg("cryptography not installed install it with pip install cryptography via cmd or powershell (On Windows)")

punctuation is important your message is incomprehensible language is not just a bunch of words thrown together

        exitmsg("Wrong password (-:")

Print error messages to standard error, not standard output. And when there's an error, exit with a nonzero status.

loc = input("Script to use: ")

Don't read script arguments from standard input. Read them from sys.argv. That's what it's for.

try:
    exec(…)
except Exception as ex:
    if(type(ex).__name__ == "InvalidToken"):
        exitmsg("Wrong password (-:")
    print(ex)

You're reporting a “wrong password” error for a certain class of errors. It's not clear to me what you're trying to do with InvalidToken. Rather than hope that the wrong key will result in a particular kind of parse error, start the ciphertext with a known value and check that known value. Since you know that the ciphertext is Python code, start the ciphertext wih a Python comment line.

plaintext = fernet.decrypt(ciphertext)
if not ciphertext.startswith('# OfficialNoob PasswordScriptLocker ciphertext version 000000001\n'):
    raise IncorrectPassword

And once again, error messages go on standard error, and the script must exit with a nonzero status on error.

Of course the whole thing is pretty pointless. Entering a password on each use of the script would be annoying. Using gpg to decrypt the script once and for all would be more flexible (you can use keys instead of a password), more convenient, and more secure (less risk of implementation errors).


Flask Database Design with ORM I am designing a Stock Currency application and for that, I created a database.
I searched for my question  here  first but the answerer told me to ask the same question here.
flask_sqlalchemy import SQLAlchemy
db = SQLAlchemy()

class Stock_Data(db.Model):
    __tablename__ = 'stock_datas'
    # Column names start with capital letter for convention to use data easier (for now)
    id = db.Column(db.Integer(), primary_key=True)
    Date = db.Column(db.DateTime, nullable=False)
    Open = db.Column(db.Float(), nullable=False)
    High = db.Column(db.Float(), nullable=False)
    Low = db.Column(db.Float(), nullable=False)
    Currency_Close = db.Column(db.Float(), nullable=False)
    Volume = db.Column(db.Integer(), nullable=False)

    # Foreign Key
    stock_id = db.Column(db.Integer, db.ForeignKey('stocks.id'))

    # Foreign Attribute To Reach
    stock = db.relationship('Stock', backref='Stock_Data', primaryjoin='Stock_Data.stock_id==Stock.id', lazy=True)

class Stock(db.Model):
    __tablename__ = 'stocks'
    id = db.Column(db.Integer(), primary_key=True)
    symbol = db.Column(db.String(10), unique=True)
    name = db.Column(db.String(50), unique=True)
    sector = db.Column(db.String(10), nullable=True)
    currency = db.Column(db.String(3), nullable=False)

class Parity_Data(db.Model):
    __tablename__ = 'parity_datas'
    id = db.Column(db.Integer(), primary_key=True)
    Parity_Close = db.Column(db.Float())
    Date = db.Column(db.DateTime, nullable=False)

    # Foreign Key
    parity_id = db.Column(db.Integer, db.ForeignKey('parities.id'))

    # Foreign Attribute To Reach
    parity = db.relationship('Parity', backref='Parity_Data', primaryjoin='Parity_Data.parity_id==Parity.id', lazy=True)

class Parity(db.Model):
    __tablename__ = 'parities'
    id = db.Column(db.Integer(), primary_key=True)
    parity_name = db.Column(db.String(8), unique=True)

I created this database design all classes are in separate files. Sotck is related to Stock_Data (1-N) and Parity is related to Parity_Data (1-N) as seen.
I am creating a connection between Stock and Parity by
string_stock = 'APPL'
stock = session.query(Stock).filter(Stock.symbol == str(string_stock).upper()).first()
stock_data = session.query(Stock_Data).filter(Stock_Data.stock_id == stock.id).all()
parity = session.query(Parity).filter(Parity.parity_name.endswith(stock.currency)).first()
parity_data = session.query(Parity_Data).filter(Parity_Data.parity_id==parity.id).all()

By this method, I can fetch all stock_data and parity_data separately. However, when I try to merge stock_data and parity_data I get two lists and cannot be merged.
After that, I tried
data_joined_on_time = session.query(Stock_Data).join(Parity_Data, Stock_Data.Date == Parity_Data.Date).all()
dir(session.query(Stock_Data).join(Parity_Data, Stock_Data.Date == Parity_Data.Date).all()[0])

This method joins data but the attributes:
['Currency_Close',
'Date',
'High',
'Low',
'Open',
'Volume',
'class',
'delattr',
'dict',
'dir',
'doc',
'eq',
'format',
'ge',
'getattribute',
'gt',
'hash',
'init',
'init_subclass',
'le',
'lt',
'mapper',
'module',
'ne',
'new',
'reduce',
'reduce_ex',
'repr',
'setattr',
'sizeof',
'str',
'subclasshook',
'table',
'tablename',
'weakref',
'_decl_class_registry',
'_sa_class_manager',
'_sa_instance_state',
'id',
'metadata',
'query',
'query_class',
'stock',
'stock_id']
There is nothing about Parity_Data part.
My question consists of two parts:

I could not think of another way for my database design.Is my database design incorrect?
Is there a way to merge those two (stock_data and parity_data) by SQLAlchemy? I would like to filter first and join after it.
If there is no way I will merge them on pandas dataframe but firstly I want to try it on SQLAlchemy.

At the end of the day I used pandas.
df = pandas.read_sql_query("""My join query""")

This all just solved my problem.


Synchronized LinkedHashed map I've written the following code ages ago (10+ years) which is part of a simple chat server. I'm going to refactor it a bit in Java and then for fun I'm going to convert it to Scala and use Akka actors instead of threading.

Do you see any thread issues that can arise from the method of synchronization?

If a thread appends a record and another thread performs a read on the thread that reads, is it not going to have the current data?

Upon reading this article, I assume that is not the case?

I could use a ConcurrentHashMap instead but it doesn't preserve the order of insertion.

final class UserRecord
{
    //unsynchronized version of hashtable which maintains the order
    //which the objects where inserted.
    private LinkedHashMap container;

    public UserRecord()
    {
        container = new LinkedHashMap();
    }

    //return true if the user name is not taken
    synchronized boolean add(ClientHandler handler)
    {
        if (container.containsKey(handler.getUserName()) == true)
            return false;

        container.put(handler.getUserName(), handler);
        return true;
    }

    synchronized void remove(ClientHandler handler)
    {
        container.remove(handler.getUserName());
    }

    synchronized int size()
    {
        return container.values().size();
    }

    synchronized ClientHandler getHandler(String userName)
    {
        return (ClientHandler) container.get(userName);
    }

    synchronized Iterator getIterator()
    {
        return container.values().iterator();
    }

    synchronized void clear()
    {
        this.container.clear();
    }
}

Yes there is a thread safety issue. While the getIterator() is synchronized, the Iterator it returns is not. LinkedHashMap supports removal through the values() view, and hence the Iterator can be used to modify the Map without proper synchronization.

  UserRecord userRecord = new UserRecord();
  populate(userRecord);

  Iterator iterator = userRecord.getIterator();

  iterator.next();
  iterator.remove(); // unsynchronized modification of the Map

The easiest solution is to use Collections.synchronizedMap() which basically does what you attempted.

private Map<String, ClientHandler> container;

public UserRecord()
{
    container = Collections.synchronizedMap(new LinkedHashMap<>());
}

If you do this, the methods on UserRecord no longer need to be synchronized.

As an aside, you will want to properly use generics.


Infinite producer using BlockingCollection Please have a review of this Code chunk and let me know whether it is a good way for infinite Producer?

UniqueStrings = new BlockingCollection<string>(10);
        Task.Run(() =>
        {
            while (true)
            {
                try
                {
                    UniqueStrings.Add(GetNextUniqueStringNumber("RCH04"));
                }
                catch (OperationCanceledException e)
                {
                    Console.WriteLine("ERROR_UNIQUE_STRING_ADD: {0}", e.Message);
                }
            }
        });

Where GetNextUniqueStringNumber("RCH04") gives unique string every time. In other words, I want Unique string readily available every time Consumer takes from it.

EDIT:

public void Process()
    {
        while (!ProcessedCollection.IsCompleted)
        {
            try
            {
                string stringId = UniqueStrings.Take();
                Console.WriteLine("Unique String: {0}", stringId);
                // TODO - Use this String Id for further processing
            }
            catch (OperationCanceledException e)
            {
                Console.WriteLine("DATABASE_MANAGER: Operation Canceled. " + e.Message);
            }
        }
    }

Looks Good To Me.

I was initially confused by this code, so I would want to rename the collection from UniqueStrings to something like UniqueStringBuffer, to make the purpose clear. To me, "buffer" says "I will use this object to hold data temporarily, because the data is consumed at a different rate than it is produced" - which is exactly what you have here.

If you want to take the next step in that direction... I would note that a BlockingCollection can be used for many things, but the only method that should be called on your instance is Take. For that reason, you may want to encapsulate this code inside of a class, with Take (or perhaps something more explicit like GetNext) as its only public method. That would serve the dual purpose of preventing misuse and, again, making the purpose clear. Whether or not you want to go that far is up to you.


Calculating amount of diet soda a dieter can drink without dying The following question was taken from Absolute Java 5th ed. by Walter Savitch:

  A government research lab has concluded that an artificial sweetener commonly used in diet soda pop will cause death in laboratory mice. A friend of yours is desperate to lose weight but cannot give up soda pop. Your friend wants to know how much diet soda pop it is possible to drink without dying as a result. Write a program to supply the answer. The program has no input but does have defined constants for the following items: the amount of artificial sweetener needed to kill a mouse, the weight of the mouse, the starting weight of the dieter, and the desired weight of the dieter. To ensure the safety of your friend, be sure the program uses the weight at which the dieter will stop dieting, rather than the dieter’s current weight, to calculate how much soda pop the dieter can safely drink. You may use any reasonable values for these defined constants. Assume that diet soda contains 1/10th of 1% artificial sweetener. Use another named constant for this fraction. You may want to express the percent as thedouble  value  0.001.  (If your program turns out not to use a defined constant, you may remove that defined constant from your program.)

This is the code that I have written:

public class Question4 {

    private static final double AMOUNT_OF_ARTIFICIAL_SWEETENER_TO_KILL_MOUSE_IN_LITRE = 0.001;
    private static final double WEIGHT_OF_MOUSE_IN_GRAMS = 25;
    private static final double DESIRED_WEIGHT_OF_DIETER_IN_GRAMS = 65000;
    private static final double CONCENTRATION_OF_ARTIFICIAL_SWEETENER_IN_DIET_SODA = 0.001;

    public static void main(String[] args) {

        System.out.println(maxAmountOfDietSodaDieterCanDrinkInLitre()
                + " litres.");// 2600.00 litres
    }

    public static double maxAmountOfDietSodaAMouseCanDrinkInLitre() {
        return AMOUNT_OF_ARTIFICIAL_SWEETENER_TO_KILL_MOUSE_IN_LITRE
                / CONCENTRATION_OF_ARTIFICIAL_SWEETENER_IN_DIET_SODA;
    }

    public static double maxAmountOfDietSodaDieterCanDrinkInLitre() {
        return (maxAmountOfDietSodaAMouseCanDrinkInLitre() * DESIRED_WEIGHT_OF_DIETER_IN_GRAMS)
                / WEIGHT_OF_MOUSE_IN_GRAMS;
    }
}

Each of the three methods runs correctly, and does so in constant time.  That's good.

Comments

You wrote one comment.  Either

remove the comment (because it could become inconsistent if the constants change and is therefore a maintenance headache), or
hard-code the answer and discard everything else (System.out.println("2600.0 litres.");), or
write the comment as asssertion instead (assert maxAmountOfDietSodaDieterCanDrinkInLitre() == 2600.0;)

Constants

The names of the constants and methods are extremely clear, which is good.  You might want to shorten them, though.  If so, I suggest:

private static final double DIETER_TARGET_KG = 65,
                            MOUSE_KG = 0.025,
                            MOUSE_SWEETENER_LD50_L = 0.001,
                            SWEETENER_CONCENTRATION_IN_SODA = 0.001;

That puts the members in an order where each constant is related to its neighbour.  As a bonus, they are in alphabetical order.  I've chosen to use SI units throughout.  "LD50" is standard notation in toxicology for the median lethal dose.

Methods

I think you could have decomposed the problem more logically.  For example, what quantity is being expressed by the parenthesized expression

(maxAmountOfDietSodaAMouseCanDrinkInLitre() * DESIRED_WEIGHT_OF_DIETER_IN_GRAMS)

… whose units are L ∙ g?

I would therefore write the solution as follows:

/**
 * Median lethal dose of artificial sweetener, in litres per kilogram of
 * the subject's body weight.
 */
public static double sweetenerLD50() {
    return MOUSE_SWEETENER_LD50_L / MOUSE_KG;
}

/**
 * Median lethal dose of diet soda, in litres per kilogram of the subject's
 * body weight.
 */
public static double sodaLD50() {
    return sweetenerLD50() / SWEETENER_CONCENTRATION_IN_SODA;
}

/**
 * Median lethal dose of diet soda, in litres.
 *
 * @param subjectKG subject's body weight, in kilograms
 */
public static double sodaLD50(double subjectKG) {
    return subjectKG * sodaLD50();
}

public static void main(String[] args) {
    System.out.println(sodaLD50(DIETER_TARGET_KG) + " litres.");
}

Note that unlike the // 2600.0 litres comment, JavaDoc serves as useful documentation, and is unlikely to become out of sync if the numbers change.


Generic wrapper for System.Runtime.Caching.MemoryCache I have implemented a simple generic wrapper for MemoryCache class. Its interface looks like ConcurrentDictionary class but I tried to keep it simple. I tried to keep operations atomic, hoping not to break the thread-safety. Is there a mistake about thread-safety? I also want to know if I there are some boxing-unboxing issues I have to think about. Any suggestions about API or some other thing  is appreciated.

public interface IMemoryCacheWrapper<T>
{
    string Name { get; }
    long CacheMemoryLimitInBytes { get; }
    long PhysicalMemoryLimit { get; }
    TimeSpan PollingInterval { get; }
    CacheItemPolicy CacheItemPolicy { get; set; }
    void AddOrUpdate(string key, T value);
    bool TryGetValue(string key, out T value);
    bool TryRemove(string key, out T value);
    void Remove(string key);
    bool ContainsKey(string key);
    long Count { get; }
    void Dispose();
    bool IsDisposed { get; }
}

public class MemoryCacheWrapper<T> : IMemoryCacheWrapper<T>
{
    private readonly MemoryCache _memoryCache;
    private CacheItemPolicy _cacheItemPolicy;
    private bool _isDisposed;

    public MemoryCacheWrapper(string name, NameValueCollection config = null)
    {
        _memoryCache = config != null ? new MemoryCache(name, config) : new MemoryCache(name);
        _isDisposed = false;
        CacheItemPolicy = new CacheItemPolicy { SlidingExpiration = new TimeSpan(1, 0, 0) };
    }

    public string Name
    {
        get { return _memoryCache.Name; }
    }

    public long CacheMemoryLimitInBytes
    {
        get { return _memoryCache.CacheMemoryLimit; }
    }

    public long PhysicalMemoryLimit
    {
        get { return _memoryCache.PhysicalMemoryLimit; }
    }

    public TimeSpan PollingInterval
    {
        get { return _memoryCache.PollingInterval; }
    }

    public CacheItemPolicy CacheItemPolicy
    {
        get
        {
            return _cacheItemPolicy;
        }
        set
        {
            if (value != null)
            {
                _cacheItemPolicy = value;
            }
        }
    }

    public void AddOrUpdate(string key, T value)
    {
        _memoryCache.Set(key, value, CacheItemPolicy);
    }

    public bool TryGetValue(string key, out T value)
    {
        bool result = false;
        value = default(T);

        object item = _memoryCache.Get(key);
        if (item != null)
        {
            value = (T)item;
            result = true;
        }

        return result;
    }

    public bool TryRemove(string key, out T value)
    {
        bool result = false;
        value = default(T);

        object item = _memoryCache.Remove(key);
        if (item != null)
        {
            result = true;
            value = (T)item;
        }

        return result;
    }

    public void Remove(string key)
    {
        _memoryCache.Remove(key);
    }

    public bool ContainsKey(string key)
    {
        return _memoryCache.Contains(key);
    }

    public long Count
    {
        get { return _memoryCache.GetCount(); }
    }

    public void Dispose()
    {
        _memoryCache.Dispose();
        _isDisposed = true;
    }

    public bool IsDisposed
    {
        get { return _isDisposed; }
    }
}

When implementing the Dispose interface it is advisable to use a two stage clean up. So that when you use this not inside a using clause the garage collection can also work correctly.

Technically you can get away with it here; but I find it is useful to always use this pattern. Maintenance/Bug fixing then becomes easier as you do not need to start messing with other stuff just update the Dispose method. Another reason is that MS recommends this.

    public ~MemoryCacheWrapper()
    {
        // Garbage collection has kicked in tidy up your object.
        Dispose(false);
    }

    // Implement IDisposable.
    public void Dispose()
    {
        // Dispose has been called clean up your object and members that
        // are disposable.
        Dispose(true);

        // Now Make sure that you don't call the cleanup again via the Finalizer
        // Effectively you are taking over garbage collection so make sure you
        // don't do it again
        GC.SuppressFinalize(this);
    }

    private void Dispose(bool disposing)
    {
        // Only do this once.
        if(!this._isDisposed)
        {
            // If called via IDispose interface then clean up sub-objects.
            // That implement the IDispose interface.
            if(disposing)
            {
                // Dispose managed resources.
                _memoryCache.Dispose();
            }

            // Now clean-up and objects that don't implement dispose.
            // i.e close any file handles

            // Currently nothing to do.
        }
        _isDisposed = true;
    }


Concise way of "updating" element in nested collections Imagine simple game:

type Combatant = {
    hp : int
    attack : int
}

type CombatantGroup = Combatant list
type CombatantGroups = CombatantGroup list

type Battle = {
    combatantGroups : CombatantGroups
}

there is some battle, in which some amount groups participate. In each group there is several combatants. Now I want to implement a function for one combatant  to attack another. Due to immutability, I have to replace whole attacked target, and whole group and whole battle. If the battle was just between two participants, I could write code like this:

if target = battle.combatant1 then
    { battle with combatant1 = { battle.combatant1 with hp = battle.combatant1.hp - attacker.attack }}
else
    { battle with combatant2 = { battle.combatant2 with hp = battle.combatant2.hp - attacker.attack }}

which is also terrible code, I would appreciate any advice on it. But with nested collections it gets even more complicated, and event doesn't look functional to me anymore:

let containsTarget = List.contains target
let transformCombatant combatant = if combatant = target then { combatant with hp = combatant.hp - attacker.attack } else combatant
let transformGroup = List.map transformCombatant
let checkGroup group =
    if group |> containsTarget then
        group |> transformGroup
    else
        group
let transformGroups = List.map checkGroup

{ battle with combatantGroups = transformGroups battle.combatantGroups }

Can you please give me advise on how can I use features of F# and functional programming, to make given code more pretty and concise?

I think you need an id field to be sure that you're updating the correct combatant. I assume it's possible to have multiple combatants with the same hp and attack.
And in that case it makes sense to store a group as a Map of id to combatant. This makes it really simple to update a value by id and there's no need to check for existence first. See the tryUpdateCombatantById function below.
You can add small helper functions to update specific record fields with a function that is passed in.

Here's the code with all of those changes:

type Combatant = {
    id : int
    hp : int
    attack : int
}

type Battle = { combatantGroups : Map<int, Combatant> list }

let updateHp f combatant = { combatant with hp = f combatant.hp }
let removeHp attack = updateHp (fun hp -> hp - attack)

let tryUpdateCombatantById combatantId f group =
    group
    |> Map.tryFind combatantId
    |> Option.map (fun c -> group |> Map.add c.id (f c))
    |> Option.defaultValue group

let applyAttack targetId attacker battle =
    { battle with
        combatantGroups =
            battle.combatantGroups
            |> List.map (tryUpdateCombatantById targetId (removeHp attacker.attack)) }


Convert to Roman numerals in Ruby Challenged by Sandy Metz's newsletter, I've tried to implement this kata from exercism. However, my solution, which passes the tests, is quite a lot simpler than Sandy's solution. Ok, so she's using refinements and also supports the inverse, but still... As she's probably smarter than me, do I miss something?

class Fixnum
  ROMAN = {
    1000 => "M",
    900 => "CM",
    500 => "D",
    400 => "CD",
    100 => "C",
    90 => "XC",
    50 => "L",
    40 => "XL",
    10 => "X",
    9 => "IX",
    5 => "V",
    4 => "IV",
    1 => "I"
  }

  def to_roman
    result = ""
    ROMAN.reduce(self) { |number, (divider, letter)|
      letter_multiplier, remainder = number.divmod(divider)
      result << (letter * letter_multiplier)
      remainder
    }
    result
  end
end

I like your solution. You usually see it done with while/for loops that substract multipliers instead of taking the quotient/remainder. Some notes about your code:

ROMAN.reduce(self) { |number, (divider, letter)|. In a multiline block is idiomatic to use do/end.
Prefer building an array + join at the end, that appending to a string. Not that it matters with those sizes, but in general it's the recommended way (+= in a string is typically O(n^2), << on an array is O(n)).
inject is a functional abstraction. While you can perform in-place updates within the block, it will break the assumption of many readers. One solution is to encapsulate the output within the state (using a hash, for example).

I'd write:

class Fixnum
  ROMAN = {...}

  def to_roman
    ROMAN.reduce(number: self, result: []) do |state, (divider, letter)|
      letter_multiplier, remainder = state[:number].divmod(divider)
      new_result = state[:result] + [letter * letter_multiplier]
      {number: remainder, result: new_result}
    end[:result].join
  end
end

Note that this is a state machine: it iterates over an input with an initial state, on each loop the state is updated, and the result is the final state.


Simple command line parser/dispatcher in Rust I've written a very simple command line parser/dispatcher library. This is an extension of a previous review, which I've refactored, found here. Basically, a user can add functions to a Cmdr struct, then invoke them using a &str. I've made the library generic so that a user can define their own Result<T, E> to be returned from the functions added to Cmdr.

I appreciate any and all comments, especially those related to Rust best practices. One thing that slightly irks me is that I'm not sure that Cmdr should be living in the root lib.rs file. But, if I move it to say, cmdr.rs, my namespace would be something stupid like cmdr::cmdr::Cmdr, as my root directory is also named cmdr. Anyway, behold.

cmd.rs

pub struct Cmd<T, E> {
    pub name: String,
    pub invocation: Box<FnMut() -> Result<T, E>>,
}

impl<T, E> Cmd<T, E> {
    pub fn new<F: 'static + FnMut() -> Result<T, E>>(invoke_str: &str, invocation: F) -> Cmd<T, E> {
        Cmd {
            name: String::from(invoke_str),
            invocation: Box::new(invocation),
        }
    }

    pub fn invoke(&mut self) -> Result<T, E> {
        (self.invocation)()
    }
}

lib.rs

pub mod cmd;
use cmd::Cmd;

pub struct Cmdr<T, E> {
    cmds: Vec<Cmd<T, E>>,
}

impl<T, E> Cmdr<T, E> {
    pub fn new() -> Cmdr<T, E> {
        Cmdr { cmds: Vec::new() }
    }

    pub fn add<F: 'static + FnMut() -> Result<T, E>>(&mut self, name: &str, cmd: F) {
        self.cmds.push(Cmd::new(name, cmd));
    }

    pub fn invoke(&mut self, cmd_name: &str) -> Option<Result<T, E>> {
        let cmd_to_invoke = self.cmds.iter_mut().find(|cmd| cmd.name == cmd_name);
        if let Some(cmd) = cmd_to_invoke {
            Some(cmd.invoke())
        } else {
            None
        }
    }
}

mod tests {
    #[test]
    fn cmdr() {
        let mut cmdr: super::Cmdr<&str, &str> = super::Cmdr::new();
        cmdr.add("test1", || Ok("test1 executed."));
        cmdr.add("test2", || Ok("test2 executed."));

        // Good commands.
        let test1_msg = cmdr.invoke("test1").unwrap().expect("Error in test1.");
        assert!(test1_msg == "test1 executed.", "Incorrect test1 message.");
        let test2_msg = cmdr.invoke("test2").unwrap().expect("Error in test2.");
        assert!(test2_msg == "test2 executed.", "Incorrect test2 message.");

        // Bad command.
        if let Some(_) = cmdr.invoke("ghost") {
            assert!(false, "Non-existent command somehow returned Some().");
        }
    }
}

Some plans for the future are to...

Enable the library to actually utilize FnMut closures. The framework is in place, but I currently don't have a strong enough grasp of lifetimes to implement this.
Add sub-commands and command flags. It will be an interesting exercise in using the ? operator to propagate errors.
Document the code in a Rusty style. I haven't delved in to code based doc tags, as of yet.
Eventually, this will be a front end for a DHCP client/server package I'd like to implement in Rust. I may also release it on crates.io, if it matures into something worthwhile.

Prefer where clauses when your bounds get the tiniest bit complicated
An if let with an else is often better as a match.
If you match on an Option and return an Option, just use Option::map.
This allows you to remove the temporary variable and just chain functions
The tests module should have #[cfg(test)] to prevent it from even being compiled when not testing.
It's typical to use super::* in a test module.
It's better to let the compiler infer types instead of being explicit. This shows as an error because the compiler can't infer the error type. You could say cmdr: Cmdr<_, &str>, but it's better to actually exercise the error case.
Instead of unwrapping your success / failure, assert against them.
Use assert_eq instead of assert when comapring equality.
Use assert! and is_none instead of pattern matching

lib.rs

pub mod cmd;
use cmd::Cmd;

pub struct Cmdr<T, E> {
    cmds: Vec<Cmd<T, E>>,
}

impl<T, E> Cmdr<T, E> {
    pub fn new() -> Cmdr<T, E> {
        Cmdr { cmds: Vec::new() }
    }

    pub fn add<F>(&mut self, name: &str, cmd: F)
    where
        F: FnMut() -> Result<T, E> + 'static,
    {
        self.cmds.push(Cmd::new(name, cmd));
    }

    pub fn invoke(&mut self, cmd_name: &str) -> Option<Result<T, E>> {
        self.cmds.iter_mut()
            .find(|cmd| cmd.name == cmd_name)
            .map(|cmd| cmd.invoke())
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn cmdr() {
        let mut cmdr = Cmdr::new();
        cmdr.add("test1", || Ok("test1 executed."));
        cmdr.add("test2", || Err(42));

        // Good commands.
        let test1_msg = cmdr.invoke("test1");
        assert_eq!(test1_msg, Some(Ok("test1 executed.")), "Incorrect test1 message.");

        let test2_msg = cmdr.invoke("test2");
        assert_eq!(test2_msg, Some(Err(42)));

        // Bad command.
        assert!(cmdr.invoke("ghost").is_none(), "Non-existent command somehow returned Some().");
    }
}

cmd.rs

pub struct Cmd<T, E> {
    pub name: String,
    pub invocation: Box<FnMut() -> Result<T, E>>,
}

impl<T, E> Cmd<T, E> {
    pub fn new<F>(invoke_str: &str, invocation: F) -> Cmd<T, E>
    where
        F: FnMut() -> Result<T, E> + 'static,
    {
        Cmd {
            name: String::from(invoke_str),
            invocation: Box::new(invocation),
        }
    }

    pub fn invoke(&mut self) -> Result<T, E> {
        (self.invocation)()
    }
}

  I'm not sure that Cmdr should be living in the root lib.rs file.

In fact, I'd say the opposite is true: I'm not sure that Cmd belongs in it's own file. Unlike many languages, most Rust projects I've read have no hesitation in placing more than one type in a single file. I wouldn't think that Cmd carries enough weight to be separated.

There is some work underway to adjust Rust's module system, so the organization you've shown might return to vogue at some point.

  my namespace would be something stupid like cmdr::cmdr::Cmdr, as my root directory is also named cmdr

You can use the façade pattern to re-export nested types at a higher module:

// lib.rs
pub use cmdr::Cmdr;

  Option<Result<T, E>>

I'd encourage you to try this out a bit; my gut tells me that it won't be the most ergonomic to use.

  Document the code in a Rusty style

Remember that the static type system provides a baseline of documentation and that function and argument names provide a bit more. Don't document the things that are obvious from the name or type of something. Consider making more types to improve the documentation and user experience.

  Eventually, this will be a front end for a DHCP client/server package I'd like to implement in Rust. I may also release it on crates.io, if it matures into something worthwhile.

Please strongly consider not using your own hand-rolled library for such a project. Having a great DHCP server sounds wonderful, but one where the author's time has to be spent maintaining a command-line parsing library or  one that has a substandard command line interface would detract from it.

Of course, that shouldn't stop you from using it to experiment with Rust!


Project Euler #70: Time Efficiency Improvement The problem was to find which \$n\$ for which \$\varphi(n)\$ is a permutation of \$n\$ and \$n/\varphi(n)\$ is minimum,now as we all know that means (simple conclusions from mathematics):

\$\varphi(n)\$ should be a permutation of n.
\$n\$ should have least number of prime factors (2 cannot be one of the prime factor and \$n\$ cannot be prime)
The prime factors should be as large as possible. Or in other words, the number should be as large as possible.

So the problem reduces to finding all numbers which are multiples of 2 primes (if no such number is found then find one with 3 prime factors and then 4 and so on, but such a situation never occurs)

I move back from 3137 as first factor and 33..331 the other (378 and 239118 th index in primes array). I multiply 'em, add to a list and then sort it and move in descending order to see which are permutations.

Even then it is taking a lot of time. Several minutes at-least.

double ratio = 100;
    List<Integer> l = new ArrayList<Integer>();
    int[] primes = Helper.getPrimes(10000050);
    for (int i = 378; i >= 1; i--) {
        for (int j = 239118; j > i; j--) {
            BigInteger num = BigInteger.valueOf(primes[i]).multiply(BigInteger.valueOf(primes[j]));
            if (num.compareTo(BigInteger.TEN.pow(7)) < 1) {
                int x = num.intValue();
                System.out.println(primes[i] + " * " + primes[j] + " = " + x);
                l.add(x);
            }
        }
    }
    Collections.sort(l);
    for (int i = l.size() - 1; i >= 0; i--) {
        int x = l.get(i);
        int y = EulerTotient(x);
        boolean isPerm = isPermutation(x, y);
        if (isPerm) {
            if (x < y * ratio) {
                ratio = (double) x / (double) y;
                System.out.println(x);
            }
        }
    }
    return 0;

isPermutation:

private static boolean isPermutation(int x, int b) {
    if (String.valueOf(x).length() != String.valueOf(b).length()) {
        return false;
    } else {
        int[] count = new int[10];
        do {
            ++count[x % 10];
            --count[b % 10];
            x /= 10;
            b /= 10;
        } while (x != 0);// also b!=0
        for (int i = 0; i < 10; i++) {
            if (count[i] != 0) {
                return false;
            }
        }
        return true;
    }
}

EulerTotient: (take care to create a primes array)

public static int EulerTotient(int n) {
    int x = n;
    for (int i = 0; primes[i] <= n; i++) {
        if (n % primes[i] == 0) {
            x /= primes[i];
            x *= (primes[i] - 1);
        }
    }
    return x;
}

getPrimes:

isPrime = new boolean[maxValue + 1];
    Arrays.fill(isPrime, true);
    for (int i = 2; i * i <= maxValue; i++) {
        if (isPrime[i]) {
            for (int j = i; j * i <= maxValue; j++) {
                isPrime[i * j] = false;
            }
        }
    }
ArrayList<Integer> primes = new ArrayList<Integer>();
    for (int j = 2; j < lim; j++) {
        if (isPrime(j)) {
            primes.add(j);
        }
    }
    return listToIntArray(primes);

Several minutes? My stupid approach takes 1.3 second. It uses precomputed factoring of numbers below 1e7, which itself takes 0.3 s.
List<Integer> l = new ArrayList<Integer>();
int[] primes = Helper.getPrimes(10000050);
for (int i = 378; i >= 1; i--) {
    for (int j = 239118; j > i; j--) {
        BigInteger num = BigInteger.valueOf(primes[i]).multiply(BigInteger.valueOf(primes[j]));
        if (num.compareTo(BigInteger.TEN.pow(7)) < 1) {
            int x = num.intValue();
            System.out.println(primes[i] + " * " + primes[j] + " = " + x);
            l.add(x);
        }
    }
}

BigInteger??? That's the culprit. The solution is a few millions and confortable fits into an int. Using long would give me a good feeling. Using Guava's LongMath.checkedMultiply would give a guaranty I'm not computing non-sense. And so would my saturated arithmetic.
BigInteger.valueOf(primes[i])

this should be done before the inner loop,
or better stored in an array

BigInteger.TEN.pow(7)

this is a contant and shouldn't be recomputing again and again

num.compareTo(BigInteger.TEN.pow(7)) < 1
So you want that num is at most 1e7 and are scared of it overflowing 9e18? How should this happen? Not at all as i * j <= 239118**2.
Another inefficiency: Whenever num gets too big, you know it'll be even bigger for a bigger prime. So if you were iterating upwards, you could use a break. For iterating downwards, you could use one division to determine the initial j.
int y = EulerTotient(x);
In order to compute the totient, you need the factorization. But you just have thrown the primes away! Use a map or alike.
Possibly you've lost several minutes already in this part and needn't read any further.

isPermutation
if (String.valueOf(x).length() != String.valueOf(b).length()) {
    return false;

This takes probably more time than the remaining computation, which is fine. I'd go for
    do {
        ++count[x % 10];
        --count[b % 10];
        x /= 10;
        b /= 10;
        if ((x != 0) != (b != 0)) return false; // different lengths
    } while (x != 0); // also b!=0

And I wouldn't call the arguments x and b as it's confusing (there are interchangeable so should be named similarly).
EulerTotient
Method names should start with a lowercase letter. Not even the best mathematicians deserve any exception.
This part is fine, except for that you need no such computation as you actually know the factorization when you create n. For this computation, you could add
n /= primes[i];

to the if-block and gain some speed due to an earlier loop exit.
getPrimes
No problem here. One can argue that in
        for (int j = i; j * i <= maxValue; j++) {
            isPrime[i * j] = false;
        }

the expression i*j gets computed twice or that you could keep the product in a variable and just add i on each iteration, but that's what JIT excels at. So it's fine as it is.


Make code more memory efficient, or shorter //rename files
    if(!empty($_FILES[$desktop_fieldname_1280x800]['name'])){
                file_exists($desktop_filename_1280x800 = mb_strtolower($desktop_dir_1280x800 . 'wall_desktop_1280x800_' .
                $_FILES[$desktop_fieldname_1280x800]['name']));
            }
    if(!empty($_FILES[$desktop_fieldname_1366x768]['name'])){
                file_exists($desktop_filename_1366x768 = mb_strtolower($desktop_dir_1366x768 . 'wall_desktop_1366x768_' .
                $_FILES[$desktop_fieldname_1366x768]['name']));
            }
    if(!empty($_FILES[$desktop_fieldname_1920x1080]['name'])){
                file_exists($desktop_filename_1920x1080 = mb_strtolower($desktop_dir_1920x1080 . 'wall_desktop_1920x1080_' .
                $_FILES[$desktop_fieldname_1920x1080]['name']));
            }

With the above code, basically what I'm doing is renaming the files properly + consistently. What my code seems to be bit redundant to me, I think this can be done with some kind of loop or arrays, but I'm a newbie and not sure how it will be. So would like to ask your help to make this code more memory efficient, or shorten the code in some way.

The code does not rename any files, it modifies the values of certain variables.

Using assignments as a parameter to your functions is confusing.

You call file_exists but do nothing with it.  It is supposed to be used to check for the existence of a file. Something like:

if (!file_exists($destination))
{
    rename($source, $destination);
}

To answer more you would need to show how the desktop_dir_xxx variables are calculated.


Simple setting retriever from cookies I have a cookie on my site with the following value:

  eyJGIjp0cnVlLCJBIjp0cnVlLCJUIjpmYWxzZSwiUyI6ZmFsc2UsIkciOmZhbHNlLCJYIjpmYWxzZX0%3D

I use this function to retrieve the value from the site so I can use it in the applications my site uses to greenlight certain elements or to block them.

function getCookieLawSettings() {
    return document.cookie.split(';')
            .map(c => c.trim().split('='))
            .filter(c => c.shift() == 'cookie_law')
            .map(c => JSON.parse(atob(decodeURIComponent(c.pop())))).pop()
}

What kind of improvements could I make on this code? Stylistic and functional.
Are there things I missed/gotcha's I should account for?

sample with dummy data:

document.not_cookie = "_ga=GA1.2.431922604.1523436815; _gid=GA1.2.1579280874.1523436815; cookie_law=eyJGIjp0cnVlLCJBIjp0cnVlLCJUIjpmYWxzZSwiUyI6ZmFsc2UsIkciOmZhbHNlLCJYIjpmYWxzZX0%3D";
function getCookieLawSettings() {
    return document.not_cookie.split(';')
            .map(c => c.trim().split('='))
            .filter(c => c.shift() == 'cookie_law')
            .map(c => JSON.parse(atob(decodeURIComponent(c.pop())))).pop()
}
console.log(getCookieLawSettings());

Since (I assume) there should only be a single cookie fragment called cookie_law you should use find instead of filter. filter will go through the whole array even if the result is the first element.
As it stands I think that it is bad for a filter method to modify the underlying data.
Also the number of parenthesis in the line
.map(c => JSON.parse(atob(decodeURIComponent(c.pop())))).pop()

is pretty scary. I would pull at least some of that out into another method.
update
In my code I usually split out my logic with a method to get any cookie, that way I can reuse it for other cookies. The logic also ends up being simpler.
function getCookieLawSettings() {
    return decodeCookie(getCookie('cookie_law'));
}

function getCookie(na,e) {
    var cookie = document.cookie.split(';')
            .map(c => c.trim().split('='))
            .filter(c => c[0] === name);
    return cookie ? cookie[1] : undefined;
}

function decodeCookie(data) {
    return JSON.parse(atob(decodeURIComponent(data)));
}

function encodeCookie(data) {
    return btoa(encodeURIComponent(JSON.stringify)));
}


Diagonal gradient optimization With the SDL library, I'm creating an SDL_Surface* at w and h dimensions.
SDL_Color is a struct with 4 unsigned char fields: a, r, g and b for alpha, red, green and blue. c1 is the origin color, c2 the destination color, and c is the color of the pixel at position (i,j).

That gradient works fine, but it is very basic and I'm just wondering if there is any way to optimize it, whether with a better SDL use or a faster algorithm.

SDL_Surface* DrawDescendingDiagonalGradient(SDL_Color& c1, SDL_Color& c2, int w, int h)    {
    SDL_Color c;
    SDL_Surface *s = SDL_CreateRGBSurface(0, w, h, 32, 0, 0, 0, 0);
    SDL_LockSurface(s);
    for (int j = 0; j < h; j++) {
        for (int i = 0; i < w; i++) {
            c.a = c1.a * (w + h - i - j) / (w + h) + c2.a * (i + j) / (w + h);
            c.r = c1.r * (w + h - i - j) / (w + h) + c2.r * (i + j) / (w + h);
            c.g = c1.g * (w + h - i - j) / (w + h) + c2.g * (i + j) / (w + h);
            c.b = c1.b * (w + h - i - j) / (w + h) + c2.b * (i + j) / (w + h);
            PutPixel32(s, i, j, SDL_MapRGBA(s->format, c.r, c.g, c.b, c.a));
        }
    }
    SDL_UnlockSurface(s);
    return s;
}

In addition to Janos's answer.

You're truncating the color value too early here:

  c.a = c1.a * (w + h - i - j) / (w + h) + c2.a * (i + j) / (w + h);

This results in a rounding error on each channel. Also this forces the compiler to perform two divisions per channel, as it cannot combine the divisions due to truncation rules of integer types.

For example:

int a = 1;
int b = 2;
int c = 3;

int result = a / c + b / c;

The result must be calculated as (a/c) + (b/c)=0 if optimized to (a+b)/c=1 the result will differ. And in your case you actually want (a+b)/c as the correct answer.

In your case you should do:

SDL_Surface* DrawDescendingDiagonalGradient(SDL_Color& c1, SDL_Color& c2, int w, int h){
    SDL_Color c;
    SDL_Surface *s = SDL_CreateRGBSurface(0, w, h, 32, 0, 0, 0, 0);
    SDL_LockSurface(s);
    int wh = w + h;
    for (int j = 0; j < h; j++) {
        for (int i = 0; i < w; i++) {
            int ij = i + j;
            c.a = (c1.a * (wh - ij) + c2.a * ij) / wh;
            c.r = (c1.r * (wh - ij) + c2.r * ij) / wh;
            c.g = (c1.g * (wh - ij) + c2.g * ij) / wh;
            c.b = (c1.b * (wh - ij) + c2.b * ij) / wh;
            PutPixel32(s, i, j, SDL_MapRGBA(s->format, c.r, c.g, c.b, c.a));
        }
    }
    SDL_UnlockSurface(s);
    return s;
}

This will make sure you get the correct rounding and remove one division per channel. Also when running the code un-optimized (while debugging) this should perform slightly better due to elimination of some subexpressions.


PLZ HELP MEH IMPROVA MEH LOLCAT CODEZ, WHUCH CALCULUTS FAWCTORIALS SUNCE MEH BEEN WRUTEN CODEZ IN MUNY DIFFRNT PROGRAMMING LANGUAGES, MEH THOUGHTZ THAT ME TRY TO WRITE CODEZ IN AN ESOTERIC LANGUAGE. MEH CODEZ CALCULUTS A FAWCTORIAL OF UH USR INPUTTED NUMBAH IN LOLCODE, LOL.

SOME OF MEH QUSTIONS:

IZ MEH CODEZS A IDMITIC LOLCAT PROGUM?
IZ THERE A GOODER WAY OF WRUTING MEH factorial FUNCSHUNS?

TRANSLATION:

Since I've been writing and trying programs in many different languages, I thought I'd give esoteric language a crack. So I used LOLCODE to write a program that calculates the  factorial of a user given number:

Questions for consideration:

Is my LOLCODE code idiomatic to the LOLCODE language?
Is there a easier way to write my factorial function?

LOLCAT.lol

BTW Calculates the factorial for a user given integer

BTW Author: Pythonic
BTW Version: 1.0

HAI 1.2
HOW DUZ I factorial YR n
    BOTH SAEM n AN 0, O RLY?
        YA RLY
            FOUND YR 1
        NO WAI
            FOUND YR PRODUKT OF n AN factorial DIFF OF n AN 1
    OIC
IF U SAY SO

HOW DUZ I main
    I HAS A user_input

    VISIBLE "ENTER AN NUMBAH"
    VISIBLE "TO FINDZ ITZ FAWCTORIAL:: "

    GIMMEH user_input

    VISIBLE "DE FAWCTORIAL OF " user_input " IZ:: " factorial user_input
    VISIBLE "KTHXBYE!"
IF U SAY SO

main
KTHXBYE

For those of you who are not fortunate enough to have a LOLCODE compiler, here is a link to a REPL.IT of the code.

NOTE: All answer must be given in proper LOLCODE English, with an optional translation in regular English. Failure to do so will result in me releasing my LOLCATS upon you....

Your factorial function enters an infinite loop if passed a negative integer. Since the entire program seems to take 1 input and then exit, I think it would be best if the program gave an error message and then exited.