Variables for creating a 5x5 box I have to create a 5x5 box using the images, and my first solution was to create 25 variables and print them, which is very inefficient. How can I optimize this? This utilizes cocos2d-x.

auto cudrado0 = Sprite::create("images.png");
cudrado0->setPosition(Vec2(200, 200));
cudrado0->setScale(0.5);
this->addChild(cudrado0, 0);

auto cudrado1 = Sprite::create("images.png");
cudrado1->setPosition(Vec2(200, 275));
cudrado1->setScale(0.5);
this->addChild(cudrado1, 0);

 auto cudrado2 = Sprite::create("images.png");
cudrado2->setPosition(Vec2(200, 350));
cudrado2->setScale(0.5);
this->addChild(cudrado2, 0);

auto cudrado3 = Sprite::create("images.png");
cudrado3->setPosition(Vec2(200, 425));
cudrado3->setScale(0.5);
this->addChild(cudrado3, 0);

auto cudrado4 = Sprite::create("images.png");
cudrado4->setPosition(Vec2(200, 500));
cudrado4->setScale(0.5);
this->addChild(cudrado4, 0);

 auto cudrado5 = Sprite::create("images.png");
cudrado5->setPosition(Vec2(275, 200));
cudrado5->setScale(0.5);
this->addChild(cudrado5, 0);

auto cudrado6 = Sprite::create("images.png");
cudrado6->setPosition(Vec2(350, 200));
cudrado6->setScale(0.5);
this->addChild(cudrado6, 0);

 auto cudrado7 = Sprite::create("images.png");
cudrado7->setPosition(Vec2(425, 200));
cudrado7->setScale(0.5);
this->addChild(cudrado7, 0);

auto cudrado8 = Sprite::create("images.png");
cudrado8->setPosition(Vec2(500, 200));
cudrado8->setScale(0.5);
this->addChild(cudrado8, 0);

  auto cudrado9 = Sprite::create("images.png");
cudrado9->setPosition(Vec2(275, 275));
cudrado9->setScale(0.5);
this->addChild(cudrado9, 0);

 auto cudrado10 = Sprite::create("images.png");
cudrado10->setPosition(Vec2(350, 275));
cudrado10->setScale(0.5);
this->addChild(cudrado10, 0);

auto cudrado11 = Sprite::create("images.png");
cudrado11->setPosition(Vec2(425, 275));
cudrado11->setScale(0.5);
this->addChild(cudrado11, 0);

auto cudrado12 = Sprite::create("images.png");
cudrado12->setPosition(Vec2(500, 275));
cudrado12->setScale(0.5);
this->addChild(cudrado12, 0);

auto cudrado13 = Sprite::create("images.png");
cudrado13->setPosition(Vec2(275, 350));
cudrado13->setScale(0.5);
this->addChild(cudrado13, 0);

auto cudrado14 = Sprite::create("images.png");
cudrado14->setPosition(Vec2(350, 350));
cudrado14->setScale(0.5);
this->addChild(cudrado14, 0);

auto cudrado15 = Sprite::create("images.png");
cudrado15->setPosition(Vec2(425, 350));
cudrado15->setScale(0.5);
this->addChild(cudrado15, 0);

auto cudrado16 = Sprite::create("images.png");
cudrado16->setPosition(Vec2(500, 350));
cudrado16->setScale(0.5);
this->addChild(cudrado16, 0);

auto cudrado17 = Sprite::create("images.png");
cudrado17->setPosition(Vec2(275, 425));
cudrado17->setScale(0.5);
this->addChild(cudrado17, 0);

auto cudrado18 = Sprite::create("images.png");
cudrado18->setPosition(Vec2(350, 425));
cudrado18->setScale(0.5);
this->addChild(cudrado18, 0);

auto cudrado19 = Sprite::create("images.png");
cudrado19->setPosition(Vec2(425, 425));
cudrado19->setScale(0.5);
this->addChild(cudrado19, 0);

auto cudrado20 = Sprite::create("images.png");
cudrado20->setPosition(Vec2(500, 425));
cudrado20->setScale(0.5);
this->addChild(cudrado20, 0);

auto cudrado21 = Sprite::create("images.png");
cudrado21->setPosition(Vec2(275, 500));
cudrado21->setScale(0.5);
this->addChild(cudrado21, 0);

auto cudrado22 = Sprite::create("images.png");
cudrado22->setPosition(Vec2(350, 500));
cudrado22->setScale(0.5);
this->addChild(cudrado22, 0);

auto cudrado23 = Sprite::create("images.png");
cudrado23->setPosition(Vec2(425, 500));
cudrado23->setScale(0.5);
this->addChild(cudrado23, 0);

auto cudrado24 = Sprite::create("images.png");
cudrado24->setPosition(Vec2(500, 500));
cudrado24->setScale(0.5);
this->addChild(cudrado24, 0);

If you have to print a 5x5 box with the a different image or the same you could do something like this. It loops though all the positions that you need and assigns it to a new sprite created. It is more maintaineable, cleaner and easy to read than declaring 25 variables, and it has fewer lines of code.

// Construct the vector with the specified size.
Vec2 size(5, 5);
std::vector<Vec2> positions(size.x * size.y);

// Push back desired positions (you could use C++11 initializer lists or a loop).
// Positions needs to be declared in the same order as the sprites will be
// added with "this->addChild".
positions.push_back(position);
...

float scale(0.5);

// Loop trought the vector and assign values
for (int i = 0; i < size.x * size.y; i++) {
    auto sprite = Sprite::create("theimageyouneed.png");
    sprite->setPosition(positions[i]);
    sprite->setScale(scale);
    this->addChild(sprite, 0);
}


SASS grid system improvements I've got this own made grid system made in SASS:

@import "../mixins/cross-browser-elements/box";

$columns: 12 !default;
$gutter-width-px: 20px !default;

@function calculate-column-width($index) {
  @return percentage($index / $columns);
}

.row {
  overflow: hidden;
  max-width: 100%;

  .column {
    float: left;
    min-height: 1px;
    padding-left: ($gutter-width-px / 2);
    padding-right: ($gutter-width-px / 2);
    @include box-sizing(border-box);

    &:first-child {
      padding-left: 0;
    }

    &:last-child {
      padding-right: 0;
    }

    @for $index from 1 through $columns {
      &.size-#{$index} {
        width: calculate-column-width($index);
      }
    }
  }

}

It looks pretty good in my opinion, it works and has a working padding on each element.

The thing I worry about is when I want to make it responsive. Do I have to go in my stylesheet and give every column a width of 100% when it's below a certain screen size with the @media selector?

My HTML markup looks like this:

column-size-10

column-size-2

column-size-1

column-size-1

column-size-1

column-size-1

column-size-1

column-size-1

column-size-1

column-size-1

column-size-1

column-size-1

column-size-1

column-size-1

I'm unsure of your SCSS, because it doesn't seem to use media-queries, and isn't small-screen-first: but here are my HTML suggestions:

The columns would likely stack on small screens, and aren't really columns semantically in general. You can note that they are smaller pieces inside a container by calling them blocks or something, and then use floats in combination with @media queries. Giving them static classes like bootstrap does, describes style in HTML – when it should stay in the styles. You can use an inner .inner-w or something to emulate a 'main column' like most web pages.

        some block

        some other block

If you have an actual grid, it is a list, and should be described that way. Then you can style the list without adding classes to every li.

            grid item 1

            grid item 2

            grid item 3

            grid item 4

            grid item 5

            grid item 6


Brain Flak implementation in Haskell I'm a beginner to Haskell and functional programming as a whole and as a first project I've written a Brain-Flak implementation in Haskell.

I wanted to get some feedback on this implementation because I've never done anything this big in a functional programming language.  Is my code two iterative?  What could make this better Haskell?  I found pattern matching very useful, am I using it too much?

BrainHack:

Currently I don't have any fancy IO for this program, just a function that can be called with ghci.  The relevant function here is brainflak it takes a String as source and a [Integer] as input to the Brain-Flak.  It is really just a wrapper around the function bf that uses balanced to first check if the Brain-Flak code is syntactically valid.  bf does most of the computation using pattern matching and the Three Stack Model of Brain Flak.

-- pop is a head that defaults to zero
pop :: (Integral a) => [a] -> a
pop [] = 0
pop (x:_) = x

-- rest is a tail for pop
rest :: (Integral a) => [a] -> [a]
rest [] = []
rest (_:x) = x

-- topadd adds an Integer to the top of a [Integer]
topadd :: [Integer] -> Integer -> [Integer]
topadd [] x = [x]
topadd (a:[]) x = [a+x]
topadd (a:b) x = (a+x):b

-- ir is a helper function of interior
ir :: String -> Integer -> String
ir x 0 = ""
ir ('{':x) y = "{" ++ (ir x (y+1))
ir ('}':x) y = "}" ++ (ir x (y-1))
ir (a:x)   y = [a] ++ (ir x   y  )

-- interior finds the inside of a loop {x}... -> x
interior :: String -> String
interior x = init (ir x 1)

-- ex is a helper function for exterior
ex :: String -> Integer -> String
ex x 0 = x
ex ('{':x) y = ex x (y+1)
ex ('}':x) y = ex x (y-1)
ex  (a:x)  y = ex x y

-- exterior finds all the code after a loop {...}x -> x
exterior :: String -> String
exterior x = ex x 1

-- bf is the implementation of brain-flak
bf :: String -> ([Integer],[Integer],[Integer]) -> ([Integer],[Integer],[Integer])
bf []          (x,y,z)= (x,y,z)
bf ('(':')':a) (x,y,z)= bf a (x,y,((pop z+1):rest z))
bf ('<':'>':a) (x,y,z)= bf a (y,x,z)
bf ('{':'}':a) (x,y,z)= bf a ((rest x),y,(topadd z (pop x)))
bf ('[':']':a) (x,y,z)= bf a (x,y,(topadd z (toInteger (length x))))
bf ('(':a)     (x,y,z)= bf a (x,y,(0:z))
bf ('<':a)     (x,y,z)= bf a (x,y,(0:z))
bf ('[':a)     (x,y,z)= bf a (x,y,(0:z))
bf (')':a) (x,y,(h:z))= bf a ((h:x),y,(topadd z h))
bf (']':a) (x,y,(h:z))= bf a (x,y,(topadd z (-h)))
bf ('>':a) (x,y,(_:z))= bf a (x,y,z)
bf ('{':a)      t     = bf (exterior a) (loop (interior a) t)
bf (_:a)        t     = bf a t

-- loop runs the same code until the TOS is zero
loop :: String -> ([Integer],[Integer],[Integer]) -> ([Integer],[Integer],[Integer])
loop s ([],y,z)  = ([],y,z)
loop s (0:x,y,z) = (0:x,y,z)
loop s x         = loop s (bf s x)

-- bl is an helper function of balance
bl :: String -> String -> Bool
bl [] [] = True
bl [] _  = False
bl ('(':x) y   = bl x (')':y)
bl ('[':x) y   = bl x (']':y)
bl ('<':x) y   = bl x ('>':y)
bl ('{':x) y   = bl x ('}':y)
bl  _ [] = False
bl (a:x) (b:y) = (a == b) && (bl x y)

-- balanced checks if a particular String is balanced
balanced :: String -> Bool
balanced x = bl x []

