Replacing MIN and MAX macros with type-safe, recursive templates of variable arity I'm currently reading Mastering Perl for Bioinformatics; I'm using the book for learning bioinformatics while implementing the code in C++. I came across a situation where I needed to use the minimum of three numbers, which the author did using a function call min3 which, as expected, returns the minimum of three numbers.

Rather than using the canonical C-style macro #define MIN(a,b) ..., I wanted to try and write more idiomatic C++, first by staying away from macros, and second by using templates to write type-safe, variadic Min and Max functions.

namespace Math
{
    template <typename T1, typename T2>
    constexpr inline auto Max(T1 a, T2 b) noexcept
    {
        return (a > b) ? a : b;
    }

    template <typename T1, typename T2, typename... Types>
    constexpr inline auto Max(T1 a, T2 b, Types... args) noexcept
    {
        return Max(a, Max(b, args...));
    }

    template <typename T1, typename T2>
    constexpr inline auto Min(T1 a, T2 b) noexcept
    {
        return (a < b) ? a : b;
    }

    template <typename T1, typename T2, typename... Types>
    constexpr inline auto Min(T1 a, T2 b, Types... args) noexcept
    {
        return Min(a, Min(b, args...));
    }
} // namespace Math

Example usage:

std::cout << Math::Max(1,2)       << std::cout.widen('\n');
std::cout << Math::Min(3,8,4,3,2) << std::cout.widen('\n');

Output:

2
2

I'm using std::cout.widen('\n') so as to not call std::fflush() on each call to std::endl.

Your implementation will work nicely for integers, however, it might be doing a lot of copies which could hurt you for more expensive types.

An edge case that might sometimes be useful in case of calling this via a template: the min/max of 1 number.

Your noexcept is wrong in case of throwing copy constructors. You could change this to noexcept(std::is_nothrow_copy_constructable<T>) or fix the remark above and prevent copies.

Looking at the template arguments, you do allow T1 and T2 to be of a different type. I don't see much added value in that, as you would get an obscure error about the ?: operator.

And to end with a positive note: I really like the constexpr. This allows you to write a unit test as a static_assert.


Simple CFG parser in C++ I have written a CFG (text file) parser for my specific needs. I have found other libraries to be much too large. Even other parsers that bill themselves as "simple" run hundreds of line long.

The code that I need reviewed is the parsing and tokenizing of the input. I feel like it can be simplified. I have added more comments than required just to make it easier for you guys to read.

Here are member variables used in the class:

std::string m_cfgFilename;
std::map<std::string, std::string> m_cfgMap;
std::stringstream m_buffer;

The initialization:

bool CFG::Init(std::string filename)
{
    // Save the filename for the reload
    m_cfgFilename = filename;

    if(filename.empty())
    {
        return false;
    }

    std::ifstream t(filename);

    if (!t.is_open())
        return false;

    m_buffer << t.rdbuf();

    Parse();

    return true;
}

The parsing:

void CFG::Parse()
{
    std::string line;
    std::istringstream s(m_buffer.str());
    std::string commentKey("#");

    while (std::getline(s, line))
    {
        // Ignore empty and comment lines
        if (!line.empty() && line.compare(0, commentKey.length(), commentKey))
        {
            // Split the line
            std::vector<std::string> seglist;
            std::string segment;
            std::istringstream ss(line);

            while (std::getline(ss, segment, '='))
            {
                // Trim the white space
                std::stringstream trimmer;
                trimmer << segment;
                segment.clear();
                trimmer >> segment;

                std::transform(segment.begin(), segment.end(), segment.begin(), ::tolower);

                seglist.push_back(segment);
            }

            // Only
            if (seglist.size() == 2)
            {
                m_cfgMap[seglist[0]] = seglist[1];
            }
        }
    }
}

I am curious if there is a way to cut down on all the different strings and stream conversions.

Here is a sample cfg I would throw at it:

# Test CFG File

# General
name                =   alfred
numPopsicles        =   12
height              =   14.5
loadCharacters      =   true

I should also mention that it does work perfectly. Just worried about redundant code.

Use std::move where appropriate

When you take an object parameter by value with the intent of copying it, use std::move to take advantage of move semantics if the object supports it. This is the case with std::string. But be aware that after moving a string, the original will be left empty, so it is better to restructure CFG::Init to something like the following:

bool CFG::Init(std::string filename)
{
    if (filename.empty())
    {
        return false;
    }

    std::ifstream fileIn(filename);
    if (!fileIn.is_open())
    {
        return false;
    }

    m_buffer << fileIn.rdbuf();

    m_cfgFilename = std::move(filename);
    // 'filename' is now empty. It's contents were take by 'm_cfgFilename'

    Parse();
    return true;
}

It's probably also better to only assign m_cfgFilename at the end, so you don't set that member if you fail to open the file, but that might not be the behaviour you want. Otherwise, you can still move filename to the member string at the top and then use the member string instead.

Also notice that I've renamed the ifstream t in my example. Single letter names are pretty bad, with the exception of a few specific places like in for-loop counters.

CFG::Parse()

This line here is overly complicated:

if (!line.empty() && line.compare(0, commentKey.length(), commentKey))
{
    ...
}

std::string has the == operator overloaded, so you can compare two strings like you would compare a number. But since a comment is any line starting with '#', you might as well just test line[0].

Going further, it's better to avoid deep nesting when you can. Just continue if the condition is true.

if (line.empty() || line[0] == '#')
{
    continue;
}

Better string trim

You can cut down the size of your main reading loop by introducing a small helper function to trim the string. Having such simple utility function at hand can also be useful in other places. You can find a couple nice samples in here. This one in particular might be faster, since it doesn't create the temporary stringstream.

While you are at it, go ahead and turn that to-lowercase transform into a separate function as well:

inline std::string lowercase(std::string str)
{
    std::transform(str.begin(), str.end(), str.begin(), ::tolower);
    return str;
}

A tiny optimization

This is arguably a micro-optimization, but at scale it might have impact. If you move the declaration of your vector seglist and the temporary segment string to the top level of the function, that will avoid recreating the objects at each iteration of the loop. You just have to clear() them at the start of the loop. vector and string might be allocating and freeing dynamic memory on construction/destruction, so if they are only constructed once and just cleared, the memory stays allocated until the end of the function.

The downside of doing this is that you go against the recommendation of declaring variables as close as possible to the first point of usage, so it might not be worth it. Since this is a loop with potential hot code in it, might be interesting.


Simple string compression in Python This is my solution to exercise 1.6 in Cracking the Coding Interview.  I am interested in receiving feedback on the coding style and time/space complexity.

The exercise statement is:

  Implement a method to perform basic string compression using the counts of repeated characters.  For example, the string aabcccccaaa would become a2b1c5a3.  If the compressed string would not become smaller than the original string your method should return the original string.  You can assume the string has only uppercase and lowercase letters (a-z).

import unittest
from collections import defaultdict
from math import log10, floor

def compress_string(data: str) -> str:
    """A function to perform basic string compression using the counts of repeated characters
     If the compressed string is not smaller than the original string the function returns
     the original string.  The assumption is that the string has only uppercase and lowercase letters (a-z)."""
     curr_char_pos = 0
     frequencies = defaultdict(int)
     compressed_string = []
     compressed_string_size = 0

     for idx in range(len(data)):
        if compressed_string_size >= len(data):
            break
        if data[idx] == data[curr_char_pos]:
            frequencies[curr_char_pos] += 1
        else:
            compressed_string.append(data[curr_char_pos])
            compressed_string.append(frequencies[curr_char_pos])
            compressed_string_size += floor(log10(frequencies[curr_char_pos])) + 2
            curr_char_pos = idx
            frequencies[curr_char_pos] = 1

        compressed_string.append(data[curr_char_pos])
        compressed_string.append(frequencies[curr_char_pos])
        compressed_string_size += floor(log10(frequencies[curr_char_pos])) + 2

        if compressed_string_size < len(data):
            compressed_data = ''.join(str(char) for char in compressed_string)
            return compressed_data
       else:
            return data

class MyTest(unittest.TestCase):
    def test_compress_string(self):
        self.assertEqual(compress_string('aabcccccaaa'), 'a2b1c5a3')
        self.assertEqual(compress_string('abcd'), 'abcd')
        self.assertEqual(compress_string('tTTttaAbBBBccDd'), 'tTTttaAbBBBccDd')

This is called Run-length-encoding and there are many implementations of this on the Internet, for example: http://rosettacode.org/wiki/Run-length_encoding#Python

I have written a solution myself just now but posting it would not really help (it would be like adding another drop in the ocean) so I will comment your code thoroughly instead:

optimization?

This was probably added as an optimization:

    if compressed_string_size >= len(data):
        break

But except for inputs with no consecutive runs whatsoever, the compressed size will exceed the real size only near the end of the compression. Does the overhead of the additional conditional check each loop iteration take less time than the iterations saved? If it does, does it save enough time to justify adding complexity? I suggest you first worry about writing a working program and then apply optimizations with profiling (execution timing).

Additional variable not needed

compressed_string_size = 0

This was probably added as an optimization too, but is it worth it? No, because len runs in O(1) time complexity that is, len does not need to scan the whole list, because lists remember their length, so calling len is almost instantaneous, in fact using that variable is likely a pessimization as log is a very expensive mathematical operation.


Is this a good way to store error messages to display back to the user? I use the following code to put all of my error messages for the application in one javascript variable. Is this efficient way to store my error messages?

var cosmoBase = {
    messages: {
        empty: "Please enter ",
        numeric: "Please enter a valid number for ",
        date: "Please enter a valid date for "
    }
};
//Error messages
var cosmo = {
    messages: {
        lastName: {
            empty: cosmoBase.messages.empty + "Last Name"
        },
        dob: {
            empty: cosmoBase.messages.empty + "Date of Birth",
            invalid: cosmoBase.messages.date + "Date of Birth"
        },
        ssn: {
            empty: cosmoBase.messages.empty + "SSN",
            invalid: cosmoBase.messages.numeric + "SSN"
        },
        studentId :{
            empty: cosmoBase.messages.empty + "Student ID",
            invalid: cosmoBase.messages.numeric + "Student ID"
        }
    }
};

MORE INFO

The code above is stored in a js file

I am referencing the error messages like this :

var validator = $("form:first").validate({
    errorClass: 'Input_State_Error',
    rules: {
        lastName: {
            required: true
        },
        dob: {
            required: true,
            date: true,
            minlength: 10
        },
        ssn: {
            required: true,
            minlength: 4
        },
        studentId: {
            required: true
        }
    },
    messages: {
        lastName: cosmo.messages.lastName.empty,
        dob: {
            required: cosmo.messages.dob.empty,
            date: cosmo.messages.dob.invalid
        },
        ssn: cosmo.messages.ssn.empty,
        studentId: cosmo.messages.studentId.empty
    },
    errorPlacement: function(error, element) {
        error.appendTo($('#msg'));
    }
});

