Take from top, evaluate, put on bottom In this example, I am only looking for the first duplicate - curious if this logic in for loop (take from the top, evaluate, put on the bottom) pattern has a standard form

function findOneDupe(input){
  var b = input.split(/\s|\n/).map(Number);
  b.shift();
  for (var i=0;i<b.length;i++){
    var e = b.shift();
    if (b.indexOf(e) == -1) return e;
    b[b.length] = e;
  }
  return -1;
}

function findOneDupe(input){

You name this function findOneDupe, but this code finds the first element that is not duplicated.

    function findFirstUnique(input) {

Now it does what it says, not the opposite of what it says.

  var b = input.split(/\s|\n/).map(Number);

Why call this b?  That gives me no idea what it holds.

  var tokens = input.split(/\s|\n/).map(Number);

The name tokens is far more understandable in terms of whitespace delineated text.

  b.shift();

Why?  Presumably you have some cruft to remove from the array.  Comment this and explain why it needs to be done.  Then you don't have to worry about someone optimizing it out later.

  for (var i=0;i<b.length;i++){
    var e = b.shift();
    if (b.indexOf(e) == -1) return e;
    b[b.length] = e;
  }

This seems more complicated than it needs to be.  You are both maintaining an index variable and removing from the array.  Why not pick one or the other?

  for (var i=0; i < tokens.length; i++) {
    if (tokens.indexOf(tokens[i], i+1) == -1 && tokens.indexOf(tokens[i]) == i) {
      return tokens[i];
    }
  }

Now we don't add or remove from the array at all.  Which is good, since Javascript arrays are not lists.  Removing and adding can cause the array to not fit its memory allocation anymore.  When that happens, the array will be copied into a new memory location.  Pure waste considering that you never use the array outside this function.

Note that the two indexOf checks will never check more elements than your single check.  The first one only checks things after i and the second one only checks up to i (because it always finds at i).

Note that this takes \$O(n^2)\$ time in the worst case, as you loop over every element and search the array for it.  You can get better asymptotic performance by sorting the array.

  tokens.sort();

  var i = 1;
  while (i < tokens.length) {
    if (tokens[i-1] != tokens[i]) {
      return tokens[i-1];
    }

    do {
      i++;
    } while (tokens[i-1] == tokens[i] && i < tokens.length);

    // we want to get all the way past anything equal
    // to the original tokens[i-1] value
    i++;
  }

This takes \$O(n \log n)\$ time for the sort and linear time to find any unique elements.


Javascript plugin DRY I have written a Backbone View (javascript) for a component called "twist-panel".

A twist panel is basically a card flip, it has a front and back side, and can flip to the back and to the front.

Here is the code:

var TwistPanel = Backbone.View.extend({
  initialize: function() {
    this.$front = this.$el.find('.twist-panel-side-front');
    this.$back = this.$el.find('.twist-panel-side-back');

    this.atFront = true;
    this.isFlipping = false;

    // Fix the height of the twist panel container.
    this.$el.height(this.$front.outerHeight(true));
  },
  toBack: function() {
    var self = this;

    if (this.isFlipping || !this.atFront) return;

    // Fix height.
        this.$el.height(this.$back.outerHeight(true));

        this.$front.afterTransition(function() {
            self.atFront = false;
            self.isFlipping = false;
            self.trigger('twist', 'back');
        });

        this.isFlipping = true;
        this.$el.addClass('twist');
  },
    toFront: function(callback) {
            var self = this;

        if (this.isFlipping || this.atFront) return;

        // Fix height.
        this.$el.height(this.$front.outerHeight(true));

        this.$back.afterTransition(function() {
            self.atFront = true;
            self.isFlipping = false;
            self.trigger('twist', 'front');
        });

        this.isFlipping = true;
        this.$el.removeClass('twist');
    }
});

As you can see in the code above, there is a lot of code duplication.
I would really appreciate some help on getting the code DRY.

The usual way I approach the problem is by asking: what does change?

self.atFront is flipped: not an issue here, you can simply write self.atFront = !self.atFront.
conditions using atFront vs. !atFront (not an issue: if we factorize everything, we don't need to know what the state is: we just want to change it)
the string in self.trigger
$front vs. $back
removeClass() vs. addClass().

Quite a lot of things change, actually! At this point, we don't know yet if avoiding duplication is worth it: it often leads to more complicated code and it can be longer, too. But let's try it. I took your code, created a flip function, and tried to see what could fit in it. When writing this, I realized that it was simpler to put everything in the flip function, so I tried it:

var TwistPanel = Backbone.View.extend({
  initialize: function() {
    this.$front = this.$el.find('.twist-panel-side-front');
    this.$back = this.$el.find('.twist-panel-side-back');

    this.atFront = true;
    this.isFlipping = false;

    // Fix the height of the twist panel container.
    this.$el.height(this.$front.outerHeight(true));
  },
  flip: function() {
    var self = this;
    var otherSide = ... ? this.$back : this.$front;

    if (this.isFlipping) return;

    this.$el.height(otherSide.outerHeight(true));

    this.$front.afterTransition(function() {
         self.isFlipping = false;
         self.trigger('twist', otherSide.name);
    });

    this.isFlipping = true;
    if (this.$el.hasClass('twist')) {
        this.$el.removeClass('twist')) {
    } else {
        this.$el.addClass('twist');
    }
  }
});

(I don't know about your application, but I guess toFront can never be called when we're already in front. If it's not true, then we need to keep toFront and toBack which will call flip themselves.)

At this point, I have two big issues left: I don't know how to decide what is the other side, and I don't know how to get its name (otherSide.name, used in the self.trigger() call). Since I don't know about this, I decided to resolve another issue, even if it is minor. This issue is that the removeClass/addClass logic looks wrong. It would probably be simpler to use something that is more than a value, and HTML5 data-* attributes are perfect for this since they are key-value and supported by jQuery. You can then replace the last five lines by this single one:

this.$el.data('twist', otherSide.name);

$el will either contain data-twist="front" or data-twist="back": you then need to change your CSS to account for this minor change. What's nice here is that it's now possible to get the string you're interested in (front or back). And it's also easy to assign otherSide. We'll use jQuery data() for this. Here's the final untested code:

var TwistPanel = Backbone.View.extend({
  initialize: function() {
    this.$front = this.$el.find('.twist-panel-side-front');
    this.$back = this.$el.find('.twist-panel-side-back');

    this.isFlipping = false;

    // Fix the height of the twist panel container.
    this.$el.height(this.$front.outerHeight(true));
  },
  flip: function() {
    var self = this;
    var otherSide = this.$el.data('twist') === 'front' ? this.$back : this.$front;
    var otherSideName = this.$el.data('twist') === 'front' ? 'back' : 'front';

    if (this.isFlipping) return;

    this.$el.height(otherSide.outerHeight(true));

    this.$front.afterTransition(function() {
         self.isFlipping = false;
         self.trigger('twist', this.$el.data('twist'));
    });

    this.isFlipping = true;
    this.$el.data('twist', otherSideName);
  }
});


Encrypted text - C# SHA256, implementation I have a C# application which needs to encrypt string and save the key as a hash. The key is saved as a hash to check the right key has been input before trying to decrypt the message.
First, I have these method to generate the hash string - hopefully "off the shelf" and ready to go
        public static byte[] GetHash(string inputString)
        {
            using (HashAlgorithm algorithm = SHA256.Create())
                return algorithm.ComputeHash(Encoding.UTF8.GetBytes(inputString));
        }

        public static string GetHashString(string inputString)
        {
            StringBuilder sb = new StringBuilder();
            foreach (byte b in GetHash(inputString))
                sb.Append(b.ToString("X2"));

            return sb.ToString();
        }

Then my encryption extension method:
private const int Keysize = 256;
private const int DerivationIterations = 1000;

public static string Encrypt(this string plainText, string passPhrase)
        {
            // Salt and IV is randomly generated each time, but is preprended to encrypted cipher text
            // so that the same Salt and IV values can be used when decrypting.
            var saltStringBytes = Generate256BitsOfRandomEntropy();
            var ivStringBytes = Generate256BitsOfRandomEntropy();
            var plainTextBytes = Encoding.UTF8.GetBytes(plainText);
            using (var password = new Rfc2898DeriveBytes(passPhrase, saltStringBytes, DerivationIterations))
            {
                var keyBytes = password.GetBytes(Keysize / 8);
                var engine = new RijndaelEngine(256);
                var blockCipher = new CbcBlockCipher(engine);
                var cipher = new PaddedBufferedBlockCipher(blockCipher, new Pkcs7Padding());
                var keyParam = new KeyParameter(keyBytes);
                var keyParamWithIV = new ParametersWithIV(keyParam, ivStringBytes, 0, 32);

                cipher.Init(true, keyParamWithIV);
                var comparisonBytes = new byte[cipher.GetOutputSize(plainTextBytes.Length)];
                var length = cipher.ProcessBytes(plainTextBytes, comparisonBytes, 0);

                cipher.DoFinal(comparisonBytes, length);
                //                return Convert.ToBase64String(comparisonBytes);
                return Convert.ToBase64String(saltStringBytes.Concat(ivStringBytes).Concat(comparisonBytes).ToArray());
            }
        }

private static byte[] Generate256BitsOfRandomEntropy()
        {
            var randomBytes = new byte[32]; // 32 Bytes will give us 256 bits.
            using (var rngCsp = new RNGCryptoServiceProvider())
            {
                // Fill the array with cryptographically secure random bytes.
                rngCsp.GetBytes(randomBytes);
            }
            return randomBytes;
        }

Which is then called to a input string:
var encryptedString = inputString.Encrypt(_appSettings.EncryptionSecret + passPhrase);
The passphrase is a random set of characters, prepended with an application string.
My concern is that the passphrase is 4 sets of "diceware" words (like here: https://diceware.dmuth.org/) and therefore could be brute forced. Would adding more to the passphrase, such as as hash of DateTime.UtcNow.ToString("yyyyMMdd") or GetHashString(random-string) for example add any additional security or is it just theatre and the "diceware" passphrase is practicably unbreakable?

My concern is that the passphrase is 4 sets of "diceware" words (like here: https://diceware.dmuth.org/) and therefore could be brute forced.

The diceware password is about 51 bits in strength, so it should indeed be run through a password based key derivation function, especially if you use it to encrypt messages - because encryption can be broken offline (i.e. on the computer of an attacker).

Would adding more to the passphrase, such as as hash of DateTime.UtcNow.ToString("yyyyMMdd") or GetHashString(random-string) for example add any additional security or is it just theatre and the "diceware" passphrase is practicably unbreakable?

No, the passphrase is not big enough to be unbreakable. Adding more information could help, but only if it is not available to an attacker, which is very questionable for a date / time.

private const int Keysize = 256;

This is good because we generally use key sizes in bits (and "size" is better than "length" in my opinion).
private const int DerivationIterations = 1000;

This is not a good idea at all. First of all, you should use about a million as iteration count at the moment. It would also be a big boon if you could upgrade it to a higher count later on. The amount of operations that an attacker has to perform is the same as the number of iterations, and an attacker may use a very fast implementation. Currently it adds about log_2(1000) =~ 10 bits of security.
public static byte[] GetHash(string inputString)

Especially if this is a public method (why?) then the hash function and encoding performed on the input string should be documented. It is not clear why the input needs to be a string in the first place though.
public static string GetHashString(string inputString)

Same for the output encoding of course.
StringBuilder sb = new StringBuilder();
foreach (byte b in GetHash(inputString))
    sb.Append(b.ToString("X2"));

Reprogramming a hex encoder doesn't make sense, and why not use a generic hex encoding function? In the end you want to have something like: hex(hash(utf8(text))) in your code...
var saltStringBytes = Generate256BitsOfRandomEntropy();

No, that's 256 bits of randomness you generate, the entropy is gathered by the operating system in the end. Why not just call it a salt? A StringByte is not a structure I've ever heard of.
 using (var password = new Rfc2898DeriveBytes(passPhrase, saltStringBytes, DerivationIterations))

This badly named class of Microsoft implements PBKDF2, which is defined in the Password Based Encryption standard. So what about var pbkdf2 =  because the returned object certainly is not a password.
Note that PBKDF2 is inefficient if you ask more than the configured hash output size. That defaults to SHA-1. If you'd use SHA-512 instead then you could retrieve a key, and IV and a check value all from a 512 bit / 64 byte output (32 bytes for the key, 16 bytes for the IV and check value each, for instance).
var engine = new RijndaelEngine(256);

Why would you use Bouncy Castle, a software only provider for bog-standard AES functionality? Do you hate the AES-NI instruction set?
var keyParamWithIV = new ParametersWithIV(keyParam, ivStringBytes, 0, 32);

I'm not sure, but I'm pretty confident that ivStringBytes is already the correct size, so the offset and length should not be needed.
var comparisonBytes = new byte[cipher.GetOutputSize(plainTextBytes.Length)];

Comparison to what? You have encrypted a message, right? If you just want to check if a password is correct then just use the password hash (PBKDF2).


dynamic_pointer_cast for std::unique_ptr dynamic_pointer_cast is only implemented for std::shared_ptr, I need it for unique pointers.
The wrinkle is that dynamic_casting a pointer could fail (yield nullptr), what to do in that case? I decided that in that case i would like the original pointer to remain intact. So i have implemented the following:
template< typename T, typename S >
inline std::unique_ptr<T> dynamic_pointer_cast(std::unique_ptr<S>&& ptr_)
{
    T* const converted_ptr = dynamic_cast<T*>(ptr_.get());
    if (!converted_ptr)
        // cast failed, leave input untouched, return nullptr
        return nullptr;

    // cast succeeded, clear input, return casted ptr
    ptr_.release();
    return std::unique_ptr<T>(converted_ptr);
}

Testing code:
#include <memory>
int main(int argc, char **argv)
{
    std::unique_ptr<Base> basePtr = std::make_unique<Derived1>();
    // this should fail, basePtr should remain non-empty, return should be empty
    auto deriv2Ptr = dynamic_pointer_cast<Derived2>(std::move(basePtr));
    // this should succeed, basePtr should become empty, return should be non-empty
    auto deriv1Ptr = dynamic_pointer_cast<Derived1>(std::move(basePtr));

    return 0;
}

Is this safe? Does the interface make sense? For the latter question: I decided to take an R-value ref so users have to write std::move, denoting pointer will be emptied. But then it may not if the cast fails... Normal ref is the other option, but then its less clear at the call site that the unique_ptr will likely be cleared.

Since it's a template function, it doesn't need to be marked inline.

It would be nice to handle custom deleters too. We can access the deleter with ptr_.get_deleter().


Python 3.6 Rock-Paper-Scissors game I was asked to write a "Rock Paper Scissors" game as a practice exercise for class. I ended up with this code which works as intended. It's a one player vs. the computer game.

Overview:

Computer and player choose their 'weapons'
Decide the winner
Print the winner

Does anyone have any constructive feedback or tips on how I could improve this?

import random as r

#the possible moves in rock paper scissors
RPS = ["ROCK", "PAPER", "SCISSORS"]

def main():
    comp = computerMove()
    player = playerMove()
    winner, tie = calcWinner(comp, player)
    dispWinner(winner, tie)

#the move made by the computer
def computerMove():
   comp =  r.choice(RPS)
##   print(comp)
   return comp

#move made by the user
def playerMove():
    player = ""
    while not player in RPS:
            player = input("Choose your weapon(rock, paper, or scissors: ").upper()
##    print(player)
    return player

#determining a winner
def calcWinner(comp, player):
    print("The computer chooses {0}.".format(comp))
    winner = None
    tie = False

## if theres a tie
    if player == comp:
        print("No winner.... Restarting....\n")
        main()
        tie = True

## if the user wins
    elif ((comp == "ROCK") and (player == "PAPER")) or ((comp=="PAPER") and (player == "SCISSORS")) or ((comp=="SCISSORS") and (player == "ROCK")):
        winner = True
## if computer won
    else:
        winner = False

    return winner, tie

def dispWinner(winner, tie):
    if tie == True:
        print("")
    elif winner == True:
        print("You beat the computer!")
    elif winner == False:
        print("The computer beat you!")

main()

First of all, welcome to codereview! It's nice to see that you are attending Python courses at University / highschool.

Styling

As you're at the beginning of the road, you should know that Python has a style-guide which is called PEP8. You can read there things like:

Naming conventions: function / variable names should be lower_cased
Comments: you should add a space after # (a single one is enough, it's not necessary to abuse it) when you comment out the code, you should avoid obvious comments and also try to write docstrings to each function
Maximum line length: lines should have a maximum length of 79 characters (this is debatable as some editors - like PyCharm suggests a maximum of 120).
Spacing: you should have two new lines between methods, and one whitespace around operators

That being said, your code after some styling might look like this:

import random as r

RPS = ["ROCK", "PAPER", "SCISSORS"]

def main():
    """This is the main function"""
    comp = computer_move()
    player = player_move()
    winner, tie = calc_winner(comp, player)
    disp_winner(winner, tie)

def computer_move():
    """The move made by the computer."""
    comp = r.choice(RPS)
    return comp

def player_move():
    """The move made by the user."""
    player = ""
    while not player in RPS:
        player = input("Choose your weapon(rock, paper, or scissors: ").upper()
    return player

def calc_winner(comp, player):
    """Determining a winner."""
    print("The computer chooses {0}.".format(comp))
    winner = None
    tie = False

    if player == comp:
        print("No winner.... Restarting....\n")
        main()
        tie = True
    elif (((comp == "ROCK") and (player == "PAPER")) or
          ((comp == "PAPER") and (player == "SCISSORS")) or
          ((comp == "SCISSORS") and (player == "ROCK"))):
        winner = True
    else:
        winner = False

    return winner, tie

def disp_winner(winner, tie):
    """Display the winner of the game."""
    if tie == True:
        print("")
    elif winner == True:
        print("You beat the computer!")
    elif winner == False:
        print("The computer beat you!")

main()

More about styling

Keep in mind that your comments should be succinct and explicit and should emphasise what the method does.

I usually put the main() function after the others because it comes more naturally when you read the code.

Give descriptive names to your functions / variables. Doing this will make the code speak for itself (more readable) and will make you remove unnecessary comments.

You only use random once. Don't give it an alias.

Equality vs Identity Comparisons

is is identity testing, == is equality testing.

You should use:

if tie is True:
    ...

Instead of:

if tie == True:
    ...

Or shorted:

if tie:
    ...

! Remember that if you use is, the result will be true if and only if the object is the same object. However, == will be true any time the values of the object are the same.

Testing for membership

You should write:

while player not in RPS:
    ...

Instead of:

while not player in RPS:
    ...

Tuples vs Lists

Because your RPS is not changing, I'd recommend you use a tuple instead. A couple of advantages of doing the above:

  Tuples are faster than lists. If you're defining a constant set of values and all you're ever going to do with it is iterate through it,
  use a tuple instead of a list.
  It makes your code safer if you “write-protect” data that does not need to be changed. Using a tuple instead of a list is like having an
  implied assert statement that this data is constant, and that special
  thought (and a specific function) is required to override that.

Fix the bug

If the computer chooses scissors and you choose paper there won't be any result because you only test for half of the cases.

Choose your weapon(rock, paper, or scissors: paper
The computer chooses SCISSORS.

Process finished with exit code 0

I'll let you fix this. HINT (think of a suitable data structure that can make the things easier)

Double hint: maybe a dictionary?


Generic parameter class with HasChanged functionality I have designed a class that allows the client to set an initial value. The idea is that if later on we wish to update this value then we can use the Update method and pass in the new value. We can then use the HasChanged() method to do something with the new value if it is different than the initial value.

public class UpdatableParam<T>
{
    private readonly T m_initialValue;
    private T m_updatedValue;

    public T InitialValue
    {
        get { return m_initialValue; }
    }

    public T UpdatedValue
    {
        get { return m_updatedValue; }
    }

    public UpdatableParam(T initialValue)
    {
        m_initialValue = ObjectCopier.Clone(initialValue);
        m_updatedValue = ObjectCopier.Clone(initialValue);
    }

    public UpdatableParam(T initialValue, T updatedValue)
    {
        m_initialValue = ObjectCopier.Clone(initialValue);
        m_updatedValue = ObjectCopier.Clone(updatedValue);
    }

    public void Update(T updatedValue)
    {
        m_updatedValue = ObjectCopier.Clone(updatedValue);
    }

    public virtual bool HasChanged()
    {
        return !EqualityComparer<T>.Default.Equals(m_initialValue, m_updatedValue);
    }
}

To ensure that the values cannot be changed from outside of the class after being added I have deep copied from the values passed in, both from the constructor and the update method.

The HasChanged() method works for most types I care about and I have one overloaded version of the class for lists. The implementation is as follows:

public class UpdatableListParam<T> : UpdatableParam<List<T>>
{
    public UpdatableListParam(List<T> initialValue)
        : base(initialValue)
    {

    }

    public override bool HasChanged()
    {
        var initialValue = InitialValue;
        var updatedValue = UpdatedValue;

        if (initialValue == null && updatedValue == null) return false;
        if (initialValue == null || updatedValue == null) return true;

        return !initialValue.SequenceEqual(updatedValue);
    }
}

I understand I could use IEnumerable rather than lists but have not yet implemented that change.

Is there anything fundamentally wrong with the above code? Is there any design change I could make to improve functionality? Or better yet has someone already beat me to this and is there anything already available perhaps in a NuGet package that does the above for me?

Edit - Adding Use Case

class SomeSourceInfo
{
    private SomeSource m_someSource;
    private UpdatableParam<string> m_name;

    public SomeSourceInfo(ISomeSource someSource)
    {
         m_someSource = someSource;
    }

    public void PopulateFromSource()
    {
        // Query from a source(could be a db etc.)
        m_someSource.Refresh();
        var srcName = m_someSource.Name;
        m_name = new UpdatableParam<string>(srcName);
    }

    public void UploadToSource()
    {
        if(m_name.HasChanged())
        {
             // Upload back to source
             m_someSource.Name = m_name.UpdatedValue;
        }

        PopulateFromSource();
    }

    public void UpdateSourceName(string newName)
    {
        // Allow changing of the name through UI calls or API etc.
        m_name.Update(newName);
    }
}

This only shows one potential param but in real life we may have hundreds of real params we can update and we only want them to upload back to the source when the user tells us to through the  UploadToSource() method and therefore need to cache the values in the mean time.

I'm still not sure about its usage but here's the review.

public T InitialValue
{
    get { return m_initialValue; }
}

public T UpdatedValue
{
    get { return m_updatedValue; }
}

As a user of this class I most probably don't want to think about which value I should use. I bet it'll be always the latest one so a single property like Value returning the most up-to-date value would be more appropriate.

public UpdatableParam(T initialValue, T updatedValue)
{
    m_initialValue = ObjectCopier.Clone(initialValue);
    m_updatedValue = ObjectCopier.Clone(updatedValue);
}

Initializing the class already with an updated-value is somehow counterintuitive to me. Why would I want to do that? I thought it should track changes and not be created in a changed state already.

public UpdatableParam(T initialValue)
{
    m_initialValue = ObjectCopier.Clone(initialValue);
    m_updatedValue = ObjectCopier.Clone(initialValue);
}

I don't want to update the value. I just want to set a new one via the Value property. The object tracking it should be able to figure out what's going on which leads me to the next point:

public virtual bool HasChanged()
{
    return !EqualityComparer<T>.Default.Equals(m_initialValue, m_updatedValue);
}

This should a property, usulay like IsDirty.

With all suggestions applied I could imagine the new clean object could be implemented like this:

public class TrackingValue<T>
{
    private readonly T _initialValue;
    private T _newValue;

    public TrackingValue(T initialValue)
    {
        _initialValue = ObjectCopier.Clone(initialValue);
        _newValue = _initialValue; // No cloning, it's the same value.
    }

    public T Value
    {
        get => IsDirty ? _newValue : _initialValue;
        set => _newValue = ObjectCopier.Clone(value);
    }

    public virtual bool IsDirty => !EqualityComparer<T>.Default.Equals(_initialValue, _newValue);

    public static implicit operator T(TrackingValue<T> value) => value.Value;
}

Additionally I want to be able to use the value without effort thus the implicit cast operator so that I can put the wrapper where I usually use the actual value and the compile will figure out the rest.


Karger's min-cut algorithm implemented in python I implemented Karger's algorithm with the function find_min_cut and it works, but I don't feel satisfied with the code I wrote.

from numpy import inf
from random import choice
from copy import deepcopy

# We use the karger method for finding the minimum cut in a graph.
# Each row of "original_Graph" needs to match a vertice and each element of
# the row corrispond to the vertices to which it's connected.
# In addition in the first row there needs to be the first vertice, the second
# row the second vertice and so on.
# You also need to input the number of times you want the random "find_cut" to
# be performed.

def find_min_cut(original_Graph, num_repeats):
    min_cut = inf
    for ith_repeat in range(1, num_repeats + 1):
        print ith_repeat, "th repeat"
        Graph = deepcopy(original_Graph)
        cut = find_cut(Graph)
        if cut < min_cut:
            min_cut = cut
    return min_cut

def find_cut(Graph):
    # list_vertices keeps the the list of vertices and can be used for
    # selecting a vertice in the graph by using list_vertices.index(vertice).
    # It basically helps to keep track of the vertices in the graph.

    list_vertices = range(1, len(Graph) + 1)

    # I contract the edges until there are only two vertices
    for num_contraction in range(len(Graph) - 2):
        contract_edge(Graph, list_vertices)
    return count_cut(Graph)

def contract_edge(Graph, list_vertices):

    # I choose randomly two vertices connected to select and edge.
    rand_vertice = choice(list_vertices)
    edge = [rand_vertice, choose_edge(Graph, rand_vertice, list_vertices)]

    unite_edge(Graph, edge, list_vertices)
    clean_graph(Graph, edge, list_vertices)

def count_cut(Graph):
    # When you finished contracting the number of connection between the two
    # vertices is the length of the row containing the connection of the
    # vertices
    return len(Graph[0])

def choose_edge(Graph, vertice, list_vertices):
    # I choose a random connected vertice
    return choice(Graph[list_vertices.index(vertice)])

def unite_edge(Graph, edge, list_vertices):
    new_vertice = list_vertices.index(edge[0])
    old_vertice = list_vertices.index(edge[1])

    # Extend the first vertice with the connection of the second
    Graph[new_vertice].extend(Graph[old_vertice])

    # delete the copied vertice both from the graph and the list of vertices
    del Graph[old_vertice]
    del list_vertices[old_vertice]

def clean_graph(Graph, edge, list_vertices):
    # I iterate over the vertices and save the index of the row matching
    # of each vertice.
    for vertice in list_vertices:
        index_vertice = list_vertices.index(vertice)
        # I substitute the vertice deleted in unite_edge with the remaining
        # one
        if edge[1] in Graph[index_vertice]:
            subtitute_all_elements(Graph, index_vertice, edge[1], edge[0])
        if vertice == edge[0]:
            # I remove the self-connection
            Graph[index_vertice] = filter(
                lambda a: a != edge[0], Graph[index_vertice])

def subtitute_all_elements(Graph, index_vertice, old, new):
    for i in range(len(Graph[index_vertice])):
        if Graph[index_vertice][i] == old:
            Graph[index_vertice][i] = new

So I'm asking for a review. I have some main point that I would like to have commented:

The quality of my comments. Are they clear enough? too many? or to little?
The variables names are good enough?
using deepcopy in find_min_cut is a bad choice? would you have done differently?
Are there ways that can make it faster?
Would you have approached the problem differently? If yes why?

If you have other suggestions or thoughts please tell.

edit.
An example of a graph is:

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

the first list in the list corrispond to the vertice connected with the first vertice, the second list in the list corrispond to the vertice connected with the second vertice and so on. It is an undirected graph.

contract_edge does not select edges with uniform distribution, because first a random vertex is selected and then a random edge of that vertex is chosen. This produces a skewed distribution on edges, with edges on low degree vertices having a higher chance of beeing selected than those of high degree vertices.


Equivalent of C-style "Static Local Variable" in Swift I'm porting some Obj-C code to Swift, and I've written the following code to allow me to deal with "static local variables" which do not exist in Swift.

A static local variable has these requirements:

it is shared between all instances that access it
it has one assignment method which only sets the value when it is first declared.
has another assignment method which sets its value normally (i.e. any time it is used)

There has to be a better way then what I have coded. For starters, I know that using  UnsafeBitcast is not good practice.

class Container<T:Any>{

    var _memory:Any
    var memory:T {
        get {

            if let typed_value = self._memory as? T {
                // for value types, such as "String"
                return typed_value
            } else {
                // for types conforming to "AnyObject", such as "NSString"
                return unsafeBitCast( self._memory, T.self )
            }

        }
        set {
            self._memory = newValue
        }
    }

    init( memory:Any ){
        self._memory = memory
    }

}

var Static_Containers = [String:AnyObject]()

func static_var <T:Any>(
    value:T,
    file: StaticString = __FILE__,
    line: UWord = __LINE__,
    col: UWord = __COLUMN__,
    fun: StaticString = __FUNCTION__
) -> Container<T> {

        let unique_key = "FUNC_\(fun)__LINE\(line)_COL\(col)__FILE_\(file)"

        let needs_init = !contains( Static_Containers.keys, unique_key )

        if needs_init {
            Static_Containers[unique_key] = Container<T>( memory:value )
        }

        return Static_Containers[unique_key]! as! Container<T>

}

Here's a couple tests:

func test_with_nsstring( str:NSString, init_only:Bool ) -> NSString {
    var stat_str = static_var( str )
    if !init_only {
        stat_str.memory = str
    }
    return stat_str.memory
}
test_with_nsstring( "this should get set", true )
test_with_nsstring( "this should be ignored", true ) // only repeated declaration
test_with_nsstring( "this should change the value", false )
test_with_nsstring( "as should this", false )

func test_with_int( i:Int, init_only:Bool ) -> Int {
    var stat_int = static_var( i )
    if !init_only {
        stat_int.memory = i
    }
    return stat_int.memory
}
test_with_int( 0, true )
test_with_int( 1, true ) // only repeated declaration
test_with_int( 2, false )
test_with_int( 3, false )

func test_with_optstr( optstr:String?, init_only:Bool ) -> String? {
    var stat_optstr = static_var( optstr )
    if !init_only {
        stat_optstr.memory = optstr
    }
    return stat_optstr.memory
}
test_with_optstr( nil, true )
test_with_optstr( "this should be ignored", true ) // only repeated declaration
test_with_optstr( "this should change the value", false )
test_with_optstr( "as should this", false )

When I test this code in a Playground, it seems to behave correctly. I'd just like a less nutty, and less brittle, way to accomplish this.

The only work around for this that I've come up with thus far is something that looks like this:

func foo() -> Int {
    struct Holder {
        static var timesCalled = 0
    }
    return ++Holder.timesCalled;
}

It's pretty clunky in my opinion, and I'm not sure why Apple doesn't just allow straight static function variables, but this seems quite a bit cleaner than your approach.


Markov Chain generator I'm not sure if a Markov Chain generator is the proper term for this, really all it does is create the chain, it doesn't generate any text from it. The code is below, and I'd appreciate any feedback: bad practices, code smells, poor naming, anything at all really.

module Main where
import Data.List(find, delete)

type Chain = [Node]
data Node = Node {nPrefix :: String, nSuffixes :: [String]} deriving (Show, Eq, Read)

generateChain :: Int -> [String] -> Chain
generateChain n = foldl (learnSample n) []

learnSample :: Int -> Chain -> String -> Chain
learnSample n chain sample = foldl learnGroup chain groups
    where groups = (groupings (n+1) . words) sample

learnGroup :: Chain -> [String] -> Chain
learnGroup chain group =
    case find (\node -> nPrefix node == prefix) chain of
        Just node -> delete node chain ++ [Node prefix (nSuffixes node ++ [suffix])]
        Nothing   -> chain ++ [Node prefix [suffix]]
    where
        prefix = unwords $ init group
        suffix = last group

groupings :: Int -> [a] -> [[a]]
groupings n list
    | length list >= n = take n list : groupings n (drop 1 list)
    | otherwise        = []

main :: IO ()
main = print $ generateChain 1 ["a b a b a c a d"]

Looks good, the only thing I'd suggest is that since you don't really
rely on having strings as nodes, you can also generalise your
implementation to arbitrary chains, i.e. Chain Char for this particular
main function.  You'll need to change to a GADT (I think) but
otherwise the code would just need a few minor additional changes.


Using hashing to create a PRNG with seed So I wanted to code a simple PRNG for generating terrain in a game. It has to have the following properties:

Computationally cheap
Generate semi random numbers between 0 and an positive integer.
I need to be able to go back and teleport anywhere in the world so random.Next doesn't cut it.
I basically want a noise function. One output for each input with repeatable results.
No seed will be used. If I need another series I'll just shift x value by a couple of millions

These are some outputs produced by my program:

1 as input (x) gives 2 as a pseudo random number

2 as input (x) gives 2 as a pseudo random number

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

using System.Threading;
using System.Security.Cryptography;

namespace Hashing_Functions
{
    class Program
    {
        static void Main(string[] args)
        {
            int i0 = 0;

            while (true)
            {
                i0++;
                Console.WriteLine(RandomHashFunction(i0, 10));
                //returns random number between 0 and max based on input
            }
        }

        static int RandomHashFunction(int input, int max)
        {
            var SHA1 = new SHA1CryptoServiceProvider();
            StringBuilder sb = new StringBuilder();
            string hash2string;
            double output;
            byte[] hash;
            SHA1.ComputeHash(ASCIIEncoding.ASCII.GetBytes(input.ToString()));
            hash = SHA1.Hash;
            sb.Clear();

            foreach (byte b in hash)
            {
                sb.Append(b.ToString());
            }
            hash2string = sb.ToString();
            output = hash2string.GetHashCode();
            output = Math.IEEERemainder(output, max);
            output = Math.Abs(output);

            return Convert.ToInt16(output);
        }
    }
}

Clean up

I don't know why it had to be so long and to have so many temporary variables. So I shortened it to make sense out of it. Here's what I got.

static int RandomHashFunction(int input, int max)
{
    using (var sha1 = new SHA1CryptoServiceProvider()) // Disposing things is a good habit
    {
        var ascii = ASCIIEncoding.ASCII;
        var hashAsString = ascii.GetChars(
            sha1.ComputeHash(
                ascii.GetBytes(input.ToString())
            )
        );

        return Convert.ToInt32(
            Math.Abs(
                Math.IEEERemainder(hashAsString.GetHashCode(), max)
            )
        );
    }
}

WAT?

It is still not obvious to me, what are you attempting to achieve. Could you provide:

a few samples of input and output.
declare a variable that you think works as a seed.
a reference to a particular algorithm you're trying to implement or at least are inspired by.
OR at least a mathematical formula that you think will give you the desired result.

To me, there are a lot of unnecessary steps and manipulations.

Convert.ToInt16 was used to return and int (which is Int32) -- this was losing some meaning units of the number...
Math.Abs. Why bother with it if the Math.IEEERemainder already gives you a number.
Relying on String::GetHashCode may also be a bad thing since

  The behavior of GetHashCode is dependent on its implementation, which might change from one version of the common language runtime to another: MSDN

All these things seem very excessive and result in a messy code IMO.

Update 1

Updated the code to use symmetric GetChars and GetBytes based on the same encoding.

Update 2

Frankly, I am not finding the updated question very explanatory. The signature of the original function does not declare meaningful parameters. So I had to guess what their meaning is.

I also don't understand why a regular Random.Next*() won't work for you. It's trivial!

static int RandomHashFunction1(int seed = 1, int max)
{
    var randomDouble = new Random(seed).NextDouble();
    return (int)Math.Round(randomDouble * max);
}

If you really want to stay away from System.Random, you can use SHA1CryptoServiceProvider as you do in the original code, but I don't see why it is necessary.

static int RandomHashFunction2(int seed = 1)
{
    using (var sha1 = new SHA1CryptoServiceProvider())
        return BitConverter.ToInt32(sha1.ComputeHash(BitConverter.GetBytes(seed)), startIndex: 0);
}

Lastly, I don't see why do you need to use any string representation at all and go through unnecessary steps with Math.* which make your function computationally heavier.

Please consider accepting the answer as it does the job, OR carefully describe the acceptance criteria. Your problem statement is unclear!

Update 3

Okay, you updated the question with the important info, which I quote here

  need to generate terrain so say we have a line that consists of dots with an X and a Y value. The X values start at 0 and can go to positive and negative infinity depending on the player position. Depending on the X I need a random Y. System.Random gives a random number and then another random number based on the previous one. If I would only move to the right I could keep on generating terrain based on the previous dot using random.Next () But if I wanted to go back I would need the previous number with you can't do with Random.Next().

This is the new version of the function that should meet your more precise problem definition.

static int ConsistentRandom(
    int maxRandomInt,
    int seedInGame = 1,
    int specificLocationInGame = 0
)
{
    var actualSeedForRandom = seedInGame + specificLocationInGame;
    var randomDouble = new Random(actualSeedForRandom).NextDouble();
    return (int)Math.Round(randomDouble * maxRandomInt);
}

Notice that var actualSeedForRandom = seedInGame + specificLocationInGame; may be implemented in many various ways, I just used the simplest formula that came to my mind.

Here's a test console app:

public static void Main(string[] args)
{
    for (var iteration = 1; iteration <= 2; iteration++)
    {
        Console.WriteLine("TEST ITERATION " + iteration);
        for (var seed = 0; seed < 2; seed++)
        {
            Console.WriteLine("SEED " + seed);
            for (var location = 0; location < 5; location++)
            {
                Console.Write(ConsistentRandom(10, seed, location));
                if (location != 4) Console.Write(" - ");
            }
            Console.WriteLine();
        }
        Console.WriteLine();
    }

    Console.ReadKey();
}

And here's what it produces:

As you can see, as long as you know the seedInGame, you control the sequence of randoms. And if you know both the seedInGame and specificLocationInGame, you can easily get the specific value of random for this combination.

In other words, your Game should pick a new seedInGame on every new map it's trying to generate. The value of specificLocationInGame will let you "teleport" anywhere within this particular map.

Update 4 - To Address Concerns About Randomness

Here's the exact code I used to generate some consistent but random number sequence. The resulting sequence can be downloaded from here.

static void Main(string[] args)
{
    var path = @"C:\temp\generatedRandom.txt";
    if (File.Exists(path))
        File.Delete(path);
    using (var fileStream = File.Create(path))
    using (var textWriter = new BinaryWriter(fileStream, Encoding.ASCII))
    {
        var seed = 42;
        for (var location = 0; location < 3 * 3000 * 3000; location++)
        {
            var randomByte = ConsistentRandom(Int64.MaxValue, seed, location);
            textWriter.Write(randomByte);
            Console.WriteLine(randomByte);
        }
    }
}

static Int64 ConsistentRandom(
    Int64 maxRandomInt,
    int seedInGame = 1,
    int specificLocationInGame = 0
)
{
    var actualSeedForRandom = seedInGame + specificLocationInGame;
    var randomDouble = new Random(actualSeedForRandom).NextDouble();
    return (Int64)Math.Round(randomDouble * maxRandomInt);
}

I then used ent - pseudorandom number sequence test for checking how well the solution performs in terms of randomness.

I think, only the author can judge how far or how close this result is located from the desired one.

Update 5 - Visualizing the Solution (or How I Screwed Up)

Okay, I built a little tool to visualize the generated "random" sequence via the proposed algorithm (since I didn't find one online). The following picture demonstrates "little regularities" in the sequence... Well, the "random" sequence has very distinguishable regularities, so the proposed approach does not guarantee good results. I think, it's valid to suppose that approach in fact guarantees bad results. :(

@Wagacca, you were right in our conversation under my answer!


Automatically indent a whole block of text into str.format I wondered if there was any better way of doing so.

I had a hard time figuring it out (it seemed nobody else already tried to do so).

import re
import string

class BetterFormat(string.Formatter):
    """ Introduce better formatting """

    def parse(self, format_string):
        """ Receive the string to be transformed and split its elements to facilitate the actual replacing """

        # Automatically counts how many tabs are before the {} then automatically adds the new rule (defined in format_field)
        return [(before, identifiant, str(len(re.search('\t*$', before).group(0))) + '\t' + (param if param is not None else ''), modif) for before, identifiant, param, modif in super().parse(format_string)]

    def format_field(self, v, pattern):
        """ Receive the string to be transformed and the pattern according which it is supposed to be modified """

        # Hacky way to remove a dynamic sequence of characters ([0-9]+\t) and returning it

        sharedData = {'numberOfTabs': 0}

        def extractTabs(pattern):
            sharedData['numberOfTabs'] = int(pattern.group(0)[:-1]) # Save the data that will be erased
            return "" # This will erase it

        pattern = re.sub('[0-9]+\t', extractTabs, pattern)

        if sharedData['numberOfTabs']: # If there are tabs to be added
            v = (sharedData['numberOfTabs'] * '\t').join(v.splitlines(True))

        return super().format_field(v, pattern)

css = """
/* Some multi-lines CSS
All automatically indented */
"""

print(BetterFormat.format("""
<html>
    <head>
        <style type="text/css">
            {css}
        </style>
    </head>
    <body>
        <!-- Whatever -->
    </body>
</html>""",
    css=css
 ))

 # Output
 <html>
    <head>
        <style type="text/css">
            /* Some multi-line CSS
            All automatically indented */
        </style>
    </head>
    <body>
        <!-- Whatever -->
    </body>
</html>

Otherwise, previously I had to do this:

# […]

print(""" […]
            {css}
[…]""".format(css='\t\t\t'.join(css.splitlines(True)))
)

And just so you better understand, I first only implemented BetterFormat.format_field, then I was able to do that, but I was still required to count the number of tabulations myself:

# […]

print(""" […]
            {css:3\t}
[…]""".format(css=css)
)

Could textwrap do some of the job?

Particularly the dedent and indent function?  You will still need to count the number of tabs you want, but the rest will be done.


Bootstrap 3.0 Carousel gallery post WordPress I have working code to show in index.php and content-gallery.php of WordPress with Bootstrap 3.0 Carousel.

But, I know it's not clean code.  Could anyone help me clean/improve it?

This is the code we need to generate Bootstrap Carousel:

        ...

    ...

This is the content of my content-gallery-php:

 <?php function_indicators($post) ?>

 <?php function_slides($post) ?>

And here are 3 functions to my functions.php:

function wp_get_attachment( $attachment_id ) {

    $attachment = get_post( $attachment_id );
    return array(
        'alt' => get_post_meta( $attachment->ID, '_wp_attachment_image_alt', true ),
        'caption' => $attachment->post_excerpt,
        'description' => $attachment->post_content,
        'href' => get_permalink( $attachment->ID ),
        'src' => $attachment->guid,
        'title' => $attachment->post_title
    );
}

function functions_indicators() {
$special_gallery = get_post_gallery( $post, false );
$ids = explode( ",", $special_gallery['ids'] );
$html = '
';
foreach( $ids as $id ) {
    $link   = wp_get_attachment_url( $id );
    $class = ( $i == 0 ) ? 'active ' : '';
    $i++;
    $b=1;
    $html .= '

';
}
$html .= '
';
echo $html;

}

function function_slides() {
$special_gallery = get_post_gallery( $post, false );
$ids = explode( ",", $special_gallery['ids'] );
$html = '
';
foreach( $ids as $id ) {
    $link   = wp_get_attachment_url( $id );
    $attachment_meta = wp_get_attachment($id);
    $class = ( $i == 0 ) ? 'active ' : '';
    $i++;
    $html .= '
' . '

'.$attachment_meta['title'].'

'.$attachment_meta['description']. '

';
}
$html .= '
';
echo $html;}

IMO, clean wp code is hard, if not impossible to acchieve. I can tell you, however, that the first thing I'd remove from your functions is the echo $html; statements.

A function is a block of code that does something, and returns that data, to be used anywhere. Your functions don't return anything (apart from the implicit null), but echo a heck of a lot. That could be considered a side-effect. Functions that have side-effects are bad news. Why not replace the echo $html; with a simple return $html; for a start, then call the functions like so:

<?=function_slides();?>

<?= is short for <?php echo, and doesn't require the short_tags to be enabled anyway.
This is only the tip of the ice-berg, though. Your functions contain an awful lot of duplicate statements ($special_gallery = get_post_gallery( $post, false ), for one). Why not pass that as an argument, so you can keep it in scope for all calls, and avoid the overhead of repeating the same function calls?
What's more: the entire function bodies of both function_slides and function_indicators are remarkably similar, and could easily be combined into a single function:

function functions_slide_indicator($post) //specify argument, you forgot this
{
    $special_gallery = get_post_gallery( $post, false );
    $ids = explode( ",", $special_gallery['ids'] );
    $formats = array(
        'indicator' => '

',
        'slide'     => '

%s

%s

'
    );
    $html = array(
        'indicator' => '

',
        'slide'     => '
'
    );
    $link= wp_get_attachment_url($ids[0]);//get data for first id, not in loop
    $attachment_meta = wp_get_attachment($ids[0]);
    $html['slide'] .= sprintf(str_replace('<div class="item"', '<div class="active item"', $formats['slide']), $link, $attachment_meta['title'], $attachment_meta['description']);
    for($i = 1, $j = count($ids);$i<$j;$i++)
    {//start at 1, not 0, so no active class in here
        $link   = wp_get_attachment_url( $ids[$i] );
        $attachment_meta = wp_get_attachment($ids[$i]);
        $html['indicator'] .= sprintf($formats['indicator'], $i);// no need to increment/decrement it here, it'll be the value you need
        $html['slide'] .= sprintf($formats['slide'],$link, $attachment_meta['title'], $attachment_meta['description']);
    }
    $html['indicator'] .= '
';
    $html['slide'] .= '
';//close markup
    return $html;//return array
}

Call this like so:

<?php
    $content = functions_slide_indicator($post);
    echo $content['indicator'];//indicators
    echo $content['slide'];//carousel

Of course, this is only the start, instead of calling that function in the loop, I'd personally go for:

$links = array_map($ids, 'wp_get_attachment_url');

For example. Though this isn't the most performant way to go, at least it looks cleaner, though it could look cleaner still, if you used Objects.


Importing Excel file to a database just once How should I rewrite an if-return-else condition in the middle of this function?

def import_excel_to_database(xls_file_path)
  q_param = { report_type:report_type, item:product, exchange:exchange, product_exchange:row[0], date: row[2]} # timestamp_utc: row[2].strftime('%Q').to_i

  # if exsisted then return or continue the remaining
  if ImportHistory.where(q_param).exists?
    return false
  else
    ImportHistory.create(q_param)
  end

  xls = Roo::Excel.new(xls_file_path)
  xls.default_sheet = xls.sheets[0]
  columns = get_columns(xls.row(1))
  iterate_data_rows(xls_file_path, xls, columns)
end

Some notes:

AFAIK there is no consensus whether to write spaces or not after { and before }. But, definitely, always put an space after :.
This first line is too long. < 80/100 chars is more advisable. When writing hashes, it's easy: one line per pair. Much easier to read.
# if existed then return or continue the remaining: This comment is repeating the code, so it's not really conveying much.
Questions: Where does row come from? what does iterate_data_rows return?
Answering your question about the if, you have two options (you wrote the intermediate, which is not advisable):

One-line guard: return false if ImportHistory.where(q_param).exists?
Move the rest of the code within the else (I like this one).

I'd write:

def import_excel_to_database(xls_file_path)
  q_param = {
    report_type: report_type,
    item: product,
    exchange: exchange,
    product_exchange: row[0],
    date: row[2],
    # timestamp_utc: row[2].strftime('%Q').to_i,
  }

  if ImportHistory.where(q_param).exists?
    false
  else
    ImportHistory.create(q_param)
    xls = Roo::Excel.new(xls_file_path)
    xls.default_sheet = xls.sheets[0]
    columns = get_columns(xls.row(1))
    iterate_data_rows(xls_file_path, xls, columns)
  end
end


C++ password generator with WinAPI clipboard function I made a simple password generator with a copy-to-clipboard functionality implemented via WinAPI calls. Here is the code:

#include <iostream>
#include <string>
#include <random>
#include <algorithm>
#include <Windows.h>

void copytoclipboard(const std::string& s){
    HGLOBAL hMem = GlobalAlloc(GMEM_MOVEABLE, s.size() + 1);
    memcpy(GlobalLock(hMem), s.c_str(), s.size() + 1);
    GlobalUnlock(hMem);
    OpenClipboard(0);
    EmptyClipboard();
    SetClipboardData(CF_TEXT, hMem);
    CloseClipboard();
}
int main(){
    std::string chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789%^&$+-_!@#%?";
    std::string pw = "";
    const int pass_length = 12;
    const int from = 0;
    const int to = chars.size();
    std::random_device rd;
    std::mt19937 g(rd());
    std::uniform_int_distribution<int>  distr(from, to);
    auto rng = std::default_random_engine{};
    std::shuffle(chars.begin(), chars.end(), rng);
    for (int i = 0; i < pass_length; ++i){
        pw.push_back(chars[distr(g)]);
    }
    copytoclipboard(pw);
}

What can be improved and are there any pitfalls with the above approach such as the lack of GlobalFree(hMem);?

Out Of Bounds Problem

const int to = chars.size();
...
std::uniform_int_distribution<int>  distr(from, to);

uniform_int_distribution - Produces random integer values i, uniformly distributed on the closed interval [a, b]

So if chars were "ABC", size is 3, and the range of values returned would be [0,3]
 = 0,1,2 and 3. 3 is an invalid index. Therefore 'to' should be chars.size() - 1.

Error Checking

You should check the return values on system functions like OpenClipboard.

if (!OpenClipboard())
{
    auto lastError = GetLastError();
    // get error message, throw exception?
}

GlobalFree

From MSDN - If SetClipboardData succeeds, the system owns the object identified by the hMem parameter. The application may not write to or free the data once ownership has been transferred to the system

So it looks like a GlobalFree call is not needed. You should free it though if SetClipboardData (or any of the other system calls) fails.


Responsive Navigation The following responsive navigation on JSfiddle works well, however I am just wondering whether there is any way to improve what I have done. I am new at JQuery so all help appreciated: JSFiddle here

$(document).ready(function () {
    $(".menu").click(function () {
        $('#menu').animate({
            'left': '0px'
        });
    });
    $("#close").click(function () {
        $('#menu').animate({
            'left': '-100px'
        });
    });
    $(".menu").click(function () {
        $('#container').animate({
            'left': '100px'
        });
    });
    $("#close").click(function () {
        $('#container').animate({
            'left': '0px'
        });
    });
});

Instead of attaching multiple click handlers for the same element, you should just combine them into one function:

$(document).ready(function () {
    $(".menu").click(function () {
        $('#menu').animate({
            'left': '0px'
        });
        $('#container').animate({
            'left': '100px'
        });
    });
    $("#close").click(function () {
        $('#menu').animate({
            'left': '-100px'
        });
        $('#container').animate({
            'left': '0px'
        });
    });
});

Other than that, i don't see anything to optimize.


Database class design I'm writing a SQLite wrapper for C++. I have this class representing a database:

  class Database {
    sqlite3 *db;
  public:
    Database(const std::string& file_path, bool readonly = false) throw(SQLiteException);
    ~Database();

    std::vector<std::map<std::string, Value> > Query(const std::string& query) throw(SQLiteException);
    std::vector<std::map<std::string, Value> > Query(const std::string& query, const std::vector<Value>& bindings) throw(SQLiteException);
  };

Where Value is a container for an int, double, std::string or std::vector<char> (because the returned type isn't yet known at compile-time).

I'm concerned about the return value types of Database::Query. How could I simplify this?

Types have to be known at compile time. Unfortunately to be flexable you need to look up arbitory types at runtime for a DB. So either you need to use some form of template-meta programming or you can use a variant type. I would go for something like boost Any object or boost variant this will allow you to return arbitrary types.

I don't think you want to return the result from the DB after creating the Query. You want to return a query object that will allow you to bind things and execute the query at a latter stage. This is because in most DB systems creating the query is relatively expensive.

And when you do execute the query you want to use the concept of the cursor (iterator) to move over the result set rather than generating the full result in one go (as this could potentially be huge).

int main();
{
    Database   db("Bla", "Passowrd", true);
    Query      getName = db.query("SELECT Name,Address,Phone "
                                  "FROM Users Where Age=%d and Sex=%s");

    // Allow user to bind values to the query and execute.
    // As building the query can be expensive. So you want to re-use it.
    Result     age25   = getName.execute(bind() % 25 % "M"));

    // Re-use the Query object for the next query   (Note bind should allow multiple param)
    Result      age31   = getName.execute(bind() % 31 % "F"));

    // Rather than returning all the results from the query in one go
    // Have the result object provide an `input iterator` allowing you to retrieve the
    // the rows as required. That way you do not need to copy out huge objects all in
    // one go. You can scan over the input until you get the bit you want (potentially)
    // only copying selected parts out of the result.

    if (!age25) // convert object to boolean to test for OK
    {           // Like a std::stream object. This way if the query has
                // an error you can quickly an easily test for it.

        std::cout << age25.error() << "\n";
    }
    else
    {
        for(RowIter row = RowIter(age25); row != RowIter(); ++row)
        {                                 //     ^^^^^^^^^  like std::istream_iteator
                                          //                an empty object is end()

            std::cout << db_cast<std::string>(row->get("Name")) << " "
                      << db_cast<std::string>(row->get("Address")) << " "
                      << db_cast<int>(row->get("Phone")) << "\n";
                     //  ^^^^^^^^^^^^
                     //  Convert the object into the correct type
                     //  using a cast like syntax.

        }
    }
}

Also do not limit it to sqlite.
Make it so it is easily expandable to other DB types.

I would go with a prefix Notation on the DB name:

DataBase    db1("sqlite://FileName1", "OptionalPassword", flags);
DataBase    db2("mysql://db@host.bob.com", "Password", flags);
Database    db3("oracle://plop.foo.bar");

Internally you can then use the PIMPL design pattern to abstract the different types of DB. For now you just implement the sqlite version make the others generate an exception.


Is this Repository Design Pattern Valid and efficient? I am using Dapper ORM in my Data access Layer, but I think analysis of the code below doesn't depend upon any specific ORM.  What should be the single common place throughout the entire Data Access Layer to initialise the Connection String?

Base Abstract Class

abstract class Repository
    {
        protected static IDbConnection _db = new SqlConnection(ConfigurationManager.ConnectionStrings["GuideDB"].ConnectionString);

      //should I use Constructor here
    }

A Generic Repository Interface

public interface IRepository<T>
    {
        IEnumerable<T> GetAll();
        T GetItem(int id);
        T Create(T item);
        bool Update(T item);
        bool Delete(int id);
        void Reset();
    }

Concrete class

public class StateRepository :Repository, IRepository<State>
    {

        private static ICollection<State> _states = PopulateStates();// doing this valid ??
        private static object _lock = new object();

        #region Public Methods

        public IEnumerable<State> GetAll()
        {
            lock (_lock)
            {
                return _states.ToArray();
            }
        }

        public IEnumerable<State> GetAllStatesByCountryId(short id)
        {
            lock (_lock)
            {
                return _states.Where(s => s.CountryId == id).ToArray();
            }
        }

        public State GetItem(int id)
        {
            lock (_lock)
            {
                return _states.FirstOrDefault(s => s.Id == id);
            }
        }

        public State Create(State state)
        {
            lock (_lock)
            {
                if (state == null)
                {
                    throw new ArgumentNullException("state");
                }

                state.Id = (short)(_states.Any() ? _states.Max(s => s.Id) + 1 : 1);
                _states.Add(state);
                return state;
            }
        }

        public bool Update(State item)
        {
            lock (_lock)
            {
                var state = _states.FirstOrDefault(s => s.Id == item.Id);

                if (state != null)
                {
                    state.Id = item.Id;
                    state.Name = item.Name;
                    state.ShortInfo = item.ShortInfo;
                    state.CountryId = item.CountryId;

                    return true;
                }

                return false;
            }
        }

        public bool Delete(int id)
        {
            lock (_lock)
            {
                var state = _states.FirstOrDefault(s => s.Id == id);
                return _states.Remove(state);
            }
        }

        public void Reset()
        {
            lock (_lock)
            {
                _states = PopulateStates(); // doing this valid ??
            }
        }
        #endregion

        private static ICollection<State> PopulateStates()
        {
            return  _db.Query<State>("SELECT * FROM States").ToList(); //Dapper ORM Code
        }

       //How to Add a efficient Dispose method to this repository , is it really needed
    }

I think your abstract class should "implement" the IRepository interface, because otherwise you will always have to put both the interface and the base class in your inheritance list. Also, I think you should name your Repository -> SqlRepository, since all the code in the base class is coupled to Sql. As @Heslacher pointed out, you should inject your connection, because keeping the static connection might cause problems. What would happen if one of my Repository implementation would dispose the connection? All your repositories would be down.

abstract class SqlRepository<T> : IRepository<T>
{
    protected IDbConnection Connection { get; set; }

    protected SqlRepository<T>(string connectionString)
    {
        Connection = new SqlConnection(connectionString);
    }

    abstract IEnumerable<T> GetAll();
    abstract T GetItem(int id);
    abstract T Create(T item);
    abstract bool Update(T item);
    abstract bool Delete(int id);
    abstract void Reset();
}

If your application runs on one instance, your repository would be okay. But since it puts all database entities in cache (your collection) and never returns to the database unless you specify it, you have a problem. Example : Client A (On an X machine) loads the database in cache, Client B (on a Y machine) does the same then adds a State to the repository. Client A calls GetAll, what should be the expected output? Client A wouldn't have the new State. And if Client B calls Reset the new State was never persisted, so it is lost.

I don't think you should use the ICollection<State>, each of your call should go to the database, so you are sure you have the latest version of your data. If you remove it, I don't think you need to handle thread safety anymore.


Morse Code Converter in C I'm learning C so I thought to apply it to miniproject on reddit. The smorse function takes in a string and spits out some morse code. It works if you supply the function with only lowercase letters. I feel awkward making a first for loop in my smorse function to calculate the size of buffer I need to malloc and then making a second for loop to add the characters.

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

char **master = NULL;

char* smorse(char *str) {
    int i;
    char *ele;
    int end = strlen(str);
    char *output = "";
    int totalSize = 0;

    for (i = 0; i < end; i++) {
        ele = master[(*str) - 97];
        totalSize += sizeof(ele) * sizeof(char);
        str++;
    }

    output = malloc(totalSize);
    str -= end;

    for (i = 0; i < end; i++) {
        ele = master[(*str) - 97];
        strcat(output, ele);
        str++;
    }

    return output;
}

int main(int argc, char *argv[]) {
    int i;
    char key[] = ".- -... -.-. -.. . ..-. --. .... .. .--- -.- .-.. -- -. --- .--. --.- .-. ... - ..- ...- .-- -..- -.-- --..";
    master = (char **)malloc(26 * sizeof(char *));
    char *ele = strtok(key, " ");

    for (i = 0; i < 26; i++) {
        if (i != 0)
            ele = strtok(NULL, " ");
        master[i] = (char *)malloc(sizeof(ele) * sizeof(char));
        master[i] = ele;
    }

    printf("%s\n", smorse("sos"));
    printf("%s\n", smorse("daily"));
    printf("%s\n", smorse("programmer"));
    printf("%s\n", smorse("bits"));
    printf("%s\n", smorse("three"));

    free(master);
    return 0;
}

Memory leak
Consider this (I removed the pointless cast and identity multiplication):

    master[i] = malloc(sizeof ele);
    master[i] = ele;

Here, we allocate memory (if malloc() succeeds), but then immediately overwrite our one and only pointer to it, so we're unable to ever free it.  The first assignment should just be removed, leaving only master[i] = ele.
Another memory leak:

printf("%s\n", smorse("sos"));

The smorse() function returns a pointer to allocated memory, which we need to release when we've finished using it:
const char *m = smorse("sos");
puts(m);
free(m);

(The puts() call is exactly equivalent to using printf("%s\n", m), but more readable and possible more efficient.)
Character coding assumptions
The magic value 97 in a couple of places suggests that we're assuming that characters are encoded as ASCII values.  That's not a portable assumption: C permits a wide range of encodings.  Notably, EBCDIC systems do not have letters in the same positions as ASCII (or even as a contiguous block, so replacing with 'a' doesn't solve the problem).
Also, there's no checking that the input to the function contains only characters that we can handle.  If we pass anything outside of the expected range, then we index master outside its bounds, which is Undefined Behaviour in C.  That means that the program may do anything - if we're lucky, it will simply crash, helping us to identify the problem, but there's no guarantee of that.
We ought to be converting more than just letters - digits have a standard Morse representation, as do certain other useful symbols.
Value conversions
It seems that this has been compiled with a very low level of warnings enabled.  Building with my usual compiler flags results in over a dozen warnings, most about suspect conversions.
To pick a couple of examples:

int end = strlen(str);

strlen() returns a size_t, which is of different signedness to int, and very likely to be bigger, so there's a risk of undefined behaviour here.

char *output = "";

It's dangerous to point a modifiable pointer at constant data (a string literal is of type char const*).

printf("%s\n", smorse("sos"));

Passing a string literal loses constness here - smorse() ought to accept pointer to const instead.


Handling post-registration action This code is Express NodeJS code that handles post registration .post action where the user must select a custom username. The username is validated, and if valid, the database is updated. If not, the user is redirected to the page with an error message so he/she can try again.

I would like to know if you would refactor this code with the extract method, that is extracting some functionality into their own extracted method to improve fragmentation of this code and readability.

// This route is only used for newly registered facebook user so they can supply a custom userName.
users_router.route( "/userName" )

    // Process the customUserName recording.
    .post( function( req, res ) {
        // Receive data from the customUserName form field.
        let form = new forms.IncomingForm()
        form.parse( req, function( err, fields, files ) {

            let userModel = new User( fields ).userModel() // Instantiate current user
            let validationResults = new Validation( userModel, fields ).validationResults()

            // There is no error: Update the customUserName field in the database.
            if ( validationResults === g.error.no ) {
                let properties: string = req.session[ "userModel" ].id.content // Session holds copy of model
                let customUserNameField = fields[ "custom_user_name" ]
                let customUserNameData = { "facebookId": properties, "customUserName": customUserNameField }

                customUserNameDbUpdate( customUserNameData, function() {
                    console.log( "Custom user name was added" )
                    res.end( "ok" ) // TODO: redirect
                })
            }

            // There are errors: redirect back to user name selection page with error message.
            if ( validationResults !== g.error.no ) {
                let errorMessage = validationResults[ "custom_user_name" ].errorMessage

                res.render( "apps/users/views/chooseUsername.handlebars", { // TODO: should not redirect to template
                    error: errorMessage
                })
            }
        })
    })

It's not a good practice to put everything into the request parser. You can put there your formParser, but let an UserController.* handle the rest.

Think about your userActions (UserController.updateUsername) in a more abstract way, for example if you decide to setup an admin backend, where you have some other routes, you would like to reuse your code..

If you consider, to use a seed script, it could directly talk to the UserController, or modify values from a CommandLineTool.

The best scenario would be, if your routes just points to the controllers, so you could define a bunch in a single file, to see all your endpoints, and where they are pointing too.

One more hint: use a middleware to parse the form data.


Functional Scrabble Word Score Homework:  Write a function to score a scrabble word.
values is a global dict formatted as values = {'a': 1, 'b': 3,...}
The docstring is supplied as part of the course, thus I'm only looking for feedback on the actual program logic.

I could have written a bunch of variables, iterated over the string word, looking up points from values, etc., but I've been reading about functional programming lately and this is my first attempt at something "non-procedural".

I feel like this code (while working) looks terrible, isn't easily understood, and has more than one code smell.  For example, I understand that breadth is generally preferred over depth (at least in python, right?), uses the much-hated python ternary, and is 89 chars wide (plus probably more significant smells I don't even recognize).

I'm trying to:

not declare pointless or temporary variables
be concise
avoid traditional loops
avoid changing variables' values

I considered rewriting with a nested recursive function, would this make more sense?
Also, if I'm using the term "functional" in the wrong way, or completely missing the point, please correct.

def getWordScore(word, n):
    """
    Returns the score for a word. Assumes the word is a valid word.

    The score for a word is the sum of the points for letters in the
    word, multiplied by the length of the word, PLUS 50 points if all n
    letters are used on the first turn.

    word: string (lowercase letters)
    n: integer (hand size required for additional points)
    returns: int >= 0
    """
    return len(word) * sum(map(lambda x: values[x], word)) + (50 if len(word)==n else 0)

It looks pretty good to me.

Generally, generator expressions and list comprehensions are preferred to map() in Python.  I'd write sum(values[c] for c in word).  Using c for "character" would be preferable to x, which has a connotation of being a floating-point number.

If you are practicing functional programming, then there is no sense in including "get" in the name, as every function should be a deterministic calculation based on its parameters.  By PEP 8, word_score would be a more appropriate name for the function.  Also, I suggest defaulting n=7, as that is the size of a full Scrabble hand.


Extracting Linux configuration information using Bash and Perl For few days I'm thinking about making my code faster and using less CPU. This code is a statusbar that runs in a loop. I'm asking because there's a lot of perl and I'm wondering if it could be done better.

date=$( date +"%F %R" )

battery=$(acpi -b | perl -n -e '/(\S+),\s+([0-9]+\%),\s(\S*)\s/ && print $1," ",$2," [ ",$3," ]"')

RAM=$(free -m | perl -n -e '/Mem\:\s*([0-9]+)\s*([0-9]+).*/ && print "RAM: (",$2," / ",$1,")MB"')

VolumeLv=$(amixer get Master -c 1 | perl -n -e '/\[(.*)\%\]/ && print $1')

backLightLv=$(xbacklight | perl -n -e '/^([0-9]*)/ && print $1')
statusBacklight=$?
backLightBar=""
VolumeBar=""
Sign=":"
NoSign="."
for i in $(seq 1 2 100)
do
    if [ $i -gt $VolumeLv ]
    then
        VolumeBar=$(echo "$VolumeBar$NoSign")
    else
        VolumeBar=$(echo "$VolumeBar$Sign")
    fi
    if [ $statusBacklight -eq 0 ] && [ $i -gt $backLightLv ]
    then
        backLightBar=$(echo "$backLightBar$NoSign")
    else
        backLightBar=$(echo "$backLightBar$Sign")
    fi
done

ETHcards=""
for interface in $(iwconfig 2> /dev/null | perl -n -e '/^([a-zA-Z0-9]*)\s.*ESSID\:.*$/ && print $1,"\n"')
do
    IPANDMASK=$(ifconfig $interface | perl -n -e '/.*inet\ addr:([0-9\.]+).*Mask:([0-9\.]*)\s/ && print $1, "/", $2')
    MAC=$(ifconfig $interface | perl -n -e '/HWaddr\s+([a-f0-9\:]+)\s*/ && print $1')

    AP=$(iwconfig $interface 2>/dev/null | perl -n -e '/ESSID\:(\S*)/ && print $1')
    MODE=$(iwconfig $interface 2>/dev/null | perl -n -e '/Mode:(\S+)\s/ && print $1')

    ETHcards=$(echo "$ETHcards [ $interface ][ $MAC ][ $AP ][ $IPANDMASK ][ $MODE ]  //  ")
done
xsetroot -name "$date :: $battery;Volume: [$VolumeBar]$Mute $VolumeLv%    BackLight: [$backLightBar] $backLightLv%  //  $RAM  //  $ETHcards"

[...] there's a lot of perl and I'm wondering if it could be done better.

When you need to process the output of multiple commands,
as in your example acpi, free, amixer, xbacklight,
it's acceptable to call a perl for each.

The part needs improvement the most is the loop over the network interfaces,
where you call iwconfig $interface and ifconfig $interface repeatedly to extract different part of the output into different variables.
It would be better to rewrite this to call ifconfig and iwconfig just once per interface.

At a glance, it seems to me that everything you do with Perl, you could also do with awk, which might be slightly lighter.

If performance is extremely important,
you could parse the output of all the commands with a single Perl or awk process, by carefully adding some marker lines in between commands, something like this:

marker='==== section ===='

{
    acpi -b
    echo $marker

    free -m
    echo $marker

    amixer get Master -c 1
    echo $marker

    xbacklight
    echo $marker

    iwconfig
    echo $marker

    ifconfig
} | perl -ne '...'

This way the number of processes spawned will be drastically reduced,
and the performance difference will be noticeable.


Split function while reading multiple lines The input begins with the number \$t\$ of test cases in a single line
  (\$t \le 10\$). In each of the next t lines there are two or more
  numbers \$m\$ and \$n\$ (\$1 \le m \le n \le 1000000000\$, \$n-m \le 100000\$) separated by a space.

  Print each number in a separate line which can be used further for
  summation.

  Input

2

50 100

100 50 105

  Output

50

100

100

50

105

This is the code that I've written that is giving me output:

import java.util.Scanner;
import java.util.StringTokenizer;

public class Generation {

    public static void main(String[] str) {

        Scanner keyboard = new Scanner(System.in);
        int inputSize;
        do {
            System.out.println("Enter the value of T Size");
            inputSize = keyboard.nextInt();
            keyboard.nextLine();
            if (inputSize < 2 || inputSize > 10) {
                System.out.println("Not a Valid Input Size");
            }
        } while (inputSize < 2 || inputSize > 10);

        String[] inputValue = new String[inputSize];
        int tokenCount = 0;
        for (int i = 0; i < inputSize; i++) {
            System.out.println("Enter the inputs");
            inputValue[i] = keyboard.nextLine();
            StringTokenizer strToken = new StringTokenizer(inputValue[i], " ");
            tokenCount += strToken.countTokens();
        }
        keyboard.close();

        //suppose this is 2nd part
        int[] splitedString = new int[tokenCount];
        int tempTokenCount = 0;
        for (int i = 0; i < inputSize; i++) {
            String[] tempSplitArray = inputValue[i].split(" ");
            for (int j = 0; j < tempSplitArray.length; j++) {
                splitedString[tempTokenCount] = Integer
                        .parseInt(tempSplitArray[j]);
                tempTokenCount++;
            }

        }
        /*for (String s : inputValue) {
            System.out.println(s);
        }*/
        for (Integer s : splitedString) {
            System.out.println(s);
        }

    }

}

How can I optimize the 2nd part where I have to use two for loop which result in \$O(n^2)\$ time complexity? What is the workaround for such situations?

try-with-resources

If you're on Java 7, you should use try-with-resource on your Scanner:

try (Scanner scanner = new Scanner(System.in)) {
    // your code here
}

This takes care of closing your Scanner at the end, so you do not have do perform an explicit close().

First input

I am not too sure why are you ensuring inputSize cannot be less than 2, since the problem statement only mentioned <= 10. I'll take that to mean an implicit assumption for non-negative values, so yeah even 0 is possibly a valid input. There is also an easier way of getting your first input than performing your validation twice, and while you're at that you should consider putting it in a method:

private static int getCount(Scanner scanner) {
    System.out.println("Enter the value of T Size [0, 10]:");
    int value = scanner.nextInt();
    while (value < 0 || value > 10) {
        System.out.println("Value is not within [0, 10], try again.");
        value = scanner.nextInt();
    }
    scanner.nextLine(); // gotcha
    return value;
}

The gotcha, as you probably have already figured, is that you need to 'consume' the rest of the line first before moving to other inputs. Therefore, I will also suggesting reading as nextLine() instead and then attempt to convert to an int via Integer.parseInt(). You can then catch the NumberFormatException thrown as an invalid input, and re-prompt until a valid integer is entered.

Validating your additional inputs

Note: I'm not sure if integers are implied for numbers, so I'll just go along with what you have shown...

First, I think you are missing the validation on your integers-to-be:

  \$m\$ and \$n\$ (\$1 \le m \le n \le 1000000000\$, \$n-m \le 100000\$)

Second, you should also consider converting and saving each of the integers you encounter in a single input line immediately, instead of storing into a String[] array and then performing the conversion afterwards. This avoids the additional looping which you identified in your question. :)

Come and think of it, putting all these together does sound like a good usage for Java 8 streams...

Java 8 solution ahead

So to conclude, the steps we need are:

Get number of lines (0 <= x <= 10)

stream.limit(getCount(scanner))

We only read in this number of lines, x = getCount(scanner).
Read in a line for x times

Stream.generate(scanner::nextLine)

This creates a new stream where each element is read from the scanner, i.e. System.in, i.e. the user input. This is accessed via a method reference.
On each line, split by a ' ' character

stream.flatMap(Pattern.compile(" ")::splitAsStream)

Pattern.splitAsStream(CharSequence) is a convenient way of tokenizing a line as a stream, which in turn allows us to perform a flatMap() by replacing each element of the source stream with the contents of the resulting stream.
Convert each token as an integer within a certain range

stream.map(Generation::convert)

The sample implementation shown below for convert(String) does not perform any validation. Actually, as a result of the flatMap() operation in the previous step, it will be quite tricky to make sure that \$1 \le m \le n \le 1000000000\$, so I'll leave this as an exercise to the reader... (hint: map each String input as a List to facilitate the validation first).
Collect all results into a Collection for further usage

stream.filter(Objects::nonNull).collect(Collectors.toList())

Since convert(String) may give us null values for invalid integers, we need to exclude them first. Collecting as toList() is just for illustration. If you want to sum the results together immediately, for example, you can change that to .mapToInt(Integer::intValue).sum().

public class Generation { // you probably need a better name too
    public static void main(String[] args) {
        try (Scanner scanner = new Scanner(System.in)) {
            List<Integer> result = Stream.generate(scanner::nextLine)
                    .limit(getCount(scanner))
                    .flatMap(Pattern.compile(" ")::splitAsStream)
                    .map(Generation::convert)
                    .filter(Objects::nonNull)
                    .collect(Collectors.toList());
            result.forEach(System.out::println);
        }
    }

    private static int getCount(Scanner scanner) {
        // see implementation above
    }

    private static Integer convert(String value) {
        try {
            return Integer.valueOf(value);
        } catch (NumberFormatException e) {
            return null;
        }
    }
}


"Even Tree" Python implementation Problem Statement

  You are given a tree (a simple connected graph with no cycles). You have to remove as many edges from the tree as possible to obtain a forest with the condition that : Each connected component of the forest should contain an even number of vertices.

  To accomplish this, you will remove some edges from the tree. Find out the number of removed edges.

Input Format

  The first line of input contains two integers N and M. N is the number of vertices and M is the number of edges.
  The next M lines contain two integers ui and vi which specifies an edge of the tree. (1-based index)

Output Format

  Print the answer, a single integer.

Constraints

  \$2 \le N \le 100\$

Example input

10 9
2 1
3 1
4 3
5 2
6 1
7 2
8 6
9 8
10 8

Example output

  2

Note: The tree in the input will be such that it can always be decomposed into components containing even number of nodes.

Solution

def parents(ar, child):
    """ Enumerates on the list of all the parents of a child."""
    parent = ar[child]
    while child != parent:
        yield parent
        child = parent
        parent = ar[child]

def update_child_count(node_parent, node_count, child):
    for parent in parents(node_parent, child):
        node_count[parent] += 1

if __name__ == '__main__':

    N, E = map(int, raw_input().split())
    node_count = [None]*N
    node_parent = [None]*N
    root = 0
    res = 0
    for child, parent in enumerate(node_parent):
        node_parent[child] = child
        node_count[child] = 1

    for node in xrange(E):
        child, parent = map(int, raw_input().split())
        node_parent[child-1] = parent-1
        update_child_count(node_parent, node_count, child-1)

    for child, parent in enumerate(node_count):
        if node_count[child] % 2 == 0 and child != root:
            res += 1

    print res

The solution is working fine, but I am more interested in a better/more efficient approach.

Parenting

A node in a tree either has a parent or does not have a parent. It is never its own parent. You default your values to:

node_parent[child] = child

But it would be cleaner to keep them as None. That way, your iteration on parents() would've just been:

def parents(ar, idx):
    while ar[idx] is not None:
        yield ar[idx]
        idx = ar[idx]

Children

You use child and parent in a lot of places where neither makes sense. For example:

for child, parent in enumerate(node_count):

There is no child/parent relationship in that loop. That's confusing. Furthermore, you don't even use the parent part since what you're doing is just iterating over the nodes:

for node in xrange(N):
    if node_count[node] % 2 = 0 and node_parent[node] is not None:
        res += 1

But, more generally...

Global Variables and Data Storage

This solution half-relies on global variables (update_child_count) and half on passing around arguments (parents). It also involves keeping two separate arrays (node_parent and node_count) when there really should be just the one array of both items. It'd be better to better to group these into a class: Node. A Node has two things: a size and a parent:

class Node(object):
    def __init__(self):
        self.parent = None
        self.size = 1 #itself

And it has a way to set its parent - copying the loop I indicated above:

    def add_parent(self, p):
        self.parent = p
        while p is not None:
            p.size += 1
            p = p.parent

This simplifies your algorithm drastically. Now instead of keeping two lists, we just have to keep one:

nodes = [Node() for _ in xrange(N)]

And when we input the child and parent†, we just assign as appropriate:

nodes[child-1].add_parent(nodes[parent-1])

This is more direct than having two functions to do the updating. Then, all we need to do is find all the non-root nodes that have an even size, as you did before. Full solution using your same algo:

class Node(object):
    def __init__(self):
        self.parent = None
        self.size = 1

    def add_parent(self, p):
        self.parent = p
        while p is not None:
            p.size += 1
            p = p.parent

if __name__ == '__main__':
    N, E = map(int, raw_input().split())

    nodes = [Node() for _ in xrange(N)]
    for _ in xrange(E):
        child, parent = map(int, raw_input().split())
        nodes[child-1].add_parent(nodes[parent-1])

    print sum(1 for node in nodes
              if node.size % 2 == 0 and node.parent is not None)

† I'm not convinced that you can necessarily be sure that the child goes first in the inputs. It's not stipulated in the problem and this approach does require that to be true.


Find tags in text for replacement, my brute force solution I have a text such as

string text = "Hi [img-2] tst[img-3] test ttteessstt grawr[img-1-800]tet";

I want to find img tags in the text and get their id (second value) and possibly the third value if it exists.

string text = "Hi [img-2] tst[img-3] test ttteessstt grawr[img-1-800]tet";

var th = new TagHandler(null);
var res = th.FindImgTags(text);

foreach(var tag in res)
{
    Debug.WriteLine(string.Join(", ", tag.Tag, tag.Id, tag.Width));
}

The above code outputs this:

[img-2], 2, 0
[img-3], 3, 0
[img-1-800], 1, 800

This is my "find tags code". Its crude but gets the job done...
Any suggestions on how to make this less verbose / brute

public List<ImgReplaceItem> FindImgTags(string data)
{
    var result = new List<ImgReplaceItem>();

    var pieces = data.Split(new string[] { "[img-" }, StringSplitOptions.RemoveEmptyEntries);
    foreach (var item in pieces)
    {
        if (item.Contains(']'))
        {
            var backend = item.Split(new char[] { ']' }, StringSplitOptions.RemoveEmptyEntries);
            var dataInside = backend[0];

            var r = new ImgReplaceItem();
            r.Tag = "[img-" + dataInside + "]";
            var pid = -1;
            if (dataInside.Contains('-'))
            {
                var widthParts = dataInside.Split(new char[] { '-' }, StringSplitOptions.RemoveEmptyEntries);
                int.TryParse(widthParts[0], out pid);

                if (widthParts.Count() > 1)
                {
                    var width = -1;
                    int.TryParse(widthParts[1], out width);
                    r.Width = width;
                }
            }
            else
            {
                int.TryParse(dataInside, out pid);
            }
            r.Id = pid;
            result.Add(r);
        }
    }

    return result;
}

This is a lot of code for such a simple task... but how about some regex:

string text = "Hi [img-2] tst[img-3] test ttteessstt grawr[img-1-800]tet";

var ids = Regex.Matches(
     text,
     @"\[(?<tag>[a-z]+)-(?<id>\d+)(-(?<width>\d+))?\]",
     RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture
)
.Cast<Match>()
.Select(m => new ImgReplaceItem
{
    Tag = m.Groups["tag"].Value,
    Id = m.Groups["id"].Value,
    Width = m.Groups["width"].Success ? int.Parse(m.Groups["width"].Value) : 0
});

result:

Tag Id  Width

img 2   0
img 3   0
img 1   800

As to the code. I think it's well writen. There isn't actually much to correct if anything.

Just a small adjustment:

if (item.Contains(']'))

You could make it !item.Contains(']') and use the contintue to reduce nesting.