-- Implements Brain-Flak
brainflak :: String -> [Integer] -> [Integer]
brainflak s x
 | balanced source = (\(a,_,_) -> a) (bf source (x,[],[]))
 | otherwise  = error "Unbalanced braces."
 where source = [a|a <- s, elem a "()[]<>{}"]

You can make the structure of your code a bit more intuitive and obvious by using something akin to a REPL.

Now back when Lisp was the new hot stuff, people would write interactive programs like so: (loop (print (eval (read (..))))), which is executed from inner to outer. We can adapt this structure for general interpreters, something like: print $ evalBF $ parseBF. Now since haskell doesn't like the side-effects that print entails when we're not in a Monad, we can even go so far as to abstract that away:

main :: IO ()
main = do
    -- making a bit of an assumption as to how things are passed in
    (program:arguments) <- getArgs
    let compiled = parse program
    print $ execute compiled $ asStacks arguments

This separates the responsibility of semantics from the responsibility of syntax, which enables us to theoretically define a different syntax for a BrainFlak equivalent language and choose the syntax through a command line switch or so.

So far, so clear.

At this point we want to grab a unified representation of a BF program, that doesn't rely on syntax. Now the esolangs wiki entry you linked specifies exactly these things are called: Monads and Nilads.

Now an OOP programmer would attempt to encapsulate the syntax and the behaviour of these into the same thing, but since we want to keep data and behaviour separate in haskell, it makes more sense to attack this from a different angle.

For that purpose I'd propose the following:

data BfProgram = OneNilad |
    HeightNilad |
    PopNilad |
    ToggleNilad |
    PushMonad BfProgram |
    EvalMonad BfProgram |
    LoopMonad BfProgram |
    ExecuteMonad BfProgram |
    Sequence [BfProgram]

thanks to Zeta for suggesting OneNilad and HeightNilad

This enables us to somewhat cleanly parse a string into a syntax tree like so:

parse :: String -> BfProgram
parse '(':')':cont = Sequence $ OneNilad : (flattenSeq $ parse cont)
parse '[':']':cont = Sequence $ HeightNilad : (flattenSeq $ parse cont)
parse '{':'}':cont = Sequence $ PopNilad : (flattenSeq $ parse cont)
parse '<':'>':cont = Sequence $ ToggleNilad : (flattenSeq $ parse cont)
-- order matters, previous patterns will be matched before these
parse '(':code = Sequence $ (PushMonad $ parse monad) : (flattenSeq $ parse cont)
    where (monad,cont) = divideMonad '(' ')' code
parse '[':code = Sequence $ (EvalMonad $ parse monad) : (flattenSeq $ parse cont)
    where (monad,cont) = divideMonad '[' ']' code
parse '{':code = Sequence $ (LoopMonad $ parse monad) : (flattenSeq $ parse cont)
    where (monad,cont) = divideMonad '{' '}' code
parse '<':code = Sequence $ (ExecuteMonad $ parse monad) : (flattenSeq $ parse cont)
    where (monad,cont) = divideMonad '<' '>' code
parse _ = Sequence []
-- another option would be to throw an error here

Note the similarity to your bf function?

Now this block parses a given syntactically valid BfProgram from a String into an actual Program. To not make it too easy for you, I'll only show you the type-signatures of the helper functions. The divideMonad is the more interesting here (tip, you already have something similar in your code):

divideMonad :: Char -> Char -> String -> (String, String)

and flattenSeq is the following:

flattenSeq :: BfProgram -> [BfProgram]

given this, we can now rewrite bf as execute like so:

execute :: BfProgram -> ([Integer], [Integer], [Integer]) -> ([Integer], [Integer], [Integer])
execute (Sequence []) stacks = stacks
execute (Sequence (x:xs)) stacks = execute (Sequence xs) $ execute x stacks
execute OneNilad (x,y,z) = (x, y, ((pop z+1):rest z))
execute ToggleNilad (x,y,z) = (y,x,z)
execute PopNilad (x,y,z) = ((rest x), y, (topadd z (pop x)))
execute HeightNilad (x,y,z) = (x,y,(topadd z (toInteger (length x))))
-- nilads done, always nice
execute (PushMonad p) (x,y,z) = apply $ execute p (x,y,(0:z))
    where apply (x,y,(h:z)) = ((h:x),y,(topadd z h))
execute (EvalMonad p) (x,y,z) = apply $ execute p (x,y,(0:z))
    where apply (x,y,(h:z)) = (x,y,(topadd z (-h)))
execute (ExecuteMonad p) (x,y,z) = apply $ execute p (x,y,(0:z))
    where apply (x,y,(_:z)) = (x,y,z)
execute (LoopMonad p) stacks = loop p stacks
    where loop p ([],y,z) = ([],y,z)
       loop p (x@(0:_),y,z) = (x,y,z)
       loop p stacks = loop p $ execute p stacks

Written like this we have eliminated the String from execute and thus successfuly separated semantics from syntax (as well as program data from program code)

Now if you compare this with the code you already have, you should notice some striking similarities, and some key differences.

One important difference is that I've opted to use much longer names.
Another one is the use of "local function declarations". That way I eliminate much of -- x is a helper function of y.

Last but not least, I'll recommend considering another step: pop, topadd, and rest are functions that are to be called on BF-stacks. You can clarify their usage by grouping them in a class.

The last thing that remains to say now is: I actually didn't check whether the code I provide here compiles. If you get some compiler errors, please do notify me :)


CodeAbbey - Blackjack Counting Here is my solution for CodeAbbey - Blackjack Counting

My code passes all tests on Code Abbey but I think I have some poor implementation specifically with my handling of the Aces in the score but otherwise I think I overall used good practices and design.

def calculate_player_hand(player_hand_input):
    """Return the score of a player hand"""
    # Variable to hold score for a hand
    total_player_hand_score = 0

    # Dictionary mapping cards to their score value
    card_values = {"2": 2, "3": 3, "4": 4, "5": 5, "6": 6, "7": 7, "8": 8, "9": 9, "T": 10, "J": 10, "K": 10, "Q": 10}

    # Find score of cards except Aces
    for card in player_hand_input:
        if card == "A":
            continue
        else:
            total_player_hand_score += card_values[card]

    # Return player_hand_score not including Ace card(s)
    return total_player_hand_score

# Return the total amount of Aces in a player's hand
def get_aces_count(player_hand_input):
    """Return the count of Aces"""
    return player_hand_input.count("A")

# Read in test cases
test_cases = int(input())

# List to hold all total scores
total_scores = []

# Iterate through test cases
for i in range(test_cases):

    # Read in player hand
    player_hand = input().split()

    # Variable to hold number of Aces in player hand
    player_score_without_aces = calculate_player_hand(player_hand)

    for j in range(get_aces_count(player_hand)):
        if 21 - player_score_without_aces < 11:
            player_score_without_aces += 1
        else:
            player_score_without_aces += 11

    # Rename variable since value of Aces were added
    total_player_score = player_score_without_aces

    # Add total score to total_scores list
    if total_player_score > 21:
        total_scores.append("Bust")
    else:
        total_scores.append(total_player_score)

# Output all total scores
for total_score in total_scores:
    print(total_score, end=" ")

Thank you.

You do not provide a testable function that performs the task of returning the desired answer for a specific test case. It is the ace handling that you intermangled with your test code. The main structure of your program structure should look somewhat like

def calculate_player_hand(player_hand_input):
    #[...]
    return score

def read_tests():
    #[...]
    return testcases, answers

for test, answer in zip(read_tests()):
    assert answer == calculate_player_hand(test)

Completely separate the functionality you want to provide from the tests. even split into different files.


MaxProductOfThree in C# This is my solution for the MaxProductOfThree from codility, it scores 100%, but I'm not a fan of the if if if structure it has going on.

  Task description

  A non-empty array A consisting of N integers is given. The product of
  triplet (P, Q, R) equates to A[P] * A[Q] * A[R] (0 ≤ P < Q < R < N).

  For example, array A such that:

  A[0] = -3   A[1] = 1   A[2] = 2   A[3] = -2   A[4] = 5   A[5] = 6
  contains the following example triplets:

  (0, 1, 2), product is −3 * 1 * 2 = −6 (1, 2, 4), product is 1 * 2 * 5 = 10 (2, 4, 5), product is 2 * 5 * 6 = 60 Your goal is to find the maximal product of any triplet.

  Write a function:

  class Solution { public int solution(int[] A); }

  that, given a non-empty array A, returns the value of the maximal
  product of any triplet.

  For example, given array A such that:

  A[0] = -3   A[1] = 1   A[2] = 2   A[3] = -2   A[4] = 5   A[5] = 6
  the function should return 60, as the product of triplet (2, 4, 5) is
  maximal.

  Write an efficient algorithm for the following assumptions:

  N is an integer within the range [3..100,000]; each element of array A
  is an integer within the range [−1,000..1,000].

using System;
using System.Collections.Generic;
using System.Linq;

class Solution {
    public int solution(int[] A)
    {
        List<int> B = A.ToList();
        int product = 1;
        int min1 = B.Min();
        if (A.Length > 3)
        {
            if (min1 < 0)
            {
                B.RemoveAt(B.IndexOf(min1));
                int min2 = B.Min();
                if (min2 < 0)
                {
                    int max1 = B.Max();
                    if (A.Length == 4)
                    {
                        product = min1 * min2 * max1;
                    }
                    else
                    {
                        product = Math.Max(min1 * min2 * max1, getProductOfMaximalsForMoreThanTwoElements(B));
                    }
                }
                else
                {
                    product = getProductOfMaximalsForMoreThanTwoElements(B);
                }
            }
            else
            {
                product = getProductOfMaximalsForMoreThanTwoElements(B);
            }
        }
        else
        {
            foreach (int element in B)
            {
                product *= element;
            }
        }
        return product;
    }

    private int getProductOfMaximalsForMoreThanTwoElements( List<int> B)
    {
        int max1 = B.Max();
        B.RemoveAt(B.IndexOf(max1));
        int max2 = B.Max();
        B.RemoveAt(B.IndexOf(max2));
        int max3 = B.Max();
        return max1 * max2 * max3;
    }
}

To be honest, the original idea was thought beforehand, but it took me a couple of tries until every test was passed, as with most of these exercises. Are there things to improve in terms of readability, logic and quality of this code?

This looks like a good attempt. Let's first start assuming we only have to deal with positive values.

private int getProductOfMaximalsForMoreThanTwoElements( List<int> B)
{
    int max1 = B.Max();
    B.RemoveAt(B.IndexOf(max1));
    int max2 = B.Max();
    B.RemoveAt(B.IndexOf(max2));
    int max3 = B.Max();
    return max1 * max2 * max3;
}

This method would provide the solution. So let's review it.

Starting with a couple of style choices, method names in C# should be PascalCase. Additionally, the name of the method is vague. The method provides the product of the 3 biggest numbers: GetProductOfTop3Numbers.

B.IndexOf(B.Max()) is doing unnecessary work. Max loops over the entire list to find the maximum value, and IndexOf does it all over again. That's a waste of effort. Sadly, System.Linq doesn't provide an ArgMax method, so we will have to spin our own:"

public static int ArgMax(this IList<int> list) {
  var max = int.MinValue;
  var argmax = -1;
  var index = 0;
  foreach(var item in list) {
    if (item > max) {
      max = item;
      argmax = index;
    }
    index++;
  }
  return argmax;
}