I would recommend you use a factory instead.

cosmo.error = function(id) {
    ...
};

then just use cosmo.error("ssn-empty")

Having your logic in a factory rather then an object gives you more control and makes your code significantly more DRY.

Implementation of error:

var error = (function() {
    var base = {
        empty: "Please enter ",
        numeric: "Please enter a valid number for ",
        date: "Please enter a valid date for ",
        ssn: "SSN",
        lastName: "Last Name",
        dob: "Date of Birth",
        studentId: "Student ID"
    };

    var methods = {
        "ssn": function(id) {
            if (id === "invalid") {
                return base.numeric + base.ssn;
            }
        },
        "dob": function(id) {
            if (id === "invalid") {
                return base.date + base.dob;
            }
        },
        "studentId": function(id) {
            if (id === "invalid") {
                return base.numeric + base.studentId;
            }
        },
    };

    return function(id) {
        var parts = id.split("-");
        if (parts[1] === "empty") {
            return base.empty + base[parts[0]];
        } else {
            return methods[parts[0]](parts[1]);
        }
    }
}());

console.log(error("ssn-invalid"));

live example


SQL Join with PropertyID and Name I am still fairly green when it comes to SQL joins. The below code works but I want to check that I am doing it the best way before I copy and paste errors into other work in the future.

The idea of the below code is to get the PropertyID and Name of a property from the first Table and using the PropertyID join them. In this case I am actually using every field in Table B (thus the *).

<asp:SqlDataSource ID="SqlDataSource1" runat="server" ConnectionString="<%$ ConnectionStrings:SQL %>"
    SelectCommand="
        select TblA.PropertyID as PId, TblA.Propertyname as PNa, TblB.*
        from TbPropertyDetails as TblA
            inner join TbPropertyDetailsSafeguarding as TblB on TblA.PropertyID = TblB.PropertyID
       where TblA.RegionID <> '0' and TblA.PropertyID LIKE '%' + @PropertyID + '%'
       order by TblA.PropertyID asc;"
>

<SelectParameters>
    <asp:SessionParameter DefaultValue="" Name="PropertyID" SessionField="PropertyID"
        Type="string" />
</SelectParameters>
</asp:SqlDataSource>

I am programming in VB.net but this is really standalone SQL.

In this small of a query I wouldn't alias anything.  It doesn't make it any more readable and you don't use those column aliases anywhere else in the query.

Don't do the reporting stuff on the SQL Server.  you can set the title of the column in the datagrid.  Don't do it here, it creates clutter if you try to do it in the SQL.

Whatever you are using to display this data is what you should use to give it a display title.

Always separate retrieving data from displaying data  it makes it easier to code.

I assume that you have all of your RegionID's Positive so you could change the query from

WHERE RegionID <> '0'

to

WHERE RegionID > 0

I hope that RegionID is an Integer in which case the ' are not needed because they make it a string.

Something else that I just noticed, must have been an addition when you edited your post.

WHERE TblA.PropertyID LIKE '%' + @PropertyID + '%'

I always think of an ID field being an integer, and if that is the case then you shouldn't (and CANNOT) use Wildcards or the LIKE operator on that column.  I really don't know how it is running like that.

the LIKE Operator uses a lot of performance as well, so let's say it works,  why would you want to do this in the first place, if you put in
1 for @PropertyID it will bring back records with a PropertyID of

1,11,111,1111,12,13,14,15,16,17,18,19,21,31,41,51,61,71,81,91,(100 <= x < 200)...

if you put in 13 you will get

13,113, 133,1413,1313,1334,1113,2132,1132, ...

I don't see this as being what you want to functionally happen in this query

You don't need this in the Query

ORDER BY TblA.PropertyID ASC

This should be done in the VB code as well.

This is more of a common coding practice type of thing (nitpick) all of your reserved words should be in all Caps

WHERE, JOIN, INNER, SELECT, LIKE, FROM, ORDER BY, AND, AS, ON


Word break problem with dynamic programming This is the original question.

I have implemented it in both ways. If I enter the word "icecream", should I output "ice" "cream" and also "icecream", or just "ice" and "cream"?

Is this a good example of dynamic programming?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace ConsoleApplication2
{

    public class DynamicProgramming
    {
        public DynamicProgramming()
        {
            List<string> dictionary = new List<string>{"mobile","samsung","sam","sung","man","mango",
                           "icecream","and","go","i","like","ice","cream"};
            var result = ContainsWordRecursive("ilike",dictionary);
            var result2 = ContainsWordDP("icecream", dictionary);
        }

        private List<string> ContainsWordRecursive(string word, List<string> dictionary)
        {
            List<string> res = new List<string>();
            for (int i = 0; i <= word.Length; i++)
            {
                if(dictionary.Contains(word.Substring(0,i)))
                {
                    res.Add(word.Substring(0,i));
                    var list = ContainsWordRecursive(word.Substring(i, word.Length - i ),dictionary);
                    if(list.Count > 0)
                    {
                        res.AddRange(list);
                    }
                }
            }
            return res;
        }

        private List<string> ContainsWordDP(string word, List<string> dictionary)
        {
            List<string> list = new List<string>();
            bool[] wb= new bool[word.Length+1];
            for (int i = 0; i <= word.Length; i++)
            {
                list.Clear();
                if (wb[i] == false && dictionary.Contains(word.Substring(0, i)))
                {
                    wb[i] = true;
                    list.Add(word.Substring(0, i));
                }

                if (wb[i] == true)
                {
                    if (i == word.Length)
                    {
                        return list;
                    }
                    for (int j = i + 1; j <= word.Length; j++)
                    {
                        if (wb[j] == false && dictionary.Contains(word.Substring(i, j - i)))
                        {
                            wb[j] = true;
                            list.Add(word.Substring(i, j - i));
                        }
                        // If we reached the last character
                        if (j == word.Length && wb[j] == true)
                        {
                            return list ;
                        }
                    }
                }
            }
            return list;
        }
    }
}

The first thing an algorithm needs to be is correct, and yours isn't, at least the recursive version (I didn't look at the second one).

For example, ContainsWordRecursive("ihate", dictionary) returns { "i" }, which I don't think is the right result. Instead, your method should somehow indicate that it failed, for example by returning null.


Finding circular primes I'm coding for a project euler question and every now and then, the question will demand a program that is efficient even when doing brute force. Which I struggle with.

Below is a piece of code for problem 35 which I'm fairly certain works correctly so far with numbers under 10000 however when I set it to 1 million it takes way too long to run. I still havent got an answer yet and it's been running for about 15 mins....

If anyone could give me general tips on efficiency that would be awesome!

def rwh_primes(n):
    sieve = [True] * n
    for i in range(3,int(n**0.5)+1,2):
        if sieve[i]:
            sieve[i*i::2*i]=[False]*int((n-i*i-1)/(2*i)+1)
    return [2] + [i for i in range(3,n,2) if sieve[i]]

def is_prime(n):
    for i in range(3, n):
        if n % i == 0:
            return False
    return True

def f7(seq):
    seen = set()
    seen_add = seen.add
    return [x for x in seq if not (x in seen or seen_add(x))]

primes = rwh_primes(1000000)
lisp = []
working = []
count = 0
counter = 0

for x in primes:
    z = 1
    y = (len(str(x)) + 1)

    if len(str(x)) == 2:
        thingy = list(str(x))
        number = int(thingy[1] + thingy[0])
        if is_prime(number) == True:
            lisp.append(x)

    elif len(str(x)) < 2:
        lisp.append(x)

    else:
        while count < len(sorted(str(x))) - 1:
            num = list((str(x) + str(x)))
            new = num[z:y]
            newest = ''.join(new)
            verynew = int(newest)
            working.append(verynew)
            count += 1
            z += 1
            y += 1

            if count == len(sorted(str(x))) - 1:
                for a in working:
                    if is_prime(a) == True:
                        counter += 1
                        if counter == len(working):
                            lisp.append(x)
                            lisp = f7(lisp)
        count = 0
        counter = 0
        working = []

print(len(lisp))

Some observations:

I don't really understand the majority of names. What does f7 mean? What does the rwh in rwh_primes mean? Why is the lisp variable called lisp? What is the difference between count and counter? Having good names is a huge part of good coding practice.
Your rwh_primes computes a list of primes, which is good, but you then ignore list of primes when you test for primality in the is_prime method (which, since it goes up to \$N\$ instead of \$\sqrt{N}\$, is particularly slow and probably the cause of most of your troubles). Instead of running is_prime, why not simply check whether number in primes (note that for this, you probably want to make primes a set rather than a list).
I'm not sure why you do len(sorted(str(x))) instead of just len(str(x)); sorting a list doesn't change its length.
You don't need to keep track of which numbers are circular primes, just how many there are. You could replace lisp with a simple count.
Why do you deal with the length 2 case differently than the case where the length is greater than 2? I understand why you might want to deal with the length 1 case separately, but your code that works for your larger cases should also work for the length 2 case.
Your method for getting a cyclic shift of the number is interesting (doubling the string then taking a substring), but python makes it really easy to just do int(num[i:] + num[:i]), which does most of the work of your while loop in one line. You can now also reduce your two indices y and z to a single index i.
Instead of checking whether you've appended all cyclic shifts during each repetition of your while loop, why not just finish your while loop and then loop through the cyclic shifts (i.e. move the if statement outside the while loop). You do this again in your inner loop (for a in working).
It looks like f7 uniqifies a list. While I don't think you actually need to do any uniqifying here, even if you do want the list of all circular primes (because you add them in increasing order), a much easier way to do this in python is simply list(set(x)); this is fast enough for most purposes.


improving my simple python calculator i am rather new at python and this is my very first project i did myself. i am wondering if there is anyway i can improve this or give me advice on making this better. thank you
import sys
valid = ["+", "-", "*", "/"]
invalid = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z']
def start():
    while True:
        a = input("").lower()
        for banned in a:
            if banned in invalid:
                print("error")
            else:
                sum1 = int(a)
        cal = input("")
        b = input("").lower()
        for banned in b:
            if banned in invalid:
                print("Error")
                start()
            else:
                sum2 = int(b)
        break;
    for list in cal:
        if list in valid:
            if list == "+":
                total = sum1 + sum2
                print("=" + total)
            if list == "-":
                total = sum1 - sum2
                print(total)
                start()
            if list == "*":
                total = sum1 * sum2
                print(total)
                start()
            if list == "/":
                total = sum1 / sum2
                print(total)
                start()
        else:
            print("Wrong Format")
            start()
start()

A few comments focused less on line-by-line details and more on how to build
better command-line scripts and on simplifying logic.
How do I use this program? Your input() calls
don't tell me what's going on. Help, please.
Users don't enjoy tedious interactions with computers. Would you rather
type a number, press enter, type an operator, press enter, type a number, press
enter ... OR just enter the darn equation? For example:
reply = input('Enter equation: INT OP INT: ')