Here, instead of looping through the list to find the maximum value, and then search the list for that value, we can just return the index we found that biggest value at. The max is then easily retrievable through B[index].

The act of searching the list for the max, and popping it can be its own method:

public static int PopMax(List<int> list)
{
  var argmax = list.ArgMax();
  var max = list[argmax];
  list.RemoveAt(argmax);
  return max;
}

Our initial method would then be:

private static int GetProductOfTop3Numbers(List<int> list) {
  var product = 1;
  product *= PopMax(list);
  product *= PopMax(list);
  product *= PopMax(list);
  return product;
}

Or, more generic:

private static int GetProductOfTopNNumbers(List<int> list, int n) {
  var product = 1;
  for(var times = 0; times < n; times++) {
    product *= PopMax(list);
  }
  return product;
}

Now let's return to the actual problem: negative numbers. This is what caused you problems introducing a bunch of if-statements. Let's see if we can find something for that.

We want the highest possible product. If we simply take the top 3 items from the list, we might risk missing out on very negative numbers. Take for instance the list new [] { -5, -5, -3, -1, 0, 1, 2, 3 }. In this case, the naive solution would be 1 * 2 * 3, but actually -5 * -5 * 3 has a bigger product. Note that if we consider negative numbers in our solution, it will always have to be a pair of negative numbers, to cancel out the - signs.

This pair would then always be the two lowest entries in the list. The two most negative numbers together should provide the biggest product out of all negative numbers. The product of these can then be compared to the product of the biggest numbers, to decide which of the two sets to pick.

Furthermore, because we will never take 3 negative numbers (because the product would be negative), we can freely take the biggest number in the list. Should this number also be negative (because the input only consists of negative numbers), this is fine as well, because it would then be the least negative number, which still would result in the biggest product.

So, to recap:

Take the biggest number
Take the next two biggest numbers, or take the next two smallest numbers
Check which together has the biggest product
Done

For the mins, we need to provide a similar ArgMin and PopMin implementations. Also realise that simultaneously popping items from the list at both ends might cause troubles if this list is very short, so be sure to copy the list:

private static int GetProductOfTopNNumbers(List<int> list, int n) {
  var copiedList = new List<int>(list);
  var product = 1;
  for(var times = 0; times < n; times++) {
    product *= PopMax(copiedList);
  }
  return product;
}

All in all, that leaves us with:

public int Solution(int[] A) {
  var list = A.ToList();
  var max = PopMax(list);
  return Math.Max(max * GetProductOfMinNNumbers(list, 2), max * GetProductOfTopNNumbers(list, 2));
}

public static int ArgMin(this IList<int> list)
{
  var min = int.MaxValue;
  var argmin = -1;
  var index = 0;
  foreach (var item in list)
  {
    if (item < min)
    {
      min = item;
      argmin = index;
    }
    index++;
  }
  return argmin;
}

public static int PopMin(List<int> list)
{
  var argmin = list.ArgMin();
  var min = list[argmin];
  list.RemoveAt(argmin);
  return min;
}

private static int GetProductOfMinNNumbers(List<int> list, int n)
{
  var copiedList = new List<int>(list);
  var product = 1;
  for (var times = 0; times < n; times++)
  {
    product *= PopMin(copiedList);
  }
  return product;
}


Check if a number N is a power of K I was asked this question in interview:

Check if a number N is a power of K.
Example:
N = 32, K = 2 => True
N = 40, K = 5 => False

I wrote following code but got the feedback that, complexity could have be improved, How to improve its complexity?
def check_kth_power(n, k):
        while n%k == 0:
            n = n/k

        if n != 1:
            return False
        return True

print check_kth_power(128, 5)

You could have used the fact that if \$n = k^x\$, then \$\log_k(n) = x\$ is an integer. However, due to the limited precision of floats in Python, this will generate some problems. To get around this, we can test both the ceil and the floor of the logarithm to see if we can get back n. This way we only ever need to do three expensive computations at most.

from math import log, ceil, floor

def check_kth_power(n, k):
    kth_log = log(n, k)
    return k ** int(ceil(kth_log)) == n or \
           k ** int(floor(kth_log)) == n

Now, instead of doing the exponentiation twice, we can realize that k ** int(ceil(kth_log)) = k * k ** int(floor(kth_log)) for a small speed-boost in about 50% of the cases (thanks to @KyleGullion:

def check_kth_power(n, k):
    candidate = k ** int(log(n, k))
    return candidate == n or k * candidate == n

This seems to work for quite large values (I checked up to about \$2^{32000}, 2^{20000} - 1, 3^{20000}\$). This is the version in the timing table below.

To get even more of a speed-boost, you can steal the nice premature exit from @pantarei's answer. This sacrifices some speed in the case that the number is a power of k, but gains about a factor 2 if it is not:

def check_kth_power_gp(n, k):
    if n%k != 0:
        return False
    candidate = k ** int(log(n, k))
    return candidate == n or k * candidate == n

And, finally, some timings. This compares my code and the code from @user1825464's answer, @pantarei's answer, @RyanMill's answer, @Eevee's answer and @JamesMishra's answer:

+-------------+---+----------+-------------+-----------+------------+---------+--------------+
|      n      | k | Graipher | user1825464 | panta rei | Ryan Mills |  Eevee  | James Mishra |
+-------------+---+----------+-------------+-----------+------------+---------+--------------+
| 2**1000     | 2 | 1.73 µs  | 9.9 µs      | 105 µs    | 12.9 µs    | 388 µs  | 3.23 µs      |
| 2**1000 - 1 | 2 | 1.99 µs  | 2.5 µs      | 619 ns    | 15.7 µs    | 765 ns  | 3.09 µs      |
| 3**1000     | 2 | 2.41 µs  | 4.26 µs     | 854 ns    | 22.4 µs    | 1.04 µs | 4.08 µs      |
| 3**1000     | 3 | 2.81 µs  | 12.6 µs     | 125 µs    | 13.8 µs    | 556 µs  | 4.51 µs      |
+-------------+---+----------+-------------+-----------+------------+---------+--------------+

So the log does not care if it actually is a kth power, whereas the loop has to do more work in that case, but potentially finds that it is not a power faster.

You could be checking for integerness using x % 1 == 0 for integers, != 0 otherwise (thanks to @Peilonrayz in the comments):

from math import log

def check_kth_power(n, k):
    return log(n, k) % 1 == 0

But, as noted in the comments, this works only until the precision of float is not enough anymore to distinguish that the result of the log is not an integer.

For numbers of the form \$ 2^x - 1\$, this happens at \$x = 48\$, as noted by @GarethReese, so you should have asked if there are any limits on n and k.


Crawl and gather all the urls recursively in a domain This is the script :

#!/bin/bash

set -euo pipefail

SOFT="$(basename $0)"
random=$(( ( RANDOM % 100000 ) ))
tempfolder="/tmp/$SOFT/$random"

mkdir -p "$tempfolder"
pushd "$tempfolder" > /dev/null
cleanup (){
    cat wget.txt | sed 's/:$//'  | url_matcher
    popd > /dev/null
    rm -rf -- ${tempfolder}
}

trap cleanup EXIT
site="$1"

wget -nd --spider --recursive --no-verbose --output-file=wget.txt "$site"

the script essentially creates a subfolder in /tmp/crawl and puts into it the result of wget, then feeds it to sed and url_matcher.

Initially I though of putting the cat instruction just after the wget one, but for long wget it didn't work.

The sed instruction removes colons at the end of lines, which is added by wget when said url isn't valid.
(something along the lines of "http://...: Nothing there")

url_matcher is a c++ scanner made with flex, which recognises urls in a text fed with standard input and prints them to standard output, separated with new lines characters.

the scripts allows me to fetch all urls in a website and use that as a stream directly.

Example :

$ crawl www.example.com | sed 's/some_treatments//' > super_file

There's no need to use cat here:
cat wget.txt | sed 's/:$//'  | url_matcher

Instead, simply have sed take its input from the file:
sed 's/:$//' wget.txt | url_matcher

Instead of using $RANDOM and mkdir, we can use the mktemp command (GNU coreutils) to create us a new directory (without risk of conflict with an existing one):
tempfolder=$(mktemp -d)

I think that now removes the only Bashism in the script.

Avoid pushd and popd in scripts (having to redirect the unwanted output should be a sign that these are intended for interactive shells).  Instead, we can use cd "$tempfolder" and something like cd .. to leave it (it doesn't matter where we end up, as we only use absolute paths after that).  Alternatively, don't change directory at all, but specify paths in full (I'll use a shorter name for the temporary directory):
dir=$(mktemp -d)

cleanup() {
    sed -e 's/:$//' "$dir/wget.txt" | url_matcher
    rm -rf -- "$dir"
}

trap cleanup EXIT

wget -nd --spider --recursive --no-verbose --output-file="$dir/wget.txt" "$1"

Other alternatives to pushd/popd include saving $PWD into a variable before changing directory, or using cd - (which internally does exactly that, as cd saves $PWD into $OLDPWD for that purpose).

We can avoid the temporary file altogether, to simplify the code and to allow us to start producing output without waiting for the crawl to finish.  We can do this in a couple of ways, by being creative with --output-file.  If we're on Linux, we can write to /dev/stdout instead of a file:
wget -nd --spider --recursive --no-verbose --output-file=/dev/stdout "$1" \
  | sed -e 's/:$//' | url_matcher

If we're happy to accept a Bash dependency again, we can use process substitution:
wget -nd --spider --recursive --no-verbose \
     --output-file=>(sed -e 's/:$//' | url_matcher) "$1"

However, neither of those dependencies are necessary, as wget follows the common convention of treating the filename - specially, to mean "standard output stream":
wget -nd --spider --recursive --no-verbose --output-file=- "$1" \
  | sed -e 's/:$//' | url_matcher

Modified code
#!/bin/bash

set -uo pipefail

exec wget -nd --spider --recursive --no-verbose --output-file=- "$1" \
    | sed -e 's/:$//' | url_matcher


Tic Tac Toe using Cartesian coordinate system I am trying to learn the Cartesian Coordinate System, starting with a basic tic tac toe grid for simplicity, then perhaps moving to chess. I was able to make the following code to start out:

board = []

def new_board():
    for row in range(4):
        board.append([])
        for column in range(4):
            board[row].append('_')

def print_board():
    print(board)
    print('  %s | %s | %s' % (board[1][1], board[1][2], board[1][3]))
    print('----+---+----')
    print('  %s | %s | %s' % (board[2][1], board[2][2], board[2][3]))
    print('----+---+----')
    print('  %s | %s | %s' % (board[3][1], board[3][2], board[3][3]))

new_board()
print(board)
print_board()

When the list 'board' is printed, it gives the following:

[['_', '_', '_', '_'], ['_', '_', '_', '_'], ['_', '_', '_', '_'], ['_', '_', '_', '_']]
[['_', '_', '_', '_'], ['_', '_', '_', '_'], ['_', '_', '_', '_'], ['_', '_', '_', '_']]

Which is fine, but my issue is that in order for my commands to match the board (e.g. board[1][2] targeting the 1st row and the 2nd column of the board), I have to have a 'dummy entry' for [0], for both the sublists and the entries in the sublists.

Is there a way to do this without the dummy entries and still have my board list references match the actual board spaces?

It is a convention in Python to use the _ variable as the 'throw away variable'.
In your case you can replace column with it.
Rather than using a for loop with list.append you can use a list comprehension.

data = []
for i in range(4):
    data.append(i)

data = [i for i in range(4)]

When you build an array with all of the items having the same immutable value you can make an array with just one value and multiply the array.

foo = [None for _ in range(4)]

foo = [None] * 4

You should not rely on globals to hold state. There are two solutions:

Pass and return the values from the functions.
Use a class to encapsulate state.

Strings % method for formatting is somewhat archaic and buggy. It should be noted that str.format or f-strings are a better alternate with more features.

values = (1, 2)

'%s %s' % values

'{0[0]} {0[1]}'.format(values)
'{} {}'.format(*values)

f'{values[0]} {values[1]}'

It is best practice to use an if __name__ == '__main__' guard to prevent your code from running if imported by accident.
Python uses 0 based indexing. I have had no problems with it once adjusted. However if you do you'd be better off not using Python.

Lua, Julia and R are all languages that use 1-based indexing.

def new_board():
    return [['_'] * 3 for _ in range(3)]

def print_board(board):
    print(
        '  {} | {} | {}  '.format(*board[0])
        + '----+---+----'
        + '  {} | {} | {}  '.format(*board[1])
        + '----+---+----'
        + '  {} | {} | {}  '.format(*board[2])
    )

if __name__ == '__main__':
    board = new_board()
    print(board)
    print_board(board)


Finding the minimum of two numbers For this flowchart on finding the minimum of 2 numbers:

I've created XML for this flowchart:

<start>
    <InOut>
      <in>  a </in>
      <in>  b </in>
      <out> min </out>
    </InOut>
    <calc>
         a=3
         b=5
         min=a
    </calc>
    <con gurd="min>b"  type="if">
        <calc>
            min=b
        </calc>
    </con>
    <con gurd="min>b" type="else">
        <calc>
            min=a
        </calc>
    </con>
</start>

But I don't think it's not good XML coding.  What are your suggests for improving this?

There are some confusing aspects to your XML, and some practices that you may end up appreciating.....
Technical
Your XML is valid, but it does not follow best practices. The XML:

   <con gurd="min>b"  type="if">

This should be written:
    <con gurd="min&gt;b"  type="if">

Even though your XML is valid, the > in the attribute value can lead to confusion. Even the XML Syntax parser here on Stack Exchange is failing to parse that part right....
Naming
XML is by nature relatively verbose. You are already 'paying' for that, so you may as well use it to your advantage. con should be condition, ins should be ..... instruction ? What is gurd ?
Additionally, you should be consistent in cases. InOut should be inputoutput or something. Mixing cases for XML tag names is a problem. I recommend making it all lowercase. I would actually call it 'interface', and have 'input' and 'output' as child elements.
Functional
When you use XML, you need a parser. There is always a balance between what you parse, and what you let the parser parse. The 'text' component in the XML elements are technically called PCDATA (Parsed Character Data). The XML Parser will Parse that data looking for more XML. That's all it does. Your actual data has things like: min=a Is your application manually parsing that content as well? If it is, you should probably make your text content attribute values instead.
Additionally, the white-space in your element text is scattered all over the place. Consider:

<calc>
     a=3
     b=5
     min=a
</calc>
<con gurd="min>b"  type="if">
    <calc>
        min=b
    </calc>
</con>

In your first  elements you have space surrounding the text. In your later  there is no space.
Is the space important? If it is, you should either use attributes, or set xml:space="preserve" on your XML.
Update
FYI, I ran your XML through xmllint and it output your XML as:
<?xml version="1.0"?>
<start>
  <InOut>
    <in>  a </in>
    <in>  b </in>
    <out> min </out>
  </InOut>
  <calc>
     a=3
     b=5
     min=a
  </calc>
  <con gurd="min&gt;b" type="if">
    <calc>
      min=b
    </calc>
  </con>
  <con gurd="min&gt;b" type="else">
    <calc>
      min=a
    </calc>
  </con>
</start>

xmllint --format your.xml is a fantastic commandline tool that validates and reformats your XML to be consistent. I recommend it to anyone using XML that has access to Linux


A simple PyPI crawler I had to make a program to crawl through a bunch of PyPI packages and see how many of them implement custom compare operators (i.e. grep for def __le__ etc). After downloading an HTML file with links to all Python 3.4 packages on PyPI (i.e. the directory page), I wrote this simple crawler to go through all the links, download and unzip each package, and grep them for custom compare definitions. It's rudimentary, but still, what are your comments? This is my first "shell-script" style Python program, i.e. a program where you're not computing stuff but just moving around files and doing networking.

Code:

import sys
assert(sys.version_info >= (3,5))

import re
from requests import get
import subprocess
def run(s):
    return subprocess.run(s,
                          shell=True,
                          stdout=subprocess.DEVNULL,
                          stderr=subprocess.DEVNULL)

directory = open('list.html').read() #download list from Browse Packages ->
                                     #                   Python 3.4 -> Show All

custom = no_custom = failiures = 0
for (package_url, package_name) in \
    re.findall('(https://pypi\.python\.org/pypi/([^/]+)/)', directory):
    print(custom+no_custom+failiures,
          custom,
          no_custom,
          failiures)

    try:
        package_page = get(package_url).text
        (download_url,file_type) = re.search('.+(\.tar\.gz|\.zip)',
                                             package_page).groups()
        print(package_name)
        archive = open('archive', 'wb')
        archive.write(get(download_url).content)
        archive.close()

        run('rm -r package_code')
        run('mkdir package_code')

        if file_type == '.tar.gz':
            run('tar -xzf archive -C package_code')
        if file_type == '.zip':
            run('unzip archive -d package_code')
        return_code = run('grep -Er "def __(le|lt|ge|gt)__" ./package_code').returncode

        if return_code == 0:
            custom += 1
        elif return_code == 1:
            no_custom += 1
        else:
            failiures += 1

    except (KeyboardInterrupt, SystemExit):
        raise
    except Exception as exception: #for when there is no .tar.gz or .zip on PyPI
                                   #or (rarely) when the connection is dropped
        print("FAILIURE:", type(exception).__name__)
        failiures += 1

print("""
Packages that define custom compare operators: %i
Packages that don't define custom operators: %i
Packages that didn't have source on PyPI: %i
""" % (custom, no_custom, failiures))

Here are some comments/notes about the code and potential improvements:

use with context manager when opening files
parsing HTML with regular expressions has always been a very controversial thing to do. I would switch to an HTML parser like BeautifulSoup and lxml.html. For example, getting all the PyPI links with BeautifulSoup can be as straightforward as:

from bs4 import BeautifulSoup

with open('list.html') as directory:
    soup = BeautifulSoup(directory, "html.parser")
    for link in soup.select("a[href*=pypi]"):
        print(link.get_text())

where a[href*=pypi] is a CSS selector that would match all a elements that has pypi substring inside an href attribute.
instead of using requests.get() directly, initialize a "session" to reuse the underlying TCP connection:

  ..if you're making several requests to the same host, the underlying TCP connection will be reused, which can result in a significant performance increase..

If you want to scale this up, you would need to switch for a synchronous and blocking code/approach to asynchronous - look into using Scrapy web-scraping framework which is based on the twisted networking library.


Sidebar menu to hide or expand parts of a form I have some of my code that I have tried to improve on to make it cleaner and efficient and I would like some feedback to see if I am going in the right direction with what I am trying to do.

I have a page with a menu, and when you click on menu item it displays the appropriate fieldset.

HTML:

My Account

Order History

Notifications

        <fieldset id="fs_account_details">

        </fieldset>

        <fieldset id="fs_order_history">

        </fieldset>

        <fieldset id="fs_notifications">

        </fieldset>

My original jQuery to display each fieldset was an individual block of code for each button like this:

// Show account details
$('#list_account_details').click( function(){
    $('fieldset').hide( function(){
        $('#fs_account_details').show();
    });
});

// Show order history
$('#list_order_history').click( function(){
    $('fieldset').hide( function(){
        $('#fs_order_history').show();
    });
});

// Show notifications
$('#list_notifications').click( function(){
    $('fieldset').hide( function(){
        $('#fs_notifications').show();
    });
});

And my improvement was to only have one block of code to display each fieldset like this:

// Show appropriate fieldset
$('#side_bar_menu li').click( function(){

    // Get button id
    var button_id = $(this).attr('data-fieldset-id');

    // Hide any opened fieldsets
    $('fieldset').hide( function(){
        // Show appropriate fieldset
        $(button_id).show();
    });
});

Should I be thinking to automatically write all my code like this, or am I possibly reducing individual functionality of each menu list button by incorporating it all into one block of code like that?

Is my new code even considered an improvement at all?

I think, your improved code looks okay temporarily until you try to add new functionalities.

A function should do only one thing.

Trying to add a custom functionality to your improved code would result in clutter and more if else statements.


Quicksort implementation with pivotal calculated as middle element I read quick sort algorhitm and implemented in like this:

public List<Integer> sort(List<Integer> list) {
    if (list.size() <= 1) {
        return list;
    }

    int pivotalValue = list.get(list.size() / 2);

    List<Integer> left = new ArrayList<>();
    List<Integer> pivotalValues = new ArrayList<>();
    List<Integer> right = new ArrayList<>();
    for (Integer element : list) {
        if (element < pivotalValue) {
            left.add(element);
        } else if (element > pivotalValue) {
            right.add(element);
        } else {
            pivotalValues.add(element);
        }
    }
    List<Integer> sortedLeft = sort(left);
    List<Integer> sortedRight = sort(right);
    sortedLeft.addAll(pivotalValues);
    sortedLeft.addAll(sortedRight);
    return sortedLeft;

}

What do you think about my implementation?

What do you think about my implementation?

To tell you the truth, I don't think much of your implementation.  Instead of in-place swaps you're creating a bunch of temporary lists and combining them after.  It probably more closely resembles a merge sort than a quick sort.

Creating all those lists and merging them together requires many more iterations than the usual quick sort.

Altogether, it seems to me, that both the time and space complexity is worse than the usual quick sort.


Database connection query I'm a beginner PHP developer and it's my first time using MySQLi. I'm not sure if this is a correct implementation of the connection and query because some people defend this method and some others not. I need a solid implementation for heavy database usage.

Connection:

class bbddConnection {

private $_host = 'localhost';
private $_username = 'username';
private $_password = 'password';
private $_db_name = 'DBNAME';
private $_mysqli = NULL;
private static $_instance = NULL;

private function __construct() {
    $this->connection();
}

public static function get_instance() {
    if (!isset(self::$_instance)) {
        //echo "Creating new instance.\n";
        $class_name = __CLASS__;
        self::$_instance = new $class_name;
    }
    return self::$_instance;
}

private function connection() {
    //echo "Creating new connection.\n";
    try {
        //The mysqli object will be created once and only once
        if (!$this->_mysqli instanceof mysqli) {
            $this->_mysqli = new mysqli($this->_host, $this->_username, $this->_password);
        }
        if ($this->_mysqli->connect_errno) {
            throw new Exception('An error occured: ' . $this->_mysqli->connect_error);
        } else {
            $this->select_db();
        }
    } catch (Exception $e) {
        echo $e->getMessage() . "\n";
    }
}

private function select_db() {
    //echo "Selecting database.\n";
    try {
        $this->_mysqli->select_db($this->_db_name) or die('Could not find database');
    } catch (Exception $e) {
        echo $e->getMessage() . "\n";
    }
}

public function query($query) {
    if (!isset($query))
        return;
    //echo "Prevent SQL Injection. \n"
    $query = $this->_mysqli->real_escape_string($query);
    return $this->_mysqli->query($query);
}

public function stmt_init() {
    return $this->_mysqli->stmt_init();
}

public function __destruct() {
    if ($this->_mysqli instanceof mysqli) {
        $this->_mysqli->close();
    }
}

 private function __clone() {
    trigger_error('Clone is not allowed.', E_USER_ERROR);
}

  private function __wakeup() {
    trigger_error('Unserializing is not allowed.', E_USER_ERROR);
}
}

Query:

class gestorUsuarios {

private $db;

public function __construct() {
    $this->db = bbddConnection::get_instance();
}
/**
 * Returns the Name of the user according his ID
 * @param int id
 * @return string Nombre, if an error occurred.
 * returns FALSE
 */
function getUserName($id)
{

    $stmt = $this->db->stmt_init();
    if ($stmt->prepare("Select Nombre from usuarios where id = ?")) {
        $name = '';
        $stmt->bind_param('i', $id);
        $stmt->execute();
        $stmt->bind_result($name);

        while ($stmt->fetch())
            $nombre = $name;
        $stmt->close();

        return $nombre;
    } else {
          return false;
    }
}
/**
 * Edit the Name of the user according his ID
 * @param String Nombre, int id
 * @return Boolean if an error occurred.
 * returns FALSE, else returns TRUE
 */
function setUserName($nombre, $id)
{

    $stmt = $this->db->stmt_init();
    if ($stmt->prepare("UPDATE usuarios SET Nombre=? WHERE id =?")) {

        $stmt->bind_param('si', $nombre, $id);
        $stmt->execute();
        $stmt->close();

        return true;
    } else {
        return false;
    }
}
}

Similar to this one: Minimalistic mysql db class

To answer your question: NO!

The short answer: Throw away your code, burn it, do whatever you want with it but never use it in a production enviroment. Just don't.

The long answer: Again, don't use this code, ever. You are even violating nearly every SOLID rule

As far as I can see you are trying to write some wrapper around the mysqli object. But the question remains: why? and what problem are you trying to solve?

As far as I can see, you are trying to solve 2 problems with one solution. Oh oh, this is doomed to fail. You want to solve the problem of always having to check wether an instance of mysqli exists or not and you want to not have to worry about connecting and creating a mysqli instance and query-ing/sanitizing the database with a given query. 3 different things.

Then you start designing the solution to that problem. When that is done, you should take a step back and ask the question 'Is my code easy to use?' and 'Is it easy configurable and extendable?' and most important 'is it easy to debug?'.

The answer to those 3 questions is simple: NO!

If you want to change a connection parameter you have to change the
code of the class itself, bad bad bad.
If something goes wrong (an exception) you echo an error message. So
what if I first get an instance of your database and it echo's some
error message, and then my code sends a header(); . Ooeehh, a nice
header allready sent error. Hf debugging that error...
...

But in fact, that isn't your real problem. The real problem you have is that you are writing code without designing it.

If your problem is that you want only one instance to exist you should only create one instance instead of relying on a get_instance() function or Singleton pattern. If a class needs a database connection, pass it in in the __construct(). More on this topic:

http://www.slideshare.net/go_oh/singletons-in-php-why-they-are-bad-and-how-you-can-eliminate-them-from-your-applications
https://stackoverflow.com/questions/1812472/in-a-php-project-what-patterns-exist-to-store-access-and-organize-helper-objec

To sum up, sometimes the problems we are solving tend to be meta-problems. The solution we make is actually a fix. A good article on this: http://thedailywtf.com/Articles/The_Complicator_0x27_s_Gloves.aspx

On a complete diffrent note, instead of using mysqli. use PDO.
This is a realy nice OO way of using a database. more on pdo:

http://net.tutsplus.com/tutorials/php/why-you-should-be-using-phps-pdo-for-database-access/
http://www.php.net/manual/en/class.pdo.php


Sorting an array of bands by omitting articles I've written a simple script that takes an array, sorts it (by omitting the articles) and displays it on a list. But I feel like this code is really huge, bulky and messy and could be simplified!

What I'm asking for:

What would be a more efficient way to do what I'm trying to do?
How would I make this code more neat (or compact in other words).
Could any new JS features ease up my work?

<!DOCTYPE html>

    <meta charset="UTF-8">
    <title>Sort Without Articles</title>

I would consider using a regex instead:

const articlesRegex = /^(The|An|A)\s+/i;

const sorted = bands.sort((previous, next) => {
  previous = previous.replace(articlesRegex,'');
  next = next.replace(articlesRegex,'');

You should also return 0 when the items are equal.


Code indentation for declaring inputs/outputs in an array I'm writing a list of inputs and outputs for to be compared in unit tests.

var equals = [
                //input                         //output
                ["name:(John Smith)",           "name:(John~ Smith~)"   ],
                ["name:Jon~0.1",                "name:Jon~0.1"          ],
                ["Jon",                         "Jon~"                  ],
                ["Jon Smith~0.1",               "Jon~ Smith~0.1"        ],
                ["Jon AND Bob",                 "Jon~ AND Bob~"         ],
                ["Jon AND Bob~0.1",             "Jon~ AND Bob~0.1"      ],
                ["Steve^9 Jon", ]               "Steve^9 Jon~"          ]

            ];

I've formatted it as such, so it's easy to compare and read.  However, it's unconventional.

Is this a bad idea?

An argument against table-style alignment in code

Except when the editor/IDE helps maintain alignment with little work from the programmer, and all those who work with the code have that same facility, horizontal, table-like alignment in code is more trouble than it's worth, for these reasons:

Search/replace operations on the entire code-base (as when renaming a variable or method) will tend to leave such code in a mess that needs manual cleanup.
Anyone whose editor/IDE does not make horizontal alignment easy will spend a lot of time hitting space or DEL to make things line up when adding or deleting an entry.
When the entire table needs to be realigned (as when adding a new row that causes a column to grow), the version control system will show that the entire table changed.

For those reasons, I'm inclined to put up with a little bit of ugly:

 var equals = [ //input, output
   ["name:(John Smith)", "name:(John~ Smith~)"],
   ["name:Jon~0.1", "name:Jon~0.1"],
   //...
 ];

or, as @ChrisW suggests:

 var equals = [ //input, output
   [
     "name:(John Smith)",
     "name:(John~ Smith~)"
   ],
   [
     "name:Jon~0.1",
     "name:Jon~0.1"
   ],
   //...
 ];

But maybe data doesn't want to be in the code

Sometimes data wants its own home, a place where "rows and columns" are more natural than in code.  Perhaps that's a table in the database.  In your case, where the table is input to a test, a .csv file might be good.  A non-technical user could then view and edit the data with a spreadsheet program.


Pig Latin Translator - follow-up About a week ago, I put up this code and got some really good help.  I've modified it a bit and was seeking if someone would review my second iteration. Original can be found here.

Here's the new stuff.  Basically, I stuck it all in a module, added support for the option of a dash or no dash.  And gave it the ability to work with sentences.

module PigLatin
class Translator
#how all the words end
ENDING = "ay"

def initialize (options = {:dash => true})
  @dash = options[:dash]
end

def translate (phrase)
  sentences = phrase.split(".")
  translated_sentences = sentences.map do |sentence|
    translation(sentence) + "."
  end
  translated_sentences.join(" ")
end

private

def translation (phrase)
  words = phrase.split
  translated_words = words.map do |word|
    if vowel_is_first(word)
      translate_with_vowel(word)
    else
      translate_with_consonant(word)
    end
  end
  translated_phrase = translated_words.join(" ")
  if @dash == false
    translated_phrase = translated_phrase.delete("-")
  end
  translated_phrase.capitalize
end

def vowel_is_first(word)
  return true if word[0] =~ /a|e|i|o|u|A|E|I|O|U/
  return false
end

def translate_with_vowel(word)
  "#{word}-#{"w"+ENDING}"
end

def translate_with_consonant(word)
  return "#{word[1]}-#{word[0]+ENDING}" if word.size == 2
  second_segment, first_segment = split(word)
  return "#{first_segment}-#{second_segment+ENDING}"
end

def split(word)
  split_location = word =~ /a|e|i|o|u/
  second_segment = word[0,split_location]
  first_segment = word[split_location,word.size]
  return second_segment, first_segment
end
end

class InputAcceptor

def initialize input, options
  @options = options
  @input = input
end

def do_it
  pig_latin = Translator.new(@options)
  pig_latin.translate(@input)
end

end

end
  j = PigLatin::InputAcceptor.new("Hello. You. Guy.",{:dash => false})
  p j.do_it

Some points in addition to what checkorbored pointed out:

You should use character classes and the i flag for your regexen. E.g. instead of /a|e|i|o|u|A|E|I|O|U/ you can write /[aeiou]/i.
You should get rid of the explicit return statements where not necessary.
return foo if bar
return baz

should really be written as

if bar
  foo
else
  baz
end

Except if foo and baz are booleans in which case the whole thing can just be replaced by bar or !bar.
def initialize (options = {:dash => true})
  @dash = options[:dash]
end

In this case it doesn't matter because there's only one option in your options hash, but the usual idiom to implement keyword arguments with defaults looks like this:

def initialize (options = {})
  options = {:dash => true}.merge(options)
  @dash = options[:dash]
end

The reason that (options = {defaults}) is generally not used, is that it doesn't allow the user to specify some options, but use the default for others. So if you do def bla(options = {:foo => 42, :bar => 23}) and the user calls bla(:bar => 24) the value for :foo will be nil, not 42. That doesn't happen if you use merge instead.


Determining the HTTP status code to return for a request I have a piece of code that I would like to refactor but can't figure how to do it. This is a method from a Play controller where I have to verify many precondition and respond different status for each case :

resource should exist or 404
revision should match the If-Match value or 412
payload should be valid or 422

The first attempt looks like below. How do you implement such a suite of verifications?

service.find(key).map { resource =>
  request.headers.get("If-Match").map { expected =>
    if ( resource.revision == expected ) {
      val payload = request.body.validate[Model]
      payload match {
        case JsSuccess(model, _) =>
          Ok()
        case JsError(errors) =>
          UnprocessableEntity()
      }
    } else {
      PreconditionFailed()
    }
  }
} getOrElse {
  NotFound()
}

My best attempt is now to have one method who create a triplet of Option[Resource], String, JsResult[Model] and use pattern matching for each case :

 condition match {
   case (None, _, _) =>
     NotFound()
   case (Some(resource), expected, _) if ( resource.revision!=expected ) =>
     PreconditionFailed()
   case (_, _, JsError(errors)) =>
     UnprocessableEntity()
   case (Some(resource), _, JsSuccess(model, _)) =>
     Ok()
 }

You should learn more about partial functions. Everything you need to know is written in the documentation.

It is a structure that allows you to do a basic check on something and if it succeed, it does something. It is composed of two methods :

apply(v1: A): B
isDefinedAt(x: A): Boolean

It applies the apply method on your input parameter only if the isDefinedAt method returned true when called with your input parameter.

The great thing about partial functions is that you can chain them easily with the andThen, compose and applyOrElse operators. Also, they can be written very easily by using the case syntax :

val divideByTwo: PartialFunction[Int, Int] = { case d: Int if d%2 == 0 => d/2 }

But there is another way to write it :

val divideByTwo = new PartialFunction[Int, Int] {
  def apply(d: Int) = d / 2
  def isDefinedAt(d: Int) = (d%2 == 0)
}

Here is a great article about it.


Selecting Timesheets I'm trying to improve my SQL syntax, take the following for the example

SELECT * FROM `Users:Timesheets`
WHERE
`start` >= '{$monday}'
AND `finish` <= '{$sunday}'
AND `username` = '{$username}'
AND `shift_type` = 'ONCALL'

OR

`start` >= '{$monday}'
AND `finish` IS NULL
AND `username` = '{$username}'
AND `shift_type` = 'ONCALL'

ORDER BY `id` ASC"

There must be a shorter way to achieve this or am I expecting too much?

Any insight to what I may be missing out on?

There's two style comments, and one logic comment. The logic comment is the most significant.... you can rewrite your query as:

SELECT *
FROM `Users:Timesheets`
WHERE
      `start` >= '{$monday}'
  AND (`finish` IS NULL OR `finish` <= '{$sunday}')
  AND `username` = '{$username}'
  AND `shift_type` = 'ONCALL'
ORDER BY `id` ASC

Note, the style changes:

always put each SQL clause on it's own line (the FROM on a new line).
Try to indent each clause in the where clause so that the precedence of the logic is represented by the indentation.

Update: I assume, based on the variable syntax, and so on, that this SQL is not a direct query, but is rather the input to some string substitution that expands the {$...} variables. This is potentially vulnerable to SQL Injection attacks. Are you sure you shoud be running your SQL this way? Have you considered prepared statements? Is this a prepared statement?


Getting JSON with jQuery and creating lists with click events calling more jQuery and JSON I have two elements on my page:

I'm calling some JSON with jQuery and loading elements in the divs. I'm curious if there are things I can be doing more efficiently, and better ways to use selectors and JSON.

$().ready(function() {
    //Load sections
    GetCarlineSections(_BucketID);
});

function GetCarlineSections(bucketID) {
    //Get section json list
    $.ajax(
    {
        type: "POST",
        url: _ApplicationRootURL + 'GetChildBucketList', //Call the ActionResult to return the JSON object
        data: 'parentID=' + bucketID,
        success: function (sections) { //'sections' is an array of JSON objects returned by GetChildBucketList
            $(sections).each(function () {
                $('#VerticalMenu') //Append each item to the #VerticalMenu

                    .append(
                        $('
') //Append a new
 element to
 #VerticalMenu
                        .addClass('Section')
                        .html(
                            $('
') //Create a new
 inside of the

                            .addClass(this.BucketName)
                            .html(this.BucketName)
                            .click({ parentID: this.BucketID }, function (event) { //Attach a click event to the
 element
                                $('#AccordionMenu').empty();
                                GetSectionSubSections(event.data.parentID); //Event.data.parentID is the id of the bucket represented by this
 element
                            })
                        )
                    );
            });
        }
    });
}

function GetSectionSubSections(bucketID) {
    $.ajax(
    {
        type: "POST",
        url: _ApplicationRootURL + 'GetChildBucketList',
        data: 'parentID=' + bucketID,
        success: function (SubSections) { //SubSections are the children buckets of Section, local var bucketID
            $(SubSections).each(function () {
                $('#AccordionMenu')
                    .append(
                        $('
')
                        .addClass('SubSection')
                        .html(
                            $('
')
                            .addClass(this.SEOURLName)
                            .html(this.BucketName)
                            .click({ parentID: this.BucketID }, function (event) { //Eventdata parentID passes the SubSectionID to the event object
                                GetSubSectionHeadlines(this, event.data.parentID)
                            })
                        )
                    );
            });
        }
    });
}

function GetSubSectionHeadlines(parentElement, SubSectionID) {
    //Get the Headlines based on the parent SubSection
    $(parentElement).after($('
').addClass(parentElement.className));
    $.ajax({
        type: 'POST',
        url: _ApplicationRootURL + 'GetChildBucketList',
        data: 'parentID=' + SubSectionID,
        success: function (Headlines) {
            $(Headlines).each(function () {
                $('ul.' + parentElement.className).prepend(
                    $('
')
                        .html(this.BucketName)
                )
            })
        }
    });
}

Two small nickpicks at the start:

Empty HTML lists (ul, ol) are strictly spoken invalid. They should contain at least one list item. That said, I believe there is no browser limitation or other technical reason an empty list shouldn't work.
It is custom for the names of functions, variables and object fields in JavaScript to be written with a small letter at the beginning.

You can optimize adding the list item to the list, by using .map() to create the items and appending them all at once.

$('#VerticalMenu')
  .append(
    $(sections).map(function () {
      $('<li/>')
      // ...
    })
  );

BTW, since sections is an array of simple JavaScript objects and not DOM objects it's a bit weird to wrap them in a jQuery object ($(sections).each(...)), because jQuery unwraps them immediately anyway. You should use $.each(sections, ... ) (or $.map(sections, ... )) instead.

You are assigning all items in your lists the same class (Section and SubSection respectively). Unless you remove the class later, this is usually a sign of wrong CSS design. If you leave out the class, then instead of the selector .Section { ... } you can use a descendent selector: #VerticalMenu li { ... }.

One last thing: The use of h4 elements seems wrong to me. Either this is a menu, then you shouldn't be using header elements at all, or the items in AccordionMenu are subheadlines and be using h5 instead.


Adding two numbers as strings from a textfile I just implemented code to add two ints together as strings and I'm sure my method is inferior. I've looked up other methods too but I'd appreciate criticisms specific to my code. I really want to make sure I don't develop bad habits in C++ and I think leveraging the knowledge of brighter individuals than myself on this site will help with that :).
Am I relying too much on iterators btw?

Anyhow, the code is below and be as harsh as you want. Thanks!

//This program intends to read very large integers from a text file
//the format is two integers per line. It will then add them together and output
//the sums one each line

//first, succesfully read from the text file and store into two vectors
//each digit being an element
#include<iostream>
#include<fstream>
#include<string>
#include<vector>
using namespace std;

int main()
{

    string line,s1,s2;
    vector<string> vec1, vec2; //string 1 and string 2 store each of the 2 numbers per line as individual digits
    vector<string> final;      //create vector of strings that holds the sum of each pair of strings per line
    ifstream myfile("bignum.txt"); //this opens the file bignum.txt
    if (myfile.is_open())
    {
       int  count=0; //count lines
       while (!myfile.eof())
       {
           getline(myfile,line);
           bool flag=true; //bool to see if integer 2 should be stored now
           for (int i=0; i<line.length(); i++)
           {
               if (line[i]==' ')
                   flag=false;
               else
               {
                   if (flag)
                       s1+=line[i];
                   else
                       s2+=line[i];
               }

                //by the end of this loop we have two strings representing each of the numbers
           }

           //will now append 0's to the front of the smaller string in order to make the strings the same length
          if (s1.length()<s2.length())
              s1=string(s2.length()-s1.length(),'0')+s1;
          else
              s2=string(s1.length()-s2.length(),'0')+s2;

           //create a variable that acts as the surplus/carry for each single-digit addition
           int carry=0;

           //create iterators allowing for iteration over a vector
           string::reverse_iterator it1 = s1.rbegin(),it2=s2.rbegin();
           string sum_string="";

           for (; it1!=s1.rend(); it1++, it2++)
           {
               //convert chars to ints
               int k1=*it1-'0';
               int k2=*it2-'0';

               //intermediate sum
               int sum_inter=k1+k2+carry;
               carry=0;

               sum_string.insert(0,to_string(sum_inter%10));
               carry+=int((sum_inter-sum_inter%10)/10);

           };

           //insert the last carry into the highest digit position
           sum_string.insert(0,to_string(carry));

           //go through sum_string and omit leading 0's
           bool lead_zero=true;
           string::iterator leadit=sum_string.begin();
           while(lead_zero)
           {
               if (int(*leadit-'0')==0)
                  sum_string.erase(0,1);
               else
                 lead_zero=false;
           }

           final.push_back(sum_string);

           //clear strings 1 and 2 for reuse

           s1.clear();
           s2.clear();
       }

        vector<string>::iterator i1=final.begin();
        int count3=0;
        for (; i1!=final.end(); i1++)
        {
            count3++;
            cout<<"The sum of the numbers on line "<<count3<<" is "<<*i1<<"\n";
        }

    }
    else cout <<"file could not be opened";
    return 0;
}

Some basic C++ advice:

Don't use using namespace std;. It leads to namespace pollution: it you use another library, you may import names in the global namespace that may clash with those in the std namespace. Anyway, adding std:: in front of the standard library components does not hinder readability.
return 0; at the end of main is useless. If the program reaches the end of main without having encountered any return statement, it will automagically return 0.
You compare signed and unsigned integer values in this line:

for (int i=0; i<line.length(); i++)

That means that you didn't turn on the right compiler flags (-Wall, -pedantic and -Wextra flags that you should almost always use) or that you ignored the warnings. Try never to ignore warnings, they help to write good code. To fix this warning, simply turn int i=0 into std::size_t i=0. Ideally, you should even write typename std::string::size_type i=0 but people generally agree that it is too long, and the main standard library implementations use std::size_t anyway for the length of a string.

The rationale for this warning is that in some cases, the comparison will happen, but small negative integers may be compared to big unsigned integers. On theo ther hand, if you compare unsigned integers with different sizes, you will never have such a problem.
It's really no more than a tidbit, but post-incrementation (++i) may be faster than pre-incrementation (i++). We all know that premature optimization is the root of all evils, but this one is simple enough to write as you go. In other words, you should remember this useful guideline:

  Use ++i if you don't have a specific reason to use i++.

The variable count is never used, you can remove it. You compiler should have told you that it was unused.
Try to organize your headers in alphabetical order:

#include<fstream>
#include<iotream>
#include<string>
#include<vector>

It will help you when you want to check whether a header is already included or not.
If you want to check whether an ifstream was successfully openen, don't use is_open, use operator!:

ifstream myfile("bignum.txt"); //this opens the file bignum.txt
if (not myfile)
{
    // An error occured
    // handle the error
}
// else use myfile

Also, this it not the idiomatic way to read a file line by line:

while (!myfile.eof())
{
    getline(myfile,line);
    // ...
}

Here is how you should do it, std::getline has been designed to be used in such a loop:

while (getline(myfile, line))
{
    // ...
}

Actually, using eof can read past end of file which makes it unsafe to read a file line by line until the end of the file is reached.


Am I using C++ pointers and references correctly? I am a newbie to C++ programming and am currently reading this book called Jumping to C++ by Alex Allain.  I have finished the pointers chapter and I am doing the exercises at the end of the chapter.

The following paragraphs are the exercises and included are my own solutions:

  Write a function that prompts the user to enter his or her first name and last name, as two separate values. This function should
  return both values to the caller via additional pointer (or reference)
  parameters that are passed to the function. Try doing this first with
  pointers and then with references. (Hint: the function signature will
  look be similar to the swap function from earlier!)

// pointersExercise01.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include "iostream"
#include "string"

using namespace std;

void printName(string *pointFirstName, string *pointLastName){

    cout << endl << "Hello " << *pointFirstName << " " << *pointLastName;
}

void printName(string &pointFirstName, string &pointLastName){
    cout << endl << "Hi " << pointFirstName << " " << pointLastName;
}

int _tmain(int argc, _TCHAR* argv[])
{
    string firstName, lastName;

    cout << "Enter First Name ";
    cin >> firstName;
    cout << endl << "Enter Last Name ";
    cin >> lastName;

    /*FUNCTION POINTERS*/
    printName(&firstName, &lastName); // print out using pointers

    /*REFERENCE POINTERS*/
    printName(firstName, lastName);

    system("pause");
    return 0;
}

  The next exercise is to modify the program I wrote for exercise 1 so that instead of always prompting the user for a last name, it does
  so only if the caller passes in a NULL pointer for the last name.

// nullPointerExercise3.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include "iostream"
#include "string"

using namespace std;

void userNames(string *pointerFirstName, string *pointerLastName){

    string firstN = *pointerFirstName;
    string LastN = *pointerLastName;
    bool flag;

    do{
        if(LastN == "NULL" ){
            cout << "Again what is your last name? ";
            cin >> LastN;

            flag = true;
        }else{
            flag = false;
        }

    }while(flag != false);
}

int _tmain(int argc, _TCHAR* argv[])
{
    string firstName, lastName;

    cout << "FIRSTNAME: ";
    cin >> firstName;
    cout << "\n" << "LASTNAME: ";
    cin >> lastName;

    userNames(&firstName, &lastName);

    system("pause");
    return 0;
}

If possible, I'd like to know if I did right for both of the exercise and if I am in the right direction.  Comments and tips will be helpful.

I'm not sure whether this should be reviewed yet or not.  I can't decide what critiques are things that you haven't learned yet but will, and things that you haven't learned yet and might not.  (Also, there's a handful of errors that should have been found with basic testing.)

Anyway here's a few things that jumped out at me:

There's no notion of const correctness at all in your code
system("pause") is bad -- it's slow, and implementation dependent

if you want to see the output, run it in a shell directly instead of using system("pause") as a hack

When pointerLastName is NULL, string LastN = *pointerLastName; is undefined behavior and will probably crash.
if(LastN == "NULL" ) makes no sense

a NULL pointer being dereferences does not turn into the string "NULL" -- it crashes (typically)

Your do {} while() loop in the second program is broken
using namespace std; pollutes the global namespace.  It's typically better to just qualify it directly like std::string or import specific things using std::string;.

In small, simple programs, it tends to not matter.  It can become a bad habit though.
#include "iostream" should be #include <iostream> -- I'm actually a bit surprised that compiles.

Also, for what it's worth, I wouldn't use pointers in either of these two programs.  References are a better choice.  Obviously this is a learning exercise though, so this critique doesn't really apply.

Based on our comments back and forth, there seems to be some confusion surrounding pointers.  I'm not sure if it's me misunderstanding what your saying, or a misunderstanding of pointers, but either way, I figure I'll elaborate a bit.

if(LastN == "NULL" )

That is checking if LastN is equal to the string "NULL".  That is not the same thing as checking for a NULL pointer:

string* pNullStr = NULL;
string nullStr = *pNullStr; //undefined behavior (almost certainly a crash)

The only way a string is going to have a value of NULL is if it is actually set to NULL:

string ns = "NULL";
if (ns == "NULL") {
    //this will happen
}

In other words, your userNames function isn't prompting the user for a last name only "if the caller passes in a NULL pointer for the last name" (as your instructions say).  What it's doing is prompting for a last name only if the caller passes in a string (not a pointer to a string) equivalent to "NULL".

Based on your code, the only way a last name is going to be prompted for is if you type "NULL" in when prompted for a last name (in the main code).

Perhaps this was by design, but it directly contradicts your exercise instructions.

Also, while I'm at it with the editing, if you wanted to be really careful, you could check all of the input operations.  In fact, really you should always check, but in a program this simple, it would be a bit of a pain.

To clarify:

cout << "FIRSTNAME: ";
cin >> firstName;

That can fail.  What if the user writes EOF or if the user pipes in an empty file?  That means the read is going to fail.

if (!(std::cin >> firstName)) {
    std::cerr << "Fatal error: reading last name failed";
    return EXIT_FAILURE; //(defined in cstdlib)
}

On a stylistic note, not sure why I didn't spot this before: if your loop in userNames is actually what you meant it to be, you can write it much more cleanly:

while (lastN == "NULL") {
    std::cout << "Again what is your last name? ";
    std::cin >> lastN; //Once again, this should be checked
}


Simulating Natural Sorting in MySQL with PHP I have a MySQL table which contains product SKUs among other things. PHP powers importing data into this database, managing data through a control panel, displaying the information on the front-end, etc. (For clarity, this is a Magento site.) On the front-end, I need to be able to list SKUs in natural sorting order, however MySQL does not have a natsort method the way PHP does.

There are a number of MySQL only solutions, each with their own problems. The simplest that you see is to cast the string with something like ORDER BY sku + 0 but this only works for the simplest strings. Let's look at more complex options:

MySQL: ORDER BY LENGTH(col), col

The idea here is that test5 is shorter than test10 and will be sorted first. But test10 is alphabetically before test20 so that also is sorted first, giving you test5, test10, test20 which looks correct.

However, this doesn't match PHP's natsort method. For example, test50 will come before test100 but test50a will come after test100 which is incorrect. This is because test50a is longer than test50 and the 5 comes after the 1 when comparing test50a and test100. This should read test50, test50a, test100

PHP: natsort

I have the opportunity for the PHP application (Magento) to create an Index and either totally rebuild it on command, or update it on a SKU-by-SKU basis. At first, my solution was something like this:

// $skus = array() of strings

// Natural sort the SKUs
natsort($skus);

// Reset the keys to be in sequential order by array position
$skus = array_values($skus);

foreach ($skus as $position => $sku) {
     // Assume this is correctly escaped and sent to MySQL
     $sql = "UPDATE database.products SET position = {$position} WHERE sku = '{$sku}'";
}

You could then sort by position.

The problem with this is that any time you change a SKU or add a new SKU you would have to re-sort the entire array again, and update the entire index. (Deleting a SKU wouldn't have an effect.)

PHP: preg_replace_callback + str_pad

I landed on this method:

// $skus = array() of strings

foreach ($skus as $sku) {
     $paddedSku = preg_replace_callback('#\d+#', function($m) {
          return str_pad($m[0], 8, '0', STR_PAD_LEFT);
     }, $sku);

     // Assume this is correctly escaped and sent to MySQL
     $sql = "UPDATE database.products SET padded_sku = '{$paddedSku}' WHERE sku = '{$sku}'";
}

In this case, test50 becomes test00000050 and test50a becomes test00000050a and test100 becomes test00000100. You can then do ORDER BY padded_sku and it will alphanumerically sort correctly. This even works for more complex strings like test50a-abc45

In actuality, I've collected all of these padded SKUs into an array, and then inserted them using a REPLACE (which my indexes in MySQL make possible) in chunks of 500. The padded_sku column can be indexed for speed, and I can insert or change a single SKU at any time without having to rebuild the whole index.

Question

Is there a better way to naturally sort in MySQL that supports test50 coming before test50a coming before test100? I think that with some complex MySQL I could have it doing this preg_replace and str_pad through MySQL functions on the fly, but isn't it faster to calculate this once and store it as a simple string? (Even if it has to go back and forth to PHP to do that?)

Essentially, you want to have a custom, non-trivial sorting order,
based on logic that doesn't exist in the table schema and not supported by standard SQL operators.

The general (and perhaps obvious) solution to this problem seems to be including the necessary data in the table schema explicitly.
In other words, adding a dedicated column to represent the sorting key seems a sensible solution.

Suppose it's possible to use some SQL operations to generate this sorting key on the fly.
The problem with that is that you would have to re-type the (potentially complicated) operation every time you want to select an ordered subset from the table.
An option to make that more workable is embedding the sorting key logic inside a view on the source table.
If this approach is possible,
then the right solution will depend on the impact on performance due to the recalculation of the sorting key column in the view every time you access it.


Getting list of daily team goals The old code I had before  was atrociously slow but after some advice and research I was able to take a 2-5 minutes run time down to about 5-30 seconds.  That is acceptable, but still looking to have it run as quickly as possible.

I'm still cleaning it up, like there shouldn't be two seperate PP entities calls.

public static List<DailyTeamGoal> GetListDailyTeamGoals(int teamId)
{
    string teamGoal = "";
    List<ProPit_User> lstProPit_User = new List<ProPit_User>();
    using (PPEntities db = new PPEntities())
    {
        Team team = db.Teams.Where(x => x.teamID == teamId).FirstOrDefault();

        if (team != null)
        {
            teamGoal = Convert.ToString(team.goal);
        }

        lstProPit_User = db.ProPit_User.Where(x => x.teamID == teamId).ToList();
    }

    List<DailyTeamGoal> lstDailyTeamGoal = new List<DailyTeamGoal>();
    using (TEntities db = new TEntities())
    using (PPEntities ppdb = new PPEntities())
    {
        //have to get every day of the month
        DateTime dt = DateTime.Now;
        int days = DateTime.DaysInMonth(dt.Year, dt.Month);
        decimal orderTotal = 0m;
        for (int day = 1; day <= DateTime.Now.Day; day++)
        {
            DailyTeamGoal dtg = new DailyTeamGoal();
            dtg.Date = day.ToString(); //dt.Month + "/" + day; + "/" + dt.Year;
            dtg.TeamGoal = teamGoal;
            //decimal orderTotalRep = 0m;

            DateTime dtStartDate = Convert.ToDateTime(dt.Month + "/" + day + "/" + dt.Year);
            DateTime dtEndDate = dtStartDate.AddDays(1);

            var ordersQuery = db.Orders.Where(o => o.DateCompleted >= dtStartDate
                                                && o.DateCompleted <= dtEndDate
                                                && (o.Status == 1 || o.Status == 2)
                                                && o.Kiosk != 0).ToList();

            var lstSalesRepIDs = ppdb.ProPit_User
                .Where(x => x.teamID == teamId).Select(v => v.SalesRepID).ToList();

            var total = (from o in ordersQuery
                            where lstSalesRepIDs.Contains(o.SalesRepID)
                            select o.OrderTotal).Sum();

            orderTotal += total;
            dtg.DailyTotal = orderTotal;

            lstDailyTeamGoal.Add(dtg);
        }
    }

    return lstDailyTeamGoal;
}

You are calling queries in a for-loop for each day. Database queries are slow compared to C# code execution. Send only one query to the DB for the whole range of days.

You can do the grouping by day either as DB query or query the ungrouped records and group using LINQ-to-Objects later.

Also you are querying the lstSalesRepIDs within the loop. Why? The teamId seems not to change between the loop iterations. Query it before the loop.

Also your where-clause tests the days like this: o => o.DateCompleted >= dtStartDate && o.DateCompleted <= dtEndDate. Where dtEndDate is one day more than dtStartDate. It looks to me as if you were taking two days at a time. Is this intended? If you want to include all times of a single day, change the comparison to o.DateCompleted >= dtStartDate && o.DateCompleted < dtEndDate. (Note: I used < instead of <= for the upper bound.)

Don't make date calculations by converting from DateTime to string and back! DateTime contains all the functionality you need, without using any dirty tricks.

var salesRepIDs = new HashSet<int>(ppdb.ProPit_User
    .Where(ppu => ppu.teamID == teamId)
    .Select(ppu => ppu.SalesRepID)
    .AsEnumerable()); // HashSet.Contains is faster than List.Contains.

DateTime today = DateTime.Today;
DateTime startDate = new DateTime(today.Year, today.Month, 1);
DateTime endDate = today.AddDays(1);

// Note: If the DB contains no entries with future dates,
//       you can drop the test for the upper date!
var orders = db.Orders
    .Where(o => o.DateCompleted >= startDate
                && o.DateCompleted < endDate
                && (o.Status == 1 || o.Status == 2)
                && o.Kiosk != 0)
    .AsEnumerable();

// Because of AsEnumerable() we are working with LINQ-to-Objects now.
var ordersByCompletionDate = orders
    .Where(o => salesRepIDs.Contains(o.SalesRepID))
    .GroupBy(o => o.DateCompleted.Date); // .Date in order to drop the time part.
foreach (var dayGroup in ordersByCompletionDate) {
    decimal daylyTotal = dayGroup.Sum(o => o.OrderTotal);
    // ...
}

.AsEnumerable() has the advantage over .ToList() that the source is consumed on the fly, item by item. No in-memory list needs to be created.

The salesRepIDs.Contains(..) part requires special attention. It could be performed as part of the DB query; however depending on the implementation and the number of salesRepIDs this could create queries with long IN lists: WHERE id IN (1, 2, 3, ..). There is a limit for the length of these lists. This can lead to "Query too complex" exceptions. If this is not the case, i.e., if you know that you will always have a reasonable number of ID's, could can simplify the whole code and do everything (filtering, grouping and summing) with only one single DB query.


Python requests downloading big files in slower speed than wget This is a simple toy downloader using python's requests library.
I’ve monitored the download process is slower on an ethernet connected box. The realtime speed is measured both on stdout, using get_net_speed() function, and conky.

I’m using a 4Mbps connection.

When downloading a file using wget, the downloading gets full bandwidth as expected. But when using my own downloader script, it gets highest 1Mbps speed.
Here's the code. Review and optimisation are warmly welcome.

#!/usr/bin/env python3

### get details of a requested file and download if a media one

import requests
import sys
from urllib.parse import urlparse
import socket
import re
import time
import psutil

#===================== Main Routine ===================================

def down_in_chunks(resp,file_name,conSize):
    downloaded = 0
    file_descriptor = open(file_name,'wb')
    chunk_size = 1024 * 1024 # 1MB
    for chunk in resp.iter_content(chunk_size=chunk_size):
        downloaded += int(len(chunk)) # get update how much downloaded
        per_cent = 100 * downloaded / int(conSize)
        speed = get_net_speed()
        print("downloading %.2f pc speed %s" %(per_cent,speed),end='\r')
        file_descriptor.write(chunk)
    file_descriptor.close()

#================== Auxilary Routines ==================================

def get_net_speed():
    rcv = psutil.net_io_counters()
    sp0 = rcv[1]
    time.sleep(1)
    rcv = psutil.net_io_counters()
    sp1 = rcv[1]
    speed = sp1 - sp0
    return convert_size(speed)

def convert_size(file_size):
    for unit in ["B","KiB","MiB","GiB","Tib","PiB","EiB","Zib"]:
        if abs(file_size) <= 1024.0 or abs(file_size) == 0:
            return "%3.1f%s" %(file_size,unit)
        else:
            file_size /= 1024.0
        #return "%3.1f%s" %(file_size,unit)

def get_file_details(resp,url):

    file_type = resp.headers["Content-Type"]

    if re.match(r'^html|text',file_type): # perhaps we're not interested in the page only
        print("header info... ", resp.headers)

    else:
        file_name = url.split("/")[-1]
        print("file name: %s" % file_name)
        conDate = resp.headers["Date"]
        conType = resp.headers["Content-Type"]
        conTn = resp.headers["Connection"]
        conSize = resp.headers["Content-Length"]
        conUnit = resp.headers["Accept-Ranges"]
        humane = convert_size(int(conSize))
        print("time: %s type: %s size: %s (%s) unit: %s connection: %s " %(conDate,conType,conSize,humane,conUnit,conTn))
        return file_name , conSize

# ========== Action ===========

try:

    url = sys.argv[1] # get the url
    urlObj = urlparse(url)
    addr = urlObj.netloc
    ip = socket.gethostbyname(addr.split(":")[0]) # get the ip address
    print("connecting to %s | %s |... " %(addr,ip))
    user_agent = {'User-Agent': 'Mozilla/5.0 (Windows; U; Windows NT 5.1; hu-HU; rv:1.7.8) Gecko/20050511 Firefox/1.0.4'}
    resp = requests.get(sys.argv[1],stream=True,timeout=15,headers=user_agent) # I'm not a bot Maa'm
    print("status: %s " %(resp.status_code))
    file_name,file_size = get_file_details(resp,url)

    print("downloading.......... %s" %file_name)
    start = time.time()
    down_in_chunks(resp,file_name,file_size)
    end = time.time()
    total_time = end - start
    if total_time >= 60:
        print("download time: %.2f min" %(total_time/60))

    else:
        print("download time: %.2f secs")
    resp.close()

except requests.exceptions.RequestException as E:
    print("Error: ", E)
    sys.exit(0)

By using time.sleep(1) on get_net_speed you are limiting your download speed to the block size per second, since your block has 1MB you are limited to 1Mbps.


Logic/Presentation separation I have the following code:

 Visitors

            <?php while($visitor = $visitors->fetch_object()) { ?>
            <?php $time = $visitor->time; ?>
            <?php $visitor = new User($visitor->viewer_id); ?>
            <?php
            $this->insert('visitors/visitor',[
            'id' => $visitor->id,
            'profile_image' => $visitor->profile_image,
            'name' => $visitor->name,
            'time' => $time->convertToAgo($time);
            ]);
            ?>
            <? } ?>

I've been trying to think of a way to better separate the logic from the presentation. I'm already using a template engine (Plates, native php templating), this code is from one of my template files. However, as it stands, it won't easy for anyone who has no understanding of PHP to edit my template file without getting confused with all of the PHP code.

Can anyone help me with ideas on the way to do this logic/presentation separation?

<?php
    // this code goes into your controller
    $visitor_rows = [];

    // lets do all the hard work here
    while($visitor = $visitors->fetch_object()) {
        $time = $visitor->time;
        $visitor = new User($visitor->viewer_id);
        $visitor_rows[] = [
            'id' => $visitor->id,
            'profile_image' => $visitor->profile_image,
            'name' => $visitor->name,
            'time' => $time->convertToAgo($time),
        ]);
    }

    // the template only needs to know about $visitor_rows now
?>

 Visitors

                <?php foreach ($visitor_rows as $row): ?>
                    <?= $this->insert('visitors/visitor', $row) ?>
                <?php endforeach ?>


(Leetcode) Valid parentheses This is a Leetcode problem -

  Given a string containing just the characters '(', ')', '{', '}', '[' and ']', determine if the input string is valid.

  An input string is valid if -

  Open brackets must be closed by the same type of brackets.
  Open brackets must be closed in the correct order.

  Note that an empty string is also considered valid.

  Example 1 -

Input: "()"
Output: True

  Example 2 -

Input: "()[]{}"
Output: True

  Example 3 -

Input: "(]"
Output: False

  Example 4 -

Input: "([)]"
Output: False

  Example 5 -

Input: "{[]}"
Output: True

Here is my solution to this challenge -

def is_valid(s):
    if len(s) == 0:
        return True
    parentheses = ['()', '[]', '{}']
    flag = False
    while len(s) > 0:
        i = 0
        while i < 3:
            if parentheses[i] in s:
                s = s.replace(parentheses[i], '')
                i = 0
                flag = True
            else:
                i += 1
        if len(s) == 0:
            return True
        else:
            flag = False
            break
    return False

So I would like to know whether I could improve performance and make my code shorter.

Not a performance suggestion, but you can make use of the fact that an empty collection is Falsey.

if len(s) == 0:

Is functionally the same as just:

if not s:

And similarly,

while len(s) > 0:

Can be just:

while s:

Relevant PEP entry (search for "For sequences" under the linked heading).


Credit card verification: string conversion most optimal? Below is some code which verifies a credit card number using the checksum as well as check if number of digits are appropriate as well if digits start with right numbers. I am not sure if converting the double into a string was the best bet. I wasn't going to at first but had trouble figuring out the modulo math to get every second digit without knowing the length of the double (# of digits).

Also, in my use of strtok(), what should I be doing with the balance of the string after the delimiter? Is that hanging out in memory somewhere?

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

int main(void)
{
    double cardnumber;
    printf("Give me a number: \n");
    scanf("%lf", &cardnumber);

    if(cardnumber < 1000000000000 || cardnumber > 10000000000000000)
    {
        printf("INVALID\n");
        return 0;
    }
    if(cardnumber < 100000000000000 && cardnumber > 9999999999999)
    {
        printf("INVALID\n");
        return 0;
    }

    char creditcard[17];
    sprintf(creditcard, "%f", cardnumber);
    char* ptr_cc;
    ptr_cc = strtok(creditcard,".");
    int card_size = strlen(ptr_cc);
    int sum = 0;
    for(int i = 1; i < card_size; i+=2)
    {
        int x = creditcard[card_size-1-i] - '0';
        int prod = 2 * x;
        if(prod>=10)
        {
            prod = prod%10 + prod/prod%10;
        }
        sum += prod;
    }
    for(int i = 0; i < card_size; i+=2)
    {
        int x = creditcard[card_size-1-i] - '0';
        sum += x;
    }
    if(sum%10 != 0)
    {
        printf("INVALID\n");
        return 0;
    }
    else if(creditcard[0] == '4')
    {
        printf("VISA\n");
        return 0;
    }
    else if(creditcard[0] == '3' && (creditcard[1] == '7' || creditcard[1] =='4'))
    {
        printf("AMEX\n");
        return 0;
    }
    else if(creditcard[0] == '5' && (creditcard[1] >='1' && creditcard[1] <='5'))
    {
        printf("MASTERCARD\n");
        return 0;
    }
    else
    {
        printf("INVALID\n");
        return 0;
    }
}

As @Keith says, credit card numbers should be treated as strings, not numbers, and definitely not floating-point numbers.  If you want to ensure that the input contains only digits (and maybe spaces), use strspn().  Squeeze out any spaces, then validate the length using strlen().

The Luhn checksum check should be in its own function.  The loop indexes would be more natural counting down, I think, since you are taking every other digit starting from the right.

int is_valid_luhn(const char *creditcard)
{
    int card_size = strlen(creditcard);
    int sum = 0;
    for(int i = card_size - 2; i >= 0; i -= 2)
    {
        int digit = creditcard[i] - '0';
        int prod = 2 * digit;
        sum += prod / 10 + prod % 10;      /* No special case needed */
    }
    for(int i = card_size - 1; i >= 0; i -= 2)
    {
        sum += creditcard[i] - '0';
    }
    return sum % 10 == 0;
}