Users really don't enjoy tedious interactions with computers. With rare
exceptions, sensible command-line programs don't use input() at all -- or at
least not by default. Learn about sys.argv and let your users run your
calculator directly, without forcing them to become best buddies with their
computer. Here's what a command-line usage might look like:
$ python simple-calculator.py 32 + 10
42

Learn about exception handling. Rather than getting bogged down in creating
a data structure holding every possible bad input (very hard, especially if you
consider all conceivable UTF-8 inputs), just try to do the thing you want to do
-- with a safety net.
while True:
    try:
        # Each of these lines could fail. That's OK. Be optimistic!
        s1, op, s2 = reply.split()
        a = int(s1)
        b = int(s2)
        assert op in '+-*/'
        break                  # Success.
    except Exception as e:
        # The safety net.
        print('Invalid input')

Use data structures to simplify logic. Wouldn't it be cool to get rid of a
pile of if-else checks and instead just do the thing you want to do directly?
In such situations, a data structure can often help. What we need is a way to
take an operator and then use it to get a 2-argument function that will perform
the math we need. In other words, we need a mapping between operators and the
corresponding math functions. Here are 3 ways to achieve that:
import operator

def add(a, b):
    return a + b

ops = {
    '+': add,                    # Define your own functions.
    '-': lambda a, b: a - b,     # Or lambdas.
    '*': operator.mul,           # Or get them from a library.
    '/': operator.truediv,
}

# With that data structure, our code would reduce to this.
answer = ops[op](a, b)     # Wrap in try-except to catch divide by 0.
print(answer)


Validate a currency string Please see the code below:

public bool IsValidateInputSuceeded(string EnteredCost, string EnteredPaymentReceived, out decimal Cost, out decimal PaymentReceived)
{
    bool ValidateInputSuceeded = true;

    EnteredCost= EnteredCost.Substring(EnteredCost.IndexOfAny("0123456789".ToCharArray()));
    EnteredPaymentReceived = EnteredPaymentReceived.Substring(EnteredPaymentReceived.IndexOfAny("0123456789".ToCharArray()));

    if ((Decimal.TryParse(EnteredCost.Replace(" ", "").Replace(",", ""), out Cost)) == false)
    {
        ValidateInputSuceeded = false;
    }

    if ((Decimal.TryParse(EnteredPaymentReceived.Replace(" ", "").Replace(",", ""), out PaymentReceived)) == false)
    {
        ValidateInputSuceeded = false;
    }

    if (ValidateInputSuceeded == false)
    {
        return false;
    }

    if (EnteredCost.IndexOf(" ") > 0|| EnteredPaymentReceived.IndexOf(" ") > 0)
    {
        return false;
    }

    if (EnteredCost[EnteredCost.Length-1] =='.' || EnteredCost[EnteredCost.Length - 2] == '.' || EnteredPaymentReceived[EnteredPaymentReceived.Length - 1]=='.' || EnteredPaymentReceived[EnteredPaymentReceived.Length - 2] == '.')
    {
        return false;
    }

    if (EnteredCost.IndexOf(".") == -1 || EnteredPaymentReceived.IndexOf(".")==-1)
    {
        return false;
    }

    return true;
}

EnteredCost and EnteredPaymentReceived are input parameters from MaskedTextBoxs'(https://msdn.microsoft.com/en-us/library/system.windows.forms.maskedtextbox(v=vs.110).aspx) both with an input mask of: £999,999.00.  For example, a value of one of them could be: "£   ,  3.22" (without quotes) as an input parameter into the function.

There are two string variables: EnteredCost and PaymentReceived. Should these variables be StringBuilders instead of Strings? The reason I ask is because there are a log of string manipulations going on e.g. using the REPLACE function and the indexOfAny function, which returns more strings.

Since

  EnteredCost and EnteredPaymentReceived are input parameters from MaskedTextBox

and a MaskedTextBox

  Uses a mask to distinguish between proper and improper user input.

then the validation is not necessary because all it does is to check whether the strings contain numeric data and a dot . as a separator. The MaskedTextBoxs alredy do that so why are you validating their input again?


Python Code for List of Month Names starting with current month I am looking for a cleaner, more pythonic way to get a list of the name of the months starting with the current month and ending 12 months later.

For example, it's December so my list should be

['Dec', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov']

Here is how I'm currently doing it:

from datetime import datetime

currentMonth = datetime.now().month

if (currentMonth == 1):
    theList = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec']
elif (currentMonth == 2):
    theList = ['Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec', 'Jan']
elif (currentMonth == 3):
    theList = ['Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec', 'Jan', 'Feb']
elif (currentMonth == 4):
    theList = ['Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec', 'Jan', 'Feb', 'Mar']
elif (currentMonth == 5):
    theList = ['May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec', 'Jan', 'Feb', 'Mar', 'Apr']
elif (currentMonth == 6):
    theList = ['Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec', 'Jan', 'Feb', 'Mar', 'Apr', 'May']
elif (currentMonth == 7):
    theList = ['Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun']
elif (currentMonth == 8):
    theList = ['Aug', 'Sep', 'Oct', 'Nov', 'Dec', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul']
elif (currentMonth == 9):
    theList = ['Sep', 'Oct', 'Nov', 'Dec', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug']
elif (currentMonth == 10):
    theList = ['Oct', 'Nov', 'Dec', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep']
elif (currentMonth == 11):
    theList = ['Nov', 'Dec', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct']
elif (currentMonth == 12):
    theList = ['Dec', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov']

print(theList)

Ideas?

Instead of hard-coding month names - delegate all the work to respective libraries.

I would suggest 2 approaches:

calendar.month_abbr

from datetime import datetime
from calendar import month_abbr

def get_forward_month_list():
    month = datetime.now().month   # current month number
    return [month_abbr[(month % 12 + i) or month] for i in range(12)]

dateutil.relativedelta.relativedelta

from datetime import datetime, timedelta
from dateutil.relativedelta import relativedelta

def get_forward_month_list():
    now = datetime.now()
    return [(now + relativedelta(months=i)).strftime('%b') for i in range(12)]

Both approaches will return the expected/needed list of month names:

print(get_forward_month_list())
['Dec', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov']


Creating a wrapper class to use for mocking that uses IDisposable I have a third party component that I am trying to write unit tests around. The problem is that I can't mock the object and there is no interface.

I decided to create an interface and a wrapper class calling into the code for the sake of mocking. I looked into the class definition that is generated by Visual Studio using the meta data and noticed a few things:

The class has two constructors (one takes a parameter)
The class inherits from IDisposable

My questions are:

Does my implementation below look right?
Did I handle the IDisposable implementation correctly in the proxy class?
Do I need the second constructor in the proxy class since the interface does not support constructor definitions? I use dependency injection in my code and I assume unless I tell my DI framework to use the second constructor I don't really need it but I am not sure.

The meta data looks like (slimmed down version):

public class PopClient : IDisposable
{
    public const int DefaultPort = 110;
    public const int DefaultSSLPort = 995;

    public PopClient();
    public PopClient(AddressFamily addressFamily);
    public bool HasTimeStamp { get; }
    public List<string> Capability();
    public void Connect(string host);
    protected override void GetServerGreeting();
}

Based on the meta data, my interface looks like (after removing methods/properties/access modifiers that are invalid in an interface definition):

public interface IPopClient : IDisposable
{
    bool HasTimeStamp { get; }
    List<string> Capability();
    void Connect(string host);
    void ConnectSSL(string host);
}

Based on the interface, I then created the wrapper class:

public class PopClientProxy : IPopClient
{
    private readonly Pop3 pop3;

    public PopClientProxy()
        this.pop3 = new Pop3();

    public PopClientProxy(AddressFamily addressFamily)
        this.pop3 = new Pop3(addressFamily);

    public bool HasTimeStamp
        get { return pop3.HasTimeStamp; }

    public List<string> Capability()
        return pop3.Capability();

    public void Connect(string host)
        pop3.Connect(host);

    public void Dispose()
    {
        if (pop3 != null)
            pop3.Dispose();
    }
}

Congratulations! You have successfully utilized the object-oriented design pattern known as the Adapter Pattern. I think that looks perfectly good. Though, I wouldn't check pop3 for null in the Dispose() method as it will be guaranteed to not be null by the constructors and the readonly field modifier:

public void Dispose()
{
    pop3.Dispose();
}


Get all AD groups and Exchange features of users We have about 15000 users and 1000 groups in AD. I have been getting of all AD groups of all AD users and their Exchange features  via script like below. But now I have been waiting for more than 3 hours already. My question is: How can I improve the speed of the script?

$groups = Get-ADGroup -filter * -SearchBase "OU=Security Groups,DC=contoso,DC=com"
$groupsCSV = "Username;"
$groups | foreach {
$groupsCSV += $_.sAMAccountName
$groupsCSV += ";"
}
$groupsCSV += "ActiveSyncEnabled; OWAEnabled; POPEnabled; IMAPEnabled; MAPIEnabled; MAPIBlockOutlookRpcHttp"
$groupsCSV += "`n"
$users = Get-ADUser -filter {Name -like "*"} -SearchBase "OU=test,OU=test,DC=contoso,DC=com"
$users | foreach {
$currentUser = $_
ECHO Examine: $_.Name
$isMemberList=""
$groups | foreach {
$currentGroup = $_
$currentMembers = Get-ADGroupMember $_
$isMember = ""
$currentMembers | foreach  {
if ($_.sAMAccountName -like $currentUser.sAMAccountName )  {
$isMember = "x"
}
}
$isMemberList += $isMember
$isMemberList += ";"
}
$isMemberListCSV += $currentUser.Name
$isMemberListCSV += ";"
$isMemberListCSV += $isMemberList
$MBFeatures = Get-CasMailbox $currentUser.DistinguishedName
if( $MBFeatures.ActiveSyncEnabled){
$isMemberListCSV += "x"
}
$isMemberListCSV += ";"

if( $MBFeatures.OWAEnabled){
$isMemberListCSV += "x"
}
$isMemberListCSV += ";"

if( $MBFeatures.POPEnabled){
$isMemberListCSV += "x"
}
$isMemberListCSV += ";"

if( $MBFeatures.IMAPEnabled){
$isMemberListCSV += "x"
}
$isMemberListCSV += ";"

if( $MBFeatures.MAPIEnabled){
$isMemberListCSV += "x"
}
$isMemberListCSV += ";"

if( $MBFeatures.MAPIBlockOutlookRpcHttp){
$isMemberListCSV += "x"
}
$isMemberListCSV += ";"
$isMemberListCSV += "`n"
}
$fullCSV = $groupsCSV + $isMemberListCSV
ECHO $fullCSV > fullCSV.txt

The lack of proper indentation hides the fact that for each user you request all the members of every group. That's 15000*1000 = 15 million remote calls, most of which redundant. Even if 100 calls would get answered per second, it would still take more than a day to receive all answers.

The solution is simple: Only call Get-ADGroupMember once for each group, cache the results and then use them to check what groups each user is a member of.

If Get-CASMailbox can return the mailbox information for all users at once instead of one call/user (haven't used it but the documentation indicates it might) you reduce the number of calls even further.


Find CISC prerequisites Below I have some code written in prolog as a brief example. The goal of the program is to take a Computer and Information Science (CISC) course number and return any CISC prerequisites (prereqs).

I would like to know if this code followed common practices in prolog.

%CISC Courses
cisc(100).
cisc(101).
cisc(103).
cisc(106).
cisc(120).
cisc(160).
cisc(201).
cisc(211).
cisc(225).
cisc(233).
cisc(298).
cisc(300).
cisc(301).
cisc(311).
cisc(320).
cisc(330).
cisc(333).
cisc(340).
cisc(349).
cisc(365).
cisc(370).
cisc(380).
cisc(390).
cisc(397).
cisc(399).
cisc(400).
cisc(431).
cisc(432).
cisc(433).
cisc(440).
cisc(444).
cisc(460).
cisc(491).
cisc(498).
cisc(499).

Define Prereqs
prereqs(160,120).
prereqs(211,120).
prereqs(225,120).
prereqs(300,120).
prereqs(233,160).

prereqs(301,211).
prereqs(301,233).
prereqs(311,160).
prereqs(311,225).
prereqs(320,211).

prereqs(320,233).
prereqs(330,211).
prereqs(333,233).
prereqs(340,233).
prereqs(349,233).

prereqs(370,333).
prereqs(390,120).
prereqs(397,225).
prereqs(397,301).
prereqs(399,233).

prereqs(400,233).
prereqs(400,301).
prereqs(431,233).
prereqs(431,399).
prereqs(432,233).

prereqs(433,301).
prereqs(433,399).
prereqs(440,370).
prereqs(444,433).
prereqs(460,225).

prereqs(460,233).
prereqs(491,225).
prereqs(491,301).
prereqs(498,298).

%Return prereqs to a given class with prereqs of prereqs
prereq(X,Y) :- prereqs(X,Y),format("CISC ~w is a prereq of CISC ~w~n",[Y,X]).

prereq(X,Y) :- prereqs(X,Z),
    prereq(Z, Y).

In my opinion, your code would benefit considerably from a better naming convention.

In Prolog, a good predicate name makes clear what each argument denotes.

For example, when I see:

prereqs(460, 233).

then I have no idea what the arguments are. I see it is about prerequisites, but in which direction? Is 460 a prerequisite of 233, or is it the other way around? Also, why is this called "prereqs" although each clause only denotes a single prerequisite?

A much more descriptive name for this predicate would be (for example!):

prerequisite_of_course(460, 230).

because this makes clear what is the prerequisite of what. Another very good, descriptive name would be for example:

course_prerequisite(230, 460).

You will find that, if you use more descriptive names, then emitting output yourself will become unnecessary: The predicate name alone will make the situation perfectly clear. Let the toplevel do the printing for you!

The variable names in:

prereq(X, Y) :-
    prereqs(X, Z),
    prereq(Z, Y).

could also be improved considerably by choosing more descriptive names or abbreviations, such as Req, Course etc. For transitive relations between courses, you can for example use:

Course0 → Course1 → ... → Course


Find the integer representing the last sub-string of a string I came across a question at the Udacity Intro to Computer Science course:

Define a procedure, find_last, that takes as input two strings, a search string and a target string, and returns the last position in the search string where the target string appears, or -1 if there are no occurrences.
Example: find_last('aaaa', 'a') returns 3

I think there should be a much cleaner (shorter, fewer words) way of writing this function:
def find_last(s, t):
    count = 0
    if s.find(t) >= 0:
        while s.find(t, count + 1) >= 0:
            count += 1
        return count
    else:
        return s.find(t)

Well the short answer would be str.rfind().

>>> find_last = lambda s, t: s.rfind(t)
>>> find_last("aaaa", "a")
3

Though I'm not sure how much you're required to implement yourself. (Does the task allow usage of .find() at all?) Maybe it could be written like this:

def find_last(s, t):
    start = 0
    while True:
        if s.find(t, start) < 0:
            return start - 1
        start += 1

def find_last(s, t):
    for i in range(len(s)-len(t), -1, -1):
        if s[i:i+len(t)] == t:
            return i
    return -1


Creating a specific distribution of random numbers in Powershell I originally posted this on StackOverflow but was requested to post it here instead as it relates to optimization/performance of the code rather than a specific problem.
TL;DR: Get-Random produces an even distribution of numbers where every number in the pool has an even chance of appearing. I'm looking for a way to generate random numbers where every individual number appears with a frequency that I myself specify.

If I run
for($i=1;$i -le 1000;$i++){
    Get-Random -Minimum 0 -Maximum 10
}

I get a very evenly distributed count for each number (roughly 100 for each number between 0 and 9). Using Get-Random allows me to get a random number every time but on average, every individual result will appear roughly an equal amount of times. I want to decide the frequency for how often any specific number appears, and then generate random numbers that fit that distribution. As an example:
Number   Probability
0        10
1        11
2        19
3        12
4        3
5        10
6        6
7        7
8        4
9        18

I'm looking for a way to use the above list to randomly generate a number between 0 to 9, but with the probability of each individual number appearing using the Probability column.
My very hard-coded and not so generic solution so far is that I thought of doing something like adding a cumulative percentage column:
Number   Probability   CumulativeProbability
0        10            10
1        11            21
2        19            40
3        12            52
4        3             55
5        10            65
6        6             71
7        7             78
8        4             82
9        18            100

And from here, run the object through a filter. Something like:
$RandomNumber = Get-Random -Minimum 0 -Maximum 100
$MyProbabilityObject | Where-Object {$RandomNumber -ge $_.CumulativeProbability}

This gives me all numbers with a lower Cumulative probability than the random number. Let's say the $RandomNumber was 42, that would result in:
Number   Probability   CumulativeProbability
0        10            10
1        11            21
2        19            40

From here, I could pipe the result to
Select-Object CumulativeProbability | Measure-Object -Property CumulativeProbability -Maximum

Which gives me the highest value, and then use that column as a reference to find the number with
Where-Object {$_.CumulativeProbability -eq $TheNumberIGetAbove}

While this kinda works, it feels like I'm doing several laps around a problem that should be easier and more straightforward to solve. Are there any better ways of generating random numbers that fit a distribution you specify yourself instead of using an even distribution such as Get-Random?

Since you tagged your question with Performance, my example is not totally idiomatic PowerShell.

You could build a lookup array with each number as many times as you have defined the probability for them.
Afterwards you can easily index into it using one or an array of random (index) numbers.
$propabilityDefinition = @{
    0 = 10
    1 = 11
    2 = 19
    3 = 12
    4 = 3
    5 = 10
    6 = 6
    7 = 7
    8 = 4
    9 = 18
}

# build a lookup array with each number as many times as defined as probability
$numberLookup = foreach ($entry in $propabilityDefinition.GetEnumerator()) {
    # using foreach with Linq.Enumerable.Repeat is the fastest way I found so far
    [System.Linq.Enumerable]::Repeat($entry.Key, $entry.Value)
}

# get x random numbers within the scope of the array indexes.
# with PS7+ we can also use the Count parameter of Get-Random, but don't use "Get-Random" within a loop as it hits the performance quite hard
#   PS7 variant: "$randomIndex = (Get-Random -Minimum 0 -Maximum $numberLookup.Count -Count 1000)"
# if you only want to get one random number, you can always use "$randomIndex = Get-Random -Minimum 0 -Maximum $numberLookup.Count"
$random = [System.Random]::new()
$randomIndex = foreach ($null in 0..10000) {
    $random.Next(0, $numberLookup.Count)
}

# we can use the entiry array of random numbers at once for the index operator
$numberLookup[$randomIndex] | Group-Object -NoElement | Sort-Object Count -Descending

Output:
Count Name
----- ----
 1896 2
 1821 9
 1194 3
 1141 1
  983 0
  979 5
  694 7
  596 6
  393 8
  304 4


Session generator/checker utility for a server I wrote a utility for generating and saving sessions in a MongoDB database as well as checking the session to make sure it's valid.

I'm a beginner when it comes to promises and I have a feeling that either I'm using them improperly or I'm not taking full advantage of them.

Here's the session utility:

// Dependencies
var crypto = require('crypto'),
    config = require('../../config'),
    Session = require('../database/schemas/session');

// Generate Function
var generate = function(length) {
  // Return a promise
  return new Promise(function(resolve, reject) {
    // Create a random value
    crypto.randomBytes(length, function(error, buffer) {
      // Determine if an error occurred
      if (error) {
        // Reject with error
        reject(error);
      } else {
        // Resolve with session ID
        resolve(buffer.toString('hex'));
      }
    });
  });
}

// Create Function
exports.create = function(account_id) {
  // Return a promise
  return new Promise(function(resolve, reject) {
    // Generate the session ID and fetch the promise
    generate(config.session.length)
      .then(function(sid) {
        // Save the session in the database and return the promise
        return(new Session({_id: sid, account_id: account_id}).save());
      })
      .then(function(session) {
        // Determine if session is not defined
        if (!session) {
          // Reject
          reject();
        } else {
          // Resolve with the session ID
          resolve(session._id);
        }
      })
      .catch(reject);
  });
}

// Check Function
exports.check = function(sid) {
  // Return a promise
  return new Promise(function(resolve, reject) {
    // Query for a session with the provided ID and fetch the promise
    Session.findOne({_id: sid}).lean().exec()
      .then(function(session) {
        // Determine if no session was found or if it expired
        if (!session) {
          // Resolve with false
          resolve(false);
        } else if (session.expires < Date.now()) {
          // Resolve with false
          resolve(false);
        } else {
          // Resolve with true by default
          resolve(true);
        }
      })
      .catch(reject);
  });
}

Here's the session schema:

// Dependencies
var mongoose = require('mongoose'),
    config = require('../../../config');

// Session Schema
var sessionSchema = mongoose.Schema({
  _id: {type: String},
  account_id: {type: mongoose.Schema.Types.ObjectId, required: true},
  expires: {type: Date, required: true, default: Date.now() + config.session.expires}
});

// Export the model of the schema
module.exports = mongoose.model('session', sessionSchema);

For the most part this looks pretty good, there are only a few things which I would recommend changing.

As Igor Soloydenko recommends, get rid of the comments which do nothing but restate the code. Though comments are certainly not a bad thing, they should instead be used to explain why some code is the way it is if another programmer in 6 months (you) looks at it and expects something different.
Don't wrap promises in new Promise(). The bluebird wiki has a good explanation of this. It essentially boils down to "if a function returns a promise, return that directly". Here's an example with your code. I have removed the check for a null session as save() will reject when there is an error and it is not necessary to check for it yourself.

exports.create = function(account_id) {
  return generate(config.session.length)
    .then(function(sid) {
      return new Session({_id: sid, account_id: account_id}).save();
    })
    // If all a method does is return, I prefer arrow functions
    .then(session => session._id);
}

If you are using Node v7.6.0 or newer, you can take advantage of async / await to further simplify the use of promises.

exports.create = async function(account_id) {
  let sid = await generate(config.session.length)
  let session = await new Session({_id: sid, account_id: account_id}).save();
  return session._id;
}

If you are using Node v8.0.0 or newer, you can use the built in promisify method to avoid wrapping callback based methods yourself.

const { promisify } = require('util');
var generate = function(length) {
  // If this is being called a lot, the promisify call should be saved
  return promisify(crypto.randomBytes)(length)
    .then(buffer => buffer.toString('hex'));
}

The check function can be simplified, there are several ways this could be written.

exports.check = function(sid) {
  return Session.findOne({_id: sid}).lean().exec()
    .then(function(session) {
      // Igor Soloydenko's suggestion
      if (!session || session.expires < Date.now()) {
        return false
      }
      return true;
    });
}

// This works better if there are many conditions which must be true
// Igor Soloydenko's check is probably better here.
exports.check = function(sid) {
  return Session.findOne({_id: sid}).lean().exec()
    .then(session => [
      session,
      session.expires > Date.now(),
    ].every(Boolean));
}


Array manipulation: add a value to each of the array elements between two given indices This is a Hackerrank problem: https://www.hackerrank.com/challenges/crush/problem

You are given a list of size \$N\$, initialized with zeroes. You have
to perform \$M\$ operations on the list and output the maximum of
final values of all the \$N\$ elements in the list. For every
operation, you are given three integers \$a, b\$ and \$k\$ and you
have to add value to all the elements ranging from index \$a\$ to
\$b\$ (both inclusive).
Input Format
First line will contain two integers \$N\$ and \$M\$ separated by a
single space. Next \$M\$ lines will contain three integers \$a, b\$
and \$k\$ separated by a single space. Numbers in list are numbered
from \$1\$ to \$N\$.
Constraints
\$3 \leq N \leq 10^7\$
\$1\leq M \leq 2*10^5\$
\$1 \leq a \leq b \leq N\$
\$ 0 \leq k \leq 10^9\$
Output Format
A single line containing maximum value in the updated list.
Sample Input
5 3
1 2 100
2 5 100
3 4 100

Sample Output
200

My code:
def arrayManipulation(n, queries):
    nums = [0] * (n + 1)
    for q in queries:
        nums[q[0]-1] += q[2]
        nums[q[1]] -= q[2]
    current = 0
    max = 0
    for i in nums:
        current += i
        if current > max: max = current
    return max

Is there any way to optimize this?

Nice implementation, it's already very efficient. Few suggestions:

Expand the variables in the for-loop from for q in queries to for a, b, k in queries. Given the problem description it's easier to read.
A better name for the variable current can be running_sum.
Avoid calling a variable max, since it's a built-in function in Python. An alternative name can be result.
If you change the name of the variable max then you can have result = max(result,running_sum).
As @hjpotter92 said, is better to add a description of your approach in the question, you will likely get more reviews. Few bullet points or some comments in the code is better than nothing.

Applying the suggestions:
def arrayManipulation(n, queries):
    nums = [0] * (n + 1)
    for a, b, k in queries:
        nums[a - 1] += k
        nums[b] -= k
    running_sum = 0
    result = 0
    for i in nums:
        running_sum += i
        result = max(result, running_sum)
    return result

It's already an efficient solution that runs in \$O(n+m)\$, so I wouldn't worry about performances. However, there is an alternative solution running in \$O(m*log(m))\$ in the Editorial of HackerRank.
I implemented it in Python:
def arrayManipulation(n, queries):
    indices = []
    for a, b, k in queries:
        indices.append((a, k))
        indices.append((b + 1, -k))
    indices.sort()
    running_sum = 0
    result = 0
    for _, k in indices:
        running_sum += k
        result = max(result, running_sum)
    return result

It's based on the fact that it's enough finding the running sum on the sorted indices.
FYI in the Editorial (or Discussion) section of HackerRank there are optimal solutions and detailed explanations.
Thanks to @superbrain for the corrections provided in the comments.


Use git to get SHA-1 of repo I've created the function below to get the SHA-1 of the repository used by my CI server. The hash is then used to update the pass/fail status of the commit on GitHub.

I'm a python amateur. What stands out below?

def get_sha():
    root = os.environ["PWD"]
    repodir = os.listdir(root)[0]
    # /Library/Developer/XcodeServer/Integrations/Caches/6490b1f573dca4e4e0d988197ae6c225/Source/repo_name
    repo = os.path.join(root, repodir)
    sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
    return sha

Please advise on best practices, errors this might encounter, values I should be checking, etc.

EDIT:

Based on feedback I have revised it to the following:

def get_sha():
    for subdir in os.listdir('.'):
        if is_git_directory(subdir):
            return get_repo_sha(subdir)
    assert False

def is_git_directory(path = '.'):
    return subprocess.call(['git', '-C', path, 'status'], stderr=subprocess.STDOUT, stdout = open(os.devnull, 'w')) == 0

def get_repo_sha(repo):
    sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
    return sha

It seems to me that this would be equivalent but shorter:

def get_sha():
    repo = os.listdir('.')[0]
    sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
    return sha

Because, . is essentially the same as os.environ['PWD'],
with the difference that it's not an absolute path.
Since subprocess.check_output(..., cwd=...) works fine with relative paths too,
I don't see the reason to use an absolute path, and so no need for PWD.

But more importantly,
I find it a bit odd that this script assumes that the first entry in the directory listing is a Git repository.
This setup seems a bit fragile.
It would be better to pass the repo path as a parameter to this function:

def get_sha(repo):
    sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
    return sha

Then you'd have something simple, clear, no questions asked.


n-digit Verification Code Generator This is a verification code generator, which generates n-digit numeric strings. (code can be 0000, so I chose String as a return type rather than Int or Long.)

It'll be great if anyone review this code and suggest more elegant or scalaish solution.

import scala.util.Random

object VerificationCodeGenerator {
  val rand = new Random

  def generate(digit: Int): String = {
    val sb = new StringBuilder
    for (i <- 1 to digit) {
      sb.append(rand.nextInt(10))
    }
    sb.toString()
  }
}

if your number of digits is always going to be less than about 10, then you can use a single random operation and a string format, to do all the work without the loop.

Consider a method like:

def generate(digit: Int): String = {
    var randVal = rand.nextInt(math.pow(10, digit).toInt)
    var fmt = "%0" + digit + "d"
    fmt.format(randVal)
}

The randVal pulls a value with the limited number of digits (perhaps fewer than the limit). The format operation 0-pads the value to the right number, though.

This is not so much a scala way of doing it, but it is closer, and probably more efficient.

I would consider creating instances to handle each length of digits to avoid having to create the format each time, though.

See it running in ideone


Fitter Nearest-Neighbor function with three nested loops Background Problem

Consider a set of students that we want to place into Ng groups each of which contains Ns students.  Let's label the students 1, 2, ... , Ng * Ns, and let an assignment of students be an array of shape (Ng, Ns) that encodes the groupings of students.  For example, if Ng = 2 and Ns = 3, then an assignment of students could be:

[[1, 4, 5], [2, 3, 6]]

note that any assignment that differs either in permuting students within groups, or in permuting the groups themselves would be considered an equivalent assignment.  So for the assignment above, one equivalent assignment would be:

[[6, 3, 2], [1, 5, 4]]

For a given assignment A, we define a neighboring assignment as any inequivalent assignment that differs from A by a single swap of students.  For example, a neighbor of the first assignment above is obtained by swapping students 1 and 6 to obtain

[[1, 3, 2], [6, 5, 4]]

Additionally, we have a fitness function f that takes an assignment A as its input, and outputs a real number f(A).  The goal is to construct a function which I call fitter_neighbor which takes an assignment A and a fitness function f as its inputs and returns a fitter neighboring assignment provided such an assignment exists.  If no such assignment exists, then fitter_neighbor should return A.  It is not required that fitter_neighbor returns the fittest neighbor, and in fact, the goal is to have it systematically search through the neighbors and return the first fitter neighbor it comes across.

Current seemingly-not-very-pythonic code

def fitter_neighbor(A, f):

    Ng = len(A) #number of groups
    Ns = len(A[0]) #number of students per group
    Nn = int(Ns ** 2 * Ng * (Ng - 1) / 2) #number of neighboring assignments

    A_swap = np.copy(A) #initialize copy of assignment to be changed

    g1 = 0 #group number of person A in swap
    n_swaps = 0 #counter for number of swaps considered

    while g1 < Ng and f(A_swap) <= f(A) and n_swaps < Nn:
        s1 = 0
        while s1 < Ns and f(A_swap) <= f(A) and n_swaps < Nn:
            g2 = g1 + 1 #ensures that no swaps are repeated
            while g2 < Ng and f(A_swap) <= f(A) and n_swaps < Nn:
                s2 = 0
                while s2 < Ns and f(A_swap) <= f(A) and n_swaps < Nn:
                    A_swap = np.copy(A)
                    A_swap[g1, s1], A_swap[g2, s2] = A_swap[g2, s2], A_swap[g1, s1]
                    n_swaps += 1
                    s2 += 1
                g2 += 1
            s1 += 1
        g1 += 1
    if n_swaps < Nn:
        return A_swap
    else:
        return A

Can fitter_neighbor be  written in a more pythonic way?  In particular, is it possible to eliminate the nested while loops in some clever way?

As an example fitness function for the case Ns = 2, consider

def f2(A):
    return np.sum([(g[1] - g[0]) ** 2 for g in A])

Not a solution but a line of thought you may use for implementation. You can define the group of students as sets and then make tuples of these sets, so for example ({1, 3, 5}, {2, 4, 6}), let's call this tuple A and another tuple of sets B: (1, 4, 5}, {2, 3, 6}) I assume here that the order of the sets is important as you want to compare the first set of A with the first set of B, and so on...

You can then compare these tuples of sets by:

A = ({1, 3, 5}, {2, 4, 6}, {8, 10, 12})
B = ({1, 4, 5}, {2, 3, 6}, {8, 10, 12})
C = []
D = []

for i, a_list in enumerate(A):
   for j, b_list in enumerate(B):
       if i == j:
           C.append(a_list - b_list)
           D.append(b_list - a_list)

C = tuple(C)
D = tuple(D)
print(f'A: {A}\nB: {B}\nC: {C}\nD: {D}')

Answer:

A: ({1, 3, 5}, {2, 4, 6}, {8, 10, 12})
B: ({1, 4, 5}, {2, 3, 6}, {8, 10, 12})
C: ({3}, {4}, set())
D: ({4}, {3}, set())

C and D are then tuples with sets of differences for the first set, second set, etc. If these include sets with one element you know the sets in A and B are the same except for that one element.


Performant C# Socket server I am creating a performant C# socket server. I'd like it to be able to accept as many simultaneous connections as possible. I've tested with 100 clients and all clients are fine. With 1000 however, clients begin to stop connecting. Is it possible to improve something to increase the number of simultaneous connections?

Of course, general code review would also be appreciated.

Server:

class Listener
{
    public readonly int Port;
    private readonly TcpListener _tcpListener;
    private readonly Task _listenTask;

    public Listener(int port)
    {
        Port = port;
        _tcpListener = new TcpListener(IPAddress.Any, Port);
        _tcpListener.Start();

        _listenTask = Task.Factory.StartNew(() => ListenLoop());
    }

    private async void ListenLoop()
    {
        for (; ; )
        {
            var socket = await _tcpListener.AcceptSocketAsync();
            if (socket == null)
                break;

            var client = new Client(socket);

            Task.Factory.StartNew(client.Do);
        }
    }
}

class Client
{
    private readonly Socket _socket;
    private readonly NetworkStream _networkStream;
    private readonly MemoryStream _memoryStream = new MemoryStream();
    private readonly StreamReader _streamReader;

    public Client(Socket socket)
    {
        _socket = socket;
        _networkStream = new NetworkStream(socket, true);
        _streamReader = new StreamReader(_memoryStream);
    }

    public async void Do()
    {
        byte[] buffer = new byte[4096];
        int bytesRead = await _networkStream.ReadAsync(buffer, 0, buffer.Length);

        Console.WriteLine("Doing some awesome work that takes 5 seconds.");
        Thread.Sleep(5000);
        Console.WriteLine("Finished doing work.");

        await _networkStream.WriteAsync(buffer, 0, buffer.Length);
        await _networkStream.FlushAsync();
    }
}

Client:

class Program
{
    static void Main(string[] args)
    {
        Thread.Sleep(5000);

        Console.WriteLine("Connecting...");

        TcpClient client = new TcpClient("localhost", 6666);

        Console.WriteLine("Connected.");

        try
        {
            Stream stream = client.GetStream();

            Byte[] messageToSend = new Byte[3];
            messageToSend[0] = 1;
            messageToSend[1] = 3;

            Byte[] messageReceived = new Byte[3];

            stream.WriteAsync(messageToSend, 0, 3);

            stream.Flush();

            stream.Read(messageReceived, 0, 3);

            // Echo
            Console.WriteLine(messageReceived[0] + messageReceived[1]);

            stream.Close();
        }
        finally
        {
            client.Close();
        }
    }
}

You are holding on to some IDisposable resources that you shouldn't be, which will impact scalability, GC pressure and likely performance. Here's the augmented Listener class:

internal sealed class Listener : IDisposable
{
    private readonly int port;

    private readonly TcpListener tcpListener;

    private readonly IList<Client> clients = new List<Client>();

    public Listener(int port)
    {
        this.port = port;
        this.tcpListener = new TcpListener(IPAddress.Any, this.port);
        this.tcpListener.Start();
        Task.Factory.StartNew(this.ListenLoop);
    }

    public int Port
    {
        get
        {
            return this.port;
        }
    }

    public void Dispose()
    {
        foreach (var client in this.clients)
        {
            client.Dispose();
        }
    }

    private async void ListenLoop()
    {
        while (true)
        {
            var socket = await this.tcpListener.AcceptSocketAsync();

            if (socket == null)
            {
                break;
            }

            var client = new Client(socket, this.ClientDisconnected);

            this.clients.Add(client);
            await Task.Factory.StartNew(client.Do);
        }
    }

    private void ClientDisconnected(Client client)
    {
        if (client == null)
        {
            return;
        }

        client.Dispose();
        this.clients.Remove(client);
    }
}

Here's the augmented Client class:

internal delegate void ClientDisconnectedDelegate(Client client);

internal sealed class Client : IDisposable
{
    private readonly Socket socket;

    private readonly ClientDisconnectedDelegate doDisconnect;

    private readonly Stream networkStream;

    private readonly Stream memoryStream = new MemoryStream();

    private readonly TextReader streamReader;

    public Client(Socket socket, ClientDisconnectedDelegate doDisconnect)
    {
        this.socket = socket;
        this.doDisconnect = doDisconnect;
        Task.Factory.StartNew(this.CheckForConnection);
        this.networkStream = new NetworkStream(this.socket, true);
        this.streamReader = new StreamReader(this.memoryStream);
    }

    public void Dispose()
    {
        this.streamReader.Dispose();
        this.networkStream.Dispose();
        this.socket.Dispose();
        this.memoryStream.Dispose();
    }

    public async void Do()
    {
        var buffer = new byte[4096];
        var bytesRead = await this.networkStream.ReadAsync(buffer, 0, buffer.Length);

        Console.WriteLine("Doing some awesome work that takes 5 seconds.");
        Thread.Sleep(5000);
        Console.WriteLine("Finished doing work.");

        await this.networkStream.WriteAsync(buffer, 0, bytesRead);
        await this.networkStream.FlushAsync();
    }

    private void CheckForConnection()
    {
        while (true)
        {
            bool isDisconnected;

            try
            {
                isDisconnected = this.socket.Poll(1, SelectMode.SelectRead) && (this.socket.Available == 0);
            }
            catch (SocketException)
            {
                isDisconnected = true;
            }

            if (isDisconnected && (this.doDisconnect != null))
            {
                this.doDisconnect(this);
                return;
            }

            Thread.Sleep(100);
        }
    }
}

And here's the augmented Client program:

internal static class Program
{
    private static void Main()
    {
        Thread.Sleep(5000);

        Console.WriteLine("Connecting...");

        using (var client = new TcpClient("localhost", 6666))
        {
            Console.WriteLine("Connected.");

            using (var stream = client.GetStream())
            {
                var messageToSend = new byte[3];

                messageToSend[0] = 1;
                messageToSend[1] = 3;

                var messageReceived = new byte[3];

                stream.WriteAsync(messageToSend, 0, 3);

                stream.Flush();

                stream.Read(messageReceived, 0, 3);

                // Echo
                Console.WriteLine(messageReceived[0] + messageReceived[1]);
                Console.ReadLine();
            }
        }
    }
}


jQuery Browser Sniffer I'll be honest this isn't all my code, I have made some tweaks to bring it more up to date. The main goal is to do some browser detection and add classes to whichever element we want. I am simply looking for your input regarding the code.

// Detect browser and add appropriate classes to calling element, typically 'html'
// Usage: $('html').add_browser_classes();
// Test function: alert('html classes: ' + jQuery('html').attr('class'));

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

        // Begin with the negation classes

        $(this).addClass('javascript not-webkit not-firefox not-opera not-ie');

        if (/Chrome/.test(navigator.userAgent) && /Google Inc/.test(navigator.vendor)) {
            $(this).removeClass('not-webkit').addClass('webkit chrome');
        } else if (/Safari/.test(navigator.userAgent) && /Apple Computer/.test(navigator.vendor)) {
            $(this).removeClass('not-webkit').addClass('webkit safari');
        } else if (/Firefox/.test(navigator.userAgent)) {
            $(this).removeClass('not-firefox').addClass('firefox');
        } else if (window.opera && window.opera.buildNumber) {
            $(this).removeClass('not-opera').addClass('opera');
        } else if (Function('/*@cc_on return /^10/.test(@_jscript_version) @*/')()) {
            $(this).removeClass('not-ie').addClass('ie ie10');
        } else if (!document.querySelector) {
            $(this).removeClass('not-ie');
            $(this).addClass('ie ie7 lt-ie11 lt-ie10 lt-ie9 lt-ie8');
        } else if (!document.addEventListener) {
            $(this).removeClass('not-ie');
            $(this).addClass('ie ie8 lt-ie11 lt-ie10 lt-ie9');
        } else if (!window.atob) {
            $(this).removeClass('not-ie');
            $(this).addClass('ie ie9 lt-ie11 lt-ie10');
        } else if (!document.__proto__) {
            $(this).removeClass('not-ie');
            $(this).addClass('ie ie10 lt-ie11');
        } else if (!!(navigator.userAgent.match(/Trident/) && !navigator.userAgent.match(/MSIE/))){
            $(this).removeClass('not-ie');
            $(this).addClass('ie ie11');
        }
        //The last condition checks to see if the browser is IE 11 or greater
        //If we need to test for specific versions of IE 11 and above replace the above else if conditional to this -
        //!!(navigator.userAgent.match(/Trident/) && navigator.userAgent.match(/rv[ :]11/))
        //if it isnt obvious change the 11 in the conditional to the version you are looking for
        return this;
    };
})(jQuery);

First problem you'll face is that IE, Opera, Firefox, Chrome and Safari aren't the only browsers in town. There are a lot of other browsers! Even the Steam app is just a browser. Anything that uses a form of WebView, or embedded browser (like game client patcher news sections) is also a browser. That equates to a lot of UA strings to account for.

The next problem is the most common problem with this approach: UA strings can be spoofed. Making it worse, browsers up to this day still spoof their UA to pretend like some other browser. For instance, Chrome 45 has the "Safari" and "AppleWebKit". With your code up there, you can easily mistake it for Safari or some other WebKit browser, even when Chrome 45 isn't Safari nor WebKit.

Now if you do want to do it in this approach and not mind the problem of playing catch-up with the latest browsers, then your first problem would be the use of jQuery. You seem to use it just to add and remove classes. Why not construct your determinations in an array, then just form a string afterwards?

var determination = [];

if(chrome) determination.push('isChrome');
if(webkit) determination.push('isWebkit');

var classes = determination.join(' ');

Another thing I notice is that you add your classes on the element you're attaching the plugin to. This also means your script runs every time you attach it, which is unnecessary. Why not run this script just once, during page load? For CSS purposes, you can attach your determinations as a class of <html> and prefix your styles with the class. If you use it for conditional scripts, you can store your determinations in a global variable and use indexOf.

// Put classes to <html>
document.documentElement.className = document.documentElement.className + ' ' + classes;

// Hide all inputs on all non-ie browsers
.not-ie input{
  display: none;
}

// Checking if IE with JS
if(~determinations.indexOf('lt-ie11')) alert('Y U NO USE NEW BROWSER???');

Now for your tests. As mentioned earlier, you might want to be more specific on your tests so that you won't be confusing a browser with another. Also, your tests are tied to each other. I don't know where the IE tests start and end. Everything's all over the place. Try breaking them apart into "plugins" so that you can easily just add in new tests.

BrowserTester.addTest('IE test', function(ua){
  // Add to this array all the classes you want to be put for this browser
  var classes = [];

  // do comparisons to check for IE

  // Return a string of classes for this test.
  return classes.join(' ');

});

// Do the same for other browsers
BrowserTester.addTest('Chrome', function(ua){...});
BrowserTester.addTest('Safari5', function(ua){...});
BrowserTester.addTest('Safari6', function(ua){...});

// Run tests. Add classes to <html>, expose an API to check which browser.
BrowserTester.run();


Let's revisit Roman numbers Problem Statement:

  Write a function to convert from normal numbers to Roman Numerals:
  e.g.

 1  => I
10  => X
 7  => VII

Code:

import java.util.HashMap;
import java.util.Map;
import java.util.NavigableMap;
import java.util.TreeMap;

public class RomanNumeral {
  private final int number;
  private static final Map<Integer, String> CACHE = new HashMap<>();

  private static final NavigableMap<Integer, String> NUMBER_TO_ROMAN =
      initMap();

  public RomanNumeral(int number) {
    if (number < 0 || number > 3000) {
      throw new IllegalArgumentException("Number out of range.");
    }
    this.number = number;
  }

  public String getRomanNumeral() {
    if (number == 0) {
      return "";
    }
    String result = CACHE.get(number);
    if (result != null) {
      return result;
    }
    result = toRomen(number);
    CACHE.put(number, result);
    return result;
    //return number == 0 ? "" : toRomen(number);
  }

  private static String toRomen(int num) {
    int floorNum = NUMBER_TO_ROMAN.floorKey(num);
    // Base case
    if (floorNum == num) {
      return NUMBER_TO_ROMAN.get(num);
    }
    return NUMBER_TO_ROMAN.get(floorNum) + toRomen(num - floorNum);
  }

  private static NavigableMap<Integer, String> initMap() {
    NavigableMap<Integer, String> map = new TreeMap<>();
    map.put(1, "I");
    map.put(4, "IV");
    map.put(5, "V");
    map.put(9, "IX");
    map.put(10, "X");
    map.put(40, "XL");
    map.put(50, "L");
    map.put(90, "XC");
    map.put(100, "C");
    map.put(400, "CD");
    map.put(500, "D");
    map.put(900, "CM");
    map.put(1000, "M");
    return map;
  }
}

Test Suite:

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;

import java.util.Arrays;
import java.util.Collection;

import static org.junit.Assert.assertEquals;

@RunWith(Parameterized.class)
public class RomanNumeralsTest {

    private int input;
    private String expectedOutput;

    @Parameters
    public static Collection<Object[]> data() {
        return Arrays.asList(new Object[][]{
                {0, ""},
                {1, "I"},
                {2, "II"},
                {3, "III"},
                {4, "IV"},
                {5, "V"},
                {6, "VI"},
                {9, "IX"},
                {27, "XXVII"},
                {48, "XLVIII"},
                {59, "LIX"},
                {93, "XCIII"},
                {141, "CXLI"},
                {163, "CLXIII"},
                {402, "CDII"},
                {575, "DLXXV"},
                {911, "CMXI"},
                {1024, "MXXIV"},
                {3000, "MMM"}
        });
    }

    public RomanNumeralsTest(int input, String expectedOutput) {
        this.input = input;
        this.expectedOutput = expectedOutput;
    }

    @Test
    public void convertArabicNumberalToRomanNumeral() {
        RomanNumeral romanNumeral = new RomanNumeral(input);

        assertEquals(expectedOutput, romanNumeral.getRomanNumeral());
    }
}

Design Discussion:

I have used TreeMap as the keys are ordered.
I played a bit with CACHING here since I didn't want to do the same computation again and again.

Reference:

http://exercism.io/exercises/java/roman-numerals/readme

Your code is very clear and nicely written. Also, using a TreeMap to take advantage of ordered keys is great insight.

Overall, I practically only have good comments about the code:

Throwing an IllegalArgumentException in case of a negative or greater than 3000 number is a great way to document the fact those values are not allowed.
The recursive method toRomen handles all the case correctly. You made a small typo in the name of the method: it should be toRoman instead of toRomen.
Coding against the NavigableMap interface and hiding the TreeMap concrete instantiation inside a method is a nice thing to do.
Using a cache might indeed be a good idea: it helps to ensure that the same roman numerals are not converted each time and, since we're only interested in 3000 numbers at maximum, this map won't become gigantic and consume a lot of memory.

I still have a couple of suggestions:

You are currently handling the number 0 in a special early return inside getRomanNumeral with:

if (number == 0) {
    return "";
}

This isn't strictly needed. You have done that because toRomen doesn't handle 0 as input. Handling 0 could be done inside toRomen instead. Consider the following:

private static String toRoman(int num) {
    if (num == 0) {
        return "";
    }
    int floorNum = NUMBER_TO_ROMAN.floorKey(num);
    return NUMBER_TO_ROMAN.get(floorNum) + toRoman(num - floorNum);
}

Now the method toRoman has a clear-cut base case: when the number given is 0, we return an empty String. This also takes care of the recursive call with num - floorNum when both are equal: we will fall under this case and return the empty String.
With the above change, getRomanNumeral now becomes:

public String getRomanNumeral() {
    String result = CACHE.get(number);
    if (result != null) {
        return result;
    }
    result = toRomen(number);
    CACHE.put(number, result);
    return result;
}

We could use methods introduced in Java 8 to make this shorter. Mainly, we're interested in the computeIfAbsent(key, mappingFunction) method here. This method will return the currently mapped value to the given key, or invoke the given mapping function if there are no mapping. Therefore, we can simply have here:

public String getRomanNumeral() {
    return CACHE.computeIfAbsent(number, RomanNumeral::toRoman);
}

When the cache already contains the value for number, it will be returned directly. Otherwise, the method toRoman will be invoked by giving it the number as parameter, the cache will be updated with the value returned by the function and this value will be returned.


Converting text into l33t speak Write a method leetSpeak that accepts two parameters: a Scanner
  representing an input file, and a PrintStream representing an output
  file. Your method should convert the input file's text to "leet speak"
  (aka 1337 speak), an internet dialect where various letters are
  replaced by other letters/numbers. Output the leet version of the text
  to the given output file. Preserve the original line breaks from the
  input. Also wrap each word of input in parentheses. Perform the
  following replacements:

  For example, if the input file
  lincoln.txt contains the following text:

  four score and seven years ago our

  fathers brought forth on this continent a new nation And your method
  is called in the following way:

Scanner input = new Scanner(new File("lincoln.txt"));
PrintStream output = new PrintStream(new File("leet.txt"));
leetSpeak(input,output);

  Then after the call, the output file leet.txt should contain
  the following text:

(f0ur) (sc0r3) (4nd) (s3v3n) (y34rZ) (4g0) (0ur)
(f47h3rZ) (br0ugh7) (f0r7h) (0n) (7hiZ) (c0n7in3n7) (4) (n3w) (n47i0n)

  You may assume that each token from the input file is separated by
  exactly one space.

  Hint: You may want to use the String object's replace method, which is
  used as follows:

String str = "mississippi"; str = str.replace("s", "*");    // str = "mi**i**ippi"

I'm looking for general feedback, and other ways to accomplish the task of solving this problem. My hope is that there will be some interesting feedback on this that I can apply to my future coding. If there are any bugs in my code, please let me know.

public static void leetSpeak (Scanner sc, PrintStream ps) {
    while (sc.hasNextLine()) {
        String line = new String(sc.nextLine());
        Scanner tokenScanner = new Scanner(line);
        while (tokenScanner.hasNext()) {
            String token = new String(tokenScanner.next());
            token = token.replace("o", "0");
            token = token.replace("l", "1");
            token = token.replace("e", "3");
            token = token.replace("a", "4");
            token = token.replace("t", "7");
            if (token.substring(token.length() - 1).equals("s")) {
                    token = new String(token.substring(0, token.length() - 1));
                    token += "Z";
            }
            ps.print("(" + token + ")" + " ");
        }
    ps.println();
    }
}

For the sake of efficiency, I suggest you use a StringBuilder. The problem here is that String.replace will have to copy a this string applying the character replacement several times. With a StringBuilder you can apply those character replacements in a single pass over the input string. I had this in mind:

public static String to13375p34k(String word) {
    StringBuilder sb = new StringBuilder(word);

    for (int i = 0; i < sb.length(); ++i) {
        char currentCharacter = sb.charAt(i);

        switch (currentCharacter) {
            case 'o':
                sb.setCharAt(i, '0');
                break;

            case 'l':
                sb.setCharAt(i, '1');
                break;

            case 'e':
                sb.setCharAt(i, '3');
                break;

            case 'a':
                sb.setCharAt(i, '4');
                break;

            case 't':
                sb.setCharAt(i, '7');
                break;
        }
    }

    if (sb.charAt(sb.length() - 1) == 's') {
        sb.setCharAt(sb.length() - 1, 'Z');
    }

    return sb.toString();
}


Parsing JSON in one go I need to parse a simple JSON string (flat JSON, no hierarchy) for keys and values, and there is a system constraint that I cannot use any built-in JSON library and can only read a string once due to latency requirements. I need to use Python 2.7.x series and cannot use a higher version.

Currently, my major concern for below code is, I still do a bit more than one pass string parsing, since I need to go backward/forward to remove unnecessary characters around a word, which is what wordBeautify does.

This is a follow-up to "Json String Parsing"

def wordBeautify(word1, beginIndex, endIndex):
    noMeaningChars=[' ', '"', '{', ',', ':','}']
    while word1[beginIndex] in noMeaningChars:
        beginIndex+=1
    while word1[endIndex] in noMeaningChars:
        endIndex-=1
    return (beginIndex,endIndex+1)

def parseElegant2(str1):

    keyStr=''
    valueStr=''
    beginWord = False
    isKey=True
    beginIndex=0
    endIndex=0

    for i in range(len(str1)):

        if str1[i]==':':
            endIndex=i-1
            (x,y) = wordBeautify(str1, beginIndex, endIndex)
            keyStr=str1[x:y]
            print "key string " + keyStr
            beginIndex=i+1
        elif str1[i]==',' or str1[i]=='}':
            endIndex=i
            (x,y) = wordBeautify(str1, beginIndex, endIndex)
            valueStr=str1[x:y]
            beginIndex=i+1
            print "value string " + valueStr
            print 'key and value, '+ keyStr, valueStr

def parseElegant(str1):

    keyStr=''
    valueStr=''
    beginWord = False
    isKey=True
    beginIndex=0
    endIndex=0

    for i in range(len(str1)):

        if str1[i] == '"' and beginWord==False:
            beginWord=True
            beginIndex = i+1
        elif str1[i] == '"' and beginWord==True:
            beginWord=False
            endIndex=i
            if beginIndex<endIndex:
                print "get word, " + str1[beginIndex:endIndex]
        elif str1[i]==':':
            keyStr=str1[beginIndex:endIndex]
        elif str1[i]==',':
            valueStr=str1[beginIndex:endIndex]
            print 'key and value, '+ keyStr, valueStr

if __name__ == "__main__":

    #JSONString = '{ "id": 1, "name": "A green door", "price": 12.50, "tags": ["home", "green"]}'
    JSONString1 = '{ "id": "1", "name": "A green door", "price": "12.50", "tags": "home green"}'
    JSONString2 = '{ "id": 1, "name": "A green door", "price": 12.50, "tags": "home green"}'

    parseElegant2(JSONString1)
    parseElegant2(JSONString2)

First of all, let me say well done on the progress made from your first question to now!

Code Review is for reviewing code

As you seem to be seeking advice "about how to implement" an idea (i.e. streams), I think you may be better off asking on Programmers.SE, since that is a more appropriate place for questions about "software architecture and design" and "algorithm and data structure concepts".
Code Review is "not for questions about broken code, hypothetical code, or non-existent code, as such questions will be closed as off-topic." (taken from this help page).

I can see that you have code already, and are seeking to improve it (in a way that doesn't add any functionality) - so I consider your question to be on-topic.  Although your desire to learn about other ways of implementing it entirely may not be best placed on this site.

A Quick Review

Remove old code
You have included the (now unused) code for parseElegant (version 1).  While I can understand you not wanting to erase this, you have already linked to your previous question (which contains the old code).  Including version 1 here just adds ~24 lines for reviewers to have to scroll through.

Can't you just use strip?
The wordBeautify function (which you are seeking to make redundant/remove) seems to be VERY similar to the Python built-in string.strip method?

Consider the following code:

test="{:foo }"
(x,y)=wordBeautify(test,0,len(test)-1)
a=test[x:y]
b=test.strip(' "{,:}')

Result:  a and b both work out to be "foo"!

Use better variable names!
At present, your parseElegant2 function takes an argument called str1.  While it is obvious that this means something along the lines of the first string - it is not actually particularly helpful in knowing what that variable represents.

Something more like json, text_candidate, or candidate may be better - as it expresses that this is a long text containing json, or something that is a candidate to be processed into JSON.

You have a similar issue in wordBeautify, with word1.  This could just be named word, or similar.

Return is better than print
I've noticed that in your function you are not return'ing any values.  Instead you are just print'ing to the standard output (STDOUT).
Is this your goal? Or will your program eventually need to construct some kind of data structure (such as a dict) of the JSON values?

I would recommend refactoring your code to return a data structure, and have your function write the JSON values into that data structure.

A word on string stream processing

As @ferada commented, I think you should "consider using a stream, looking only at the (single) next character at a time and then build a state machine around it.", however your current program is not doing this.

Instead, your loop is going through the indices of the string.  And is (at present), also calling your wordBeautify function to do the same (with indices).

Rather than using for i in range(len(str1)):, as you currently are - I would recommend that you loop through the characters themselves (one at a time, as @ferada suggested).  Thus, your loop may look more like: for c in str1:

This way, you are essentially forced into only looking at the current character. (i.e. you are unable to increment/decrement any number value to go forwards or backwards in the string.)

Constructing a state machine

From the Wikipedia article about Finite-state machines:

  A finite-state machine (FSM) or simply state machine, is a mathematical model of computation used to design computer programs.  It is conceived as an abstract machine that can be in one of any finite number of states.  The machine is in only one state at a time, called the current state. It can change from one state to another when triggered, this is called a transition.  A particular state machine is defined by a list of its states, and the triggering condition for each transition.
   (Emphasis mine)

You can read the article for more background information, but essentially your are seeking to construct a state machine.  As you can read in the last sentence (that I've bolded), this requires:

a list of states
the trigger conditions for each state transition

What are the states?

Luckily, the official JSON website (JSON.org) actually gives us these exact things!
In the form of railroad diagrams.  Specifically, they tell us that:

  An object is an unordered set of name/value pairs. An object begins with { (left brace) and ends with } (right brace). Each name is followed by : (colon) and the name/value pairs are separated by , (comma).

  (source: json.org)

So we know that STRING is a state, for example.  They also tell us how we get into (and out of) the STRING state:

  A string is a sequence of zero or more Unicode characters, wrapped in double quotes...

  (source: json.org)

So we know that a STRING state is triggered when we encounter a " (quote) character. (And that it ends the same way.)
Note:  For now, let's ignore the backslash (\) escape style string components.

We also are told what a VALUE might be as well:

  A value can be a string in double quotes, or a number, or true or false or null...

  (source: json.org)

This gives us another state, namely NUMBER - since a value can be a number.

Some limitations (Let's not reinvent the whole wheel)

We already know that you only want to deal with "flat JSON (no hierarchy) for keys and values".
So, we'll forget about OBJECT and ARRAY in the diagram above. ;-)

Let's also (just to make it easy), ignore the fact that a value can be true, false, or null.  (i.e. it is either a string or a number).

The states so far

So far, we have been able to use the official JSON.org website diagrams to determine we have the following two states:

STRING
NUMBER

We also know that a string is triggered by the condition: c=='"' (where c is our stream's current character).

But how is a NUMBER triggered?
Well the JSON.org website has a rather complicated way of detecting numbers (which takes into account negative numbers, scientific notation, etc).  but for these purposes let's just say that a NUMBER is triggered by a sequence of 1 or more of the following characters: 0123456789. (the decimal point is included).

Other transitions

If we look back at the first railroad diagram:

We can see that the COLON (:) and COMMA (,) characters are also transitions between states..

Putting it all together

If we combine all of these states and transitions, plus the idea of a character (rather than index) loop, and the other improvements I've mentioned - we can produce a single function which is capable of processing a JSON string "all at once":

def parseElegant3(candidate):
    READY=0
    STRING=1
    NUMBER=3
    COLON=-1
    COMMA=-2

    search={'"':STRING,  ':':COLON,  ',':COMMA,  STRING:'"',  COMMA:','}
    NOP=" {\t}\r\n"
    stringIgnore='{,:}'
    numbers='0123456789.'

    result={}
    if not len(candidate):
        return result
    accumulator=""
    mostRecentKey=None

    state=READY
    for c in candidate:
        newState=state
        if state is READY:
            if c in NOP:continue
            if c in numbers:
                newState=NUMBER
            if c in search.keys():
                newState=search[c]
        if newState is COLON:
            newState=READY
            if len(accumulator):
                accumulator=accumulator.strip()
                result[accumulator]=None
                mostRecentKey=accumulator
                accumulator=""
            else:
                state=READY
                continue
        elif newState is COMMA:
            newState=READY
        elif newState is NUMBER and not state is NUMBER:
            accumulator+=c
        if state is STRING and newState is STRING:
            if c in stringIgnore:continue
            if c==search[STRING]:
                newState=READY
                if not mostRecentKey is None:
                    result[mostRecentKey]=accumulator
                    accumulator=""
                    mostRecentKey=None
            else:
                accumulator+=c
        elif state is NUMBER:
            if c in numbers:
                accumulator+=c
            else:
                if c in NOP:continue
                assert c == search[COMMA]
                try:
                    result[mostRecentKey]=int(accumulator)
                except ValueError:
                    result[mostRecentKey]=float(accumulator)
                accumulator=""
                mostRecentKey=None
                newState=READY
        state=newState
    return result

JSONString = '{ "id": 1, "name": "A green door", "price": 12.50, "tags": "home green"}'

parseElegant3(JSONString)


Simple Translation Gem I have a small gem that I've been maintaining. This gem provides a simple way to do translation using an alternative Google API.

I've been working with Java last year and recently did some refactoring on that gem, but I don't think it's Rubyist enough.

require_relative 'extractor'
require_relative 'requester'
require_relative 'translator'

module FreeGoogle
  ##
  # Responsible for create instance of {FreeGoogle::Translator}
  # It holds all dependencies for this creation
  class Factory
    def self.create
      extractor = FreeGoogle::Extractor.new
      http_requester = FreeGoogle::Requester.new
      FreeGoogle::Translator.new(extractor, http_requester)
    end
  end
end

Source code

Are factories commonly used in Ruby?

It's hard to say much about such a short code sample, but factories simply encapsulate object creation (wiring up objects), which frees code using them from that responsibility.  In that sense, factories are relevant in ruby too.

However, note that client code would look like this, given your current code:

translator = FreeGoogle::Factory.create

which isn't as readable as it could be.  I think I'd prefer to write something like the following in my client code:

translator = FreeGoogle.new_translator


Defaulting parameters to false In my rails controllers,  I am consistently using this logic:

def report
  dump = params[:dump] ? params[:dump] : 'false'

  data = {}

  if dump != 'false'
    return render :json => p(data)
  end
end

Is there a better way to write this?  A ternary operation is just an eyesore.  I thought maybe just using the || operator:

dump = params[:dump] || false

But that doesn't yield the desired result.

How can I better write this?

I'm unclear how you are using dump = false and whether the method returns anything more than nil when params[:dump] is unassigned. If it doesn't, I would recommend the use of a guard clause:

def report
  return unless params[:dump]
  data = {}
  render :json => p(data)
end


Implementing an inner product using pyspark I'm trying to implement a dot product using pyspark in order to learn pyspark's syntax.

I've currently implemented the dot product like so:

import operator as op
from functools import reduce

def inner(rdd, rdd2):
    return (rdd.zip(rdd2)
        .map(lambda x: reduce(op.mul, x))
        .reduce(lambda x,y: x + y)
    )

My solution feels inelegant (particularly my lambda function). I'd like to know if there would be a more 'pysparkian' way of writing this.

Furthermore, are there performance considerations that I should be thinking about with regards to this problem (i.e. does my dot product solution not scale well)?

Since zipped RDD contains only two-element tuples using using reduce doesn't makes sense. You can safely replace

lambda x: reduce(op.mul, x)

with

lambda x: x[0] * x[1]

or standalone function:

def mul(xy):
    x, y = xy
    return x * y

Final reduce can be replaced with a simple sum. Putting these two pieces together:

def inner(xs, ys):
    return xs.zip(ys).map(lambda xy: xy[0] * xy[1]).sum()

The only part that remains is zip function. It requires both that both RDDs have the same number of partitions and elements per partition. While the first part is pretty easy to achieve keeping a correct order and balancing number of elements can be tricky. You can try to generalize your function using joins:

def inner(xs, ys):
    def swap(xi):
        x, i = xi
        return i, x

    # We use sortBy key to avoid shuffling during join
    xs_i = xs.zipWithIndex().map(swap).sortByKey()
    ys_i = ys.zipWithIndex().map(swap).sortByKey()

    return xs_i.join(ys_i, ).values().map(lambda xy: xy[0] * xy[1]).sum()

It will be less efficient than your current solution but more robust.