Scrabble helper: find the highest score with any 7 letters I'm making a python script that accepts 7 letters and returns the highest scoring word along with all other possible words. At the moment it has a few "loops in loops" and others things that will slow down the process.

import json
#open file and read the words, output as a list

def load_words():
    try:
        filename = "dictionary_2.json"
        with open(filename,"r") as english_dictionary:
            valid_words = json.load(english_dictionary)
            return valid_words

    except Exception as e:
        return str(e)

#make dictionary shorter as there will be maximum 7 letters
def quick():
    s = []
    for word in load_words():
        if len(word)<7:
            s.append(word)
    return s

# takes letters from user and creates all combinations of the letters
def scrabble_input(a):
    l=[]
    for i in range(len(a)):
        if a[i] not in l:
            l.append(a[i])
        for s in scrabble_input(a[:i] + a[i + 1:]):
            if (a[i] + s) not in l:
                l.append(a[i] + s)

    return l

#finds all words that can be made with the input by matching combo's to the dictionary and returns them
def word_check(A):
    words_in_dictionary = quick()
    for word in scrabble_input(A):
        if word in words_in_dictionary:
            yield word

#gives each word a score
def values(input):
    # scrabble values
    score = {"a": 1, "c": 3, "b": 3, "e": 1, "d": 2, "g": 2,
             "f": 4, "i": 1, "h": 4, "k": 5, "j": 8, "m": 3,
             "l": 1, "o": 1, "n": 1, "q": 10, "p": 3, "s": 1,
             "r": 1, "u": 1, "t": 1, "w": 4, "v": 4, "y": 4,
             "x": 8, "z": 10}
    word_total = 0
    for word in word_check(input):
        for i in word:
            word_total = word_total + score[i.lower()]
        yield (word_total, str(word))
        word_total = 0

#prints the tuples that have (scrabble score, word used)
def print_words(a):
    for i in values(a):
        print i

#final line to run, prints answer
def answer(a):
    print ('Your highest score is', max(values(a))[0], ', and below are all possible words:')
    print_words(a)

answer(input("Enter your 7 letters"))

I have removed some of the for loops and have tried to make the json dictionary I found shorter by limiting it to 7 letter words max. I suppose I could do that initially so that it doesn't need to do that each time i run the script. Any other tips on how to speed it up?

Your algorithm appears to be "generate all possible permutations of the tiles and check to see which are legal scrabble words".  That's a very inefficient way to solve the problem.

A better way is to construct a new dictionary which is a map from letter-sorted strings to legal words.  For example, suppose your dictionary is BAD, DAB, OPTS, POTS, STOP. You want to construct a map:

ABD --> { BAD, DAB }
OPST --> { OPTS, POTS, STOP }

Now when you have a rack and you're looking to see what words it forms you sort the rack by letter order first and then you only have to consider the combinations and permutations of letters in alphabetical order to look up which ones are legal scrabble words. That is enormously less work.

How much less work? Suppose you have NVSAIET on your rack and you wish to know all the seven letter bingos. In your original approach you generate the 5040 possible re-orderings of that rack and check them all.  With my approach you sort it to AEINSTV and look that up in your map to discover that the three seven letter words you can make with those letters are NAIVEST NATIVES VAINEST: that's one lookup instead of 5040.

How do you find the sixes? You look up EINSTV, AINSTV, AENSTV, AEISTV, AEINTV, AEINSV and AEINST. Now you are doing seven lookups instead of looking up each of the (again) 5040 six letter permutations from a rack of seven.

You end up doing only 120 lookups total, instead of the tens of thousands of lookups your present approach is doing.

An even better technique though is to construct a trie or a dawg (a Directed Acyclic Word Graph) from your dictionary, and recursively descend through the graph to discover what words can be made from the rack. Find yourself a good tutorial on tries, and learn how they work. Once you've mastered them, move on to dawgs.


Separation of code into functions and use of patern matching in Haskell In the following:

import Data.Bits (popCount, shiftL, (.|.))
import Data.Char (chr, ord)
import Data.Word (Word8)
import qualified Data.ByteString as B

data MByteSequence = MByteSequence [Word8] -- a sequence of map bytes (len 0-15)

data Atom = TerminationAtom |
            NonOffsetAtom { nonOffsetGapLength :: Integer -- the number of gap bytes (max 2**53-1)
                          , nonOffsetGapValue :: Bool -- the value of the gap bytes
                          , followingMapBytes :: MByteSequence -- a sequence of following map bytes (len 1-15)
                          } |
            OffsetAtom { offsetGapLength :: Integer -- the number of gap bytes (max 2**53-1)
                       , offsetGapValue :: Bool -- the value of the gap bytes and the majority of the offset byte
                       , offsetBit :: Int -- the bit with a different value in the offset byte (0-7)
                       }

getAtomType :: Atom -> Int
getAtomType TerminationAtom = 1
getAtomType (NonOffsetAtom len _ _)
  | len <= 3 = fromInteger len
  | len > 3  = 4
getAtomType (OffsetAtom len val _)
  | len <= 3 && val == False = 5
  | len > 3                  = 6
  | len <= 3 && val == True  = 7

getFillField :: Atom -> Int
getFillField TerminationAtom = 0
getFillField (NonOffsetAtom _ val _) = if val then 1 else 0
getFillField (OffsetAtom len val _) = if len > 3 then (if val then 1 else 0) else fromInteger len

getDataField :: Atom -> Int
getDataField TerminationAtom = 0
getDataField (NonOffsetAtom _ val (MByteSequence mbytes))
  | length mbytes == 1 &&
    head mbytes == (if val then 0 else 0xFF) = 0
  | otherwise                                = length mbytes
getDataField (OffsetAtom _ _ bit) = bit

serialize :: Atom -> Word8
serialize TerminationAtom = 0
serialize (NonOffsetAtom x y z) = fromIntegral $ (getAtomType all) `shiftL` 6 .|.
                                                 (getFillField all) `shiftL` 4 .|.
                                                 (getDataField all)
  where all = NonOffsetAtom x y z
serialize (OffsetAtom x y z) = fromIntegral $ (getAtomType all) `shiftL` 6 .|.
                                              (getFillField all) `shiftL` 3 .|.
                                              (getDataField all)
  where all = OffsetAtom x y z

To me, serialize especially seems pretty ugly; the use of the where clauses and the similarity of the latter two pattern match cases all seems bad to me, yet I'm unsure what should be done instead. The other functions, while split up into logical blocks, seem perhaps too small? Should I use Word8 as the return type of all the functions (the return values are all small enough to fit)?

I'd welcome any feedback on code style, Haskell best practices (in general!), and any design patterns I should be using here!

To me, serialize especially seems pretty ugly; the use of the where clauses and the similarity of the latter two pattern match cases all seems bad to me, yet I'm unsure what should be done instead.

You can rewrite the serialize function using as-patterns which will eliminate the where clause like so.

serialize :: Atom -> Word8
serialize TerminationAtom = 0
serialize all@(NonOffsetAtom _ _ _) = fromIntegral $ (getAtomType all) `shiftL` 6 .|.
                                                     (getFillField all) `shiftL` 4 .|.
                                                     (getDataField all)
serialize all@(OffsetAtom _ _ _)    = fromIntegral $ (getAtomType all) `shiftL` 6 .|.
                                                     (getFillField all) `shiftL` 3 .|.
                                                     (getDataField all)

You could eliminate the duplication further by extracting the body of the function into another function parameterized over the shift value. Something like this.

serialize all@(NonOffsetAtom _ _ _) = shifty 4 all
serialize all@(OffsetAtom _ _ _)    = shifty 3 all

shifty n a = fromIntegral $ (getAtomType a) `shiftL` 6 .|.
                            (getFillField a) `shiftL` n .|.
                            (getDataField a)

  Should I use Word8 as the return type of all the functions (the return values are all small enough to fit)?

Yes. If your functions logically only operate on 8-bit byte values, then that's an important fact you can and should capture in the type signature. If returning a value of 256 or 500000 or -1 would be absurd, then eliminate the possibility of that even happening!

In General

There are other odds and ends that could be changed or cleaned up, like comparing Bool arguments to True or False instead of using them directly (getAtomType) or using ifs instead of more pattern matching (getFillField (NonOffsetAtom _ True _) = ...).

Update

Having slept on it and cleared my head, I think your code would be much improved by leveraging ADTs and a serialization typeclass. As it stands your solution feels very C-like, and your data types are used like structs, arrays and low-level machine ints instead of rich data. I'll sketch an outline of what I would do here, please feel welcome to interrogate me in the comments if the full picture doesn't come through for you.

data Four = Zero | One | Two | Three

data AtomType = ShortBase   Four | LongBase
              | ShortOffset Bool | LongOffset

newtype FillType = Fill Four

data DataField = ...

data Word = Terminal
          | Word AtomType FillType DataField

class Serialize a where
    serialize :: a -> Word8

instance Serialize Four where
    serialize Zero  = 0
    serialize One   = 1
    serialize Two   = 2
    serialize Three = 3

instance Serialize AtomType where
    serialize (ShortBase n)       = serialize n `shiftL` 6
    serialize LongBase            = 4 `shiftL` 6
    serialize (ShortOffset False) = 5 `shiftL` 6
    serialize (ShortOffset True)  = 7 `shiftL` 6
    serialize LongOffset          = 6 `shiftL` 6

...

instance Serialize Word where
    serialize Terminal     = 0
    serialize (Word t f d) = serialize t .|. serialize f .|. serialize d

My goal here was to make everything as correct-by-construction as possible (q.v. the Four type) and to reduce the opportunities for ambiguity (e.g., a "Fill Field" may mean any number of things if all you're looking at is a literal 0, so the representation should be as precise as necessary and no more).

On the benefits of this approach

If you can determine the names based on the problem domain, this will be extremely readable
Requires less dead code (In particular I'm looking at the support for TerminationAtom everywhere)
Easily made symmetrical with a Deserialize class

Intermediary representations of data can always slow down your code, but with aggressive inlining I don't think this would be too much of a performance hit.


Duplicating an array This code returns an array that consists of two copies of the input array.

Can you review this algorithm?

//input: [1, 2, 3]  output: [1, 2, 3, 1, 2, 3]
function duplicate(arr) {
 let copy = [];
 for(let i = 0; i < arr.length; i++) {
  copy.push(arr[i])
  }
 return copy.concat(arr)
 }

The alternative approaches in the comments return arr.concat(arr); and return [...arr, ...arr]; are good replacements.

However, if you wrote this as part of a course, you should consider the following:

Your indentation is off, this is rule 1 in writing code: indent it
You are missing a semicolon after copy.push(arr[i])
In such a small function, I would have declared i up top in that let statement


Counting the longest increasing subsequence in an array of integers This is my finished function to find the length of the longest consecutive subsequence of an input array of integers. It's for an assessment so things like the numerical method used and the format of input are out of my hands (int LIS(int* seq, int* temp_seq, int seq_size)).  For the temp_seq parameter, the caller will pass an empty array of size seq_size.

As far as I can tell works perfectly, however, our lecturer has barely commented on how to write code that doesn't just do the job but is 'good code' (aka a large amount of the marking criteria).

Is there anything that you can see which could be improved here?

In the past I've lost marks for code that did the job, but didn't follow 'good practice' standards. I've tried to make this code as nice as I can, with a clear logical structure, good commenting etc but I always thought my code was good then because when I tested it it worked. I'm just looking for advice on the form of the code.

int LIS(int* seq, int* temp_seq, int seq_size)
{
    /*sets up variables and initial values*/
    int i, proceed_flag;
    int j = 0, count = -1;

    /*this'll be used to ensure each input element is only used once - 1 means not used*/
    int use_flag [seq_size];
    for (i = 0; i<seq_size; i++)
    {
        use_flag[i] = 1;
    }

    /*while the temp array isn't full*/
    while(j<seq_size)
    {
        i = 0;
        proceed_flag = 0;

        /*drops the next unused number into the temp array*/
        while(proceed_flag == 0)
        {
            if (use_flag[i] == 1)
            {
                temp_seq[j+1] = seq[i];
                use_flag[i] = 0;
//              printf("temp_seq = %d\n", temp_seq[j+1]);
                j++;

                proceed_flag = 1;
            }

            i++;
        }

        /*then drops all smaller numbers from the input array into the temp array*/
        for(i = 0; i<seq_size; i++)
        {
            if(temp_seq[j] > seq[i] && use_flag[i])
            {
                temp_seq[j+1] = seq[i];
                use_flag[i] = 0;
//              printf("temp_seq = %d\n", temp_seq[j+1]);
                j++;
            }
        }

        /*this counts the number of repeats - the initialisation value of -1 is just to calibrate the system
        it takes an extra loop around and gives a wrong result otherwise*/
        count++;
    }

return(count);
}

Here is one comment about the readability of your code.

/* 1 means not used */
int used_flag[seq_size];

I allowed myself to change the "use" to "used", because it is more grammatically correct.

Now first of all, if "1 means not used", then you should name this array not_used_flag.

Alternatively, you can keep calling it used_flag, but let 0 denote the case of "not used".

If you choose the second option, then you can initialize the entire array upon declaration:

int used_flag[seq_size] = {0};

Please note that it doesn't yield better performance because it's a non-static local array.

Nevertheless, it does clean up your code a bit...

Another point is implied in a comment at the bottom of your code:

/* the initialization value of -1 is just to calibrate the system.
   it takes an extra loop around and gives a wrong result otherwise. */

It sounds like an ugly workaround for "an extra loop" that should not take place.

So I think that you definitely need to solve it properly instead of working around it.

Last issue (though possibly related to the previous one):

while (j < seq_size)
{
    ...
    temp_seq[j+1] = ...;
}

By entering the loop with j == seq_size-1, you'll be writing beyond the boundaries of temp_seq.

Your code might be working correctly for some input, but beware that it does look suspiciously unsafe.


Find primes using Sieve of Eratosthenes with Python I was Googling for a 'correct' or 'efficient' implementation of the Sieve of Eratosthenes. I found them all more complicated than what I came up with. I am by no means a Python expert, so I was wondering if something is wrong with my code.

def get_primes(max_int):
    numbers = range(2, max_int)
    while len(numbers) > 0:
        numbers = [ num for num in numbers if num % numbers[0] != 0 or num == numbers[0] ] # only keep numbers that are NOT divisible by the prime
        yield numbers[0] # yield the prime
        numbers = numbers[1:] # make a new numbers-array, the first num is a new prime

Call it with

primes = get_primes(1000)

And it gives you a generator of primes up until 1000.

Your code looks correct to me, it does produce prime numbers
in the given range.

PEP8 Online reports some code style violations
with respect to whitespace and line length, but it is not too bad.

But: This is not the Sieve of Eratosthenes.
The Sieve of Eratosthenes keeps a (single) list of all candidate
numbers, and computes multiples
of each found prime to mark subsequent composite numbers in the list.
Your algorithm computes the remainder of all remaining candidates instead, and creates new lists in each step.

As a simple benchmark I ran

print(sum(p for p in get_primes(100000)))

with your code, this takes approximately 5 seconds on my MacBook.

It can be improved slightly by filtering the list only once in
each step and not twice:

def get_primes(max_int):
    numbers = range(2, max_int)
    while len(numbers) > 0:
        yield numbers[0]
        numbers = [num for num in numbers if num % numbers[0] != 0]

This reduces the time to 4.6 seconds.

But any “real” Eratosthenes sieve is much faster. As an example,
with this one
from Rosetta code

def primes_upto(limit):
    is_prime = [False] * 2 + [True] * (limit - 1)
    for n in range(int(limit**0.5 + 1.5)): # stop at ``sqrt(limit)``
        if is_prime[n]:
            for i in range(n*n, limit+1, n):
                is_prime[i] = False
    return [i for i, prime in enumerate(is_prime) if prime]

the above benchmark runs in 0.08 seconds.


Scraper for basketball rosters I am very new to Python and I am trying to learn through personnal projects and today I needed to collect a lot of baksetball player names and decided this was a good time to learn and practice.
The script is able to pull out all of the players names, height, nationality and DOB but I need to enter the link to the pages before launching it.
Next step will be either to build a graphic interface where I can copy/past the links or asking in the console wether I want to enter a new team.
I am looking forward to hearing from you since this is maybe my first personnal project :)
import os
import requests
import pandas as pd
import xlsxwriter
from bs4 import BeautifulSoup

def reverse(name):
    to_reverse = name
    return ' '.join(reversed(name.split(' ')))

def conversion_df(url):
    requete = requests.get(url)
    page = requete.content
    soup = BeautifulSoup(page, features="lxml")

    header = [th.getText() for th in soup.findAll("th")]    # Why do I have to put th. before getText for it to work | Why do we use th as an updator
    header = header[1:5] # Here we exclude the first (O) entry and we display the entries situated before 5

    row = soup.findAll("tr")[1:]
    stats = [[td.getText().strip() for td in row[i]] for i in range(len(row))]
    for j in stats:
        del j[0]
        del j[4]
    #print(stats)

    for i in range(len(stats)):
        stats[i][0] = reverse(stats[i][0])

    table = pd.DataFrame(data = stats, columns = header)
    return table

nanterre = conversion_df("https://www.lnb.fr/fr/espa/equipe/espoirs-nanterre-61344.html")
monaco = conversion_df("https://www.lnb.fr/fr/espa/equipe/espoirs-monaco-61343.html")
boulazac = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-boulazac-61331.html")
boulogne = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-boulogne-levallois-61332.html")
bourg = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-bourg-en-bresse-61333.html")
chalon = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-chalon-saone-61334.html")
cholet = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-cholet-61336.html")
chalon_reims = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-chalons-reims-61335.html")
dijon = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-dijon-61337.html")
gravelines = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-gravelines-dunkerque-61338.html")
le_mans = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-le-mans-61339.html")
le_portel = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-le-portel-61340.html")
orleans = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-orleans-61345.html")
pau = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-pau-lacq-orthez-61346.html")
roanne = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-roanne-61347.html")
strasbourg = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-strasbourg-61348.html")
limoges = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-limoges-61341.html")
lyon = conversion_df("https://www.lnb.fr/fr/espoirs/equipe/espoirs-lyon-villeurbanne-61342.html")

clubs = [boulazac,boulogne,bourg,chalon,cholet,chalon_reims,dijon,gravelines,le_mans,le_portel,limoges,lyon,monaco,nanterre,orleans,pau,roanne,strasbourg]

# Creating Excel Writer Object from Pandas
writer = pd.ExcelWriter('liste_espoir.xlsx',engine='xlsxwriter')
workbook=writer.book
worksheet=workbook.add_worksheet('Espoirs')
writer.sheets['Espoirs'] = worksheet

row_count = 0
i = 0

for i in range(len(clubs)):
    clubs[i].to_excel(writer,sheet_name='Espoirs',startrow=row_count , startcol=0)
    row_count += 20

writer.save()

Welcome to Code Review. Your code looks to be following some good practices from python's style guide (PEP-8). However, as a programmer, you can perhaps improve the structure/performance.

No need to have separate variables for dataframes for each club. Group all the links together, and use a map for fetch everything.
The reverse function defines an unused variables to_reverse.
Split functionality to separate function, just like you have one to reverse names.
You do not need to iterate over range(len(some_iterable)) if the index is not needed at all.
Add type hinting, and some comments to functions to help understand what kind of values it might expect (and return), as well as, what it is doing.
Put everything inside an if __name__ == "__main__" clause.

Rewritten code:
from typing import Tuple
import requests
import pandas as pd
import xlsxwriter
from bs4 import BeautifulSoup

def reverse(name: str) -> str:
    """Reverse name from 'Last First' to 'First Last'."""
    return " ".join(reversed(name.split(" ")))

def fetch_page(url: str) -> str:
    """Send request to given url, and return the contents on success."""
    response = requests.get(url)
    if response.ok:
        return response.content

def get_club_information(club_page) -> Tuple:
    """Read all `tr` elements on page, and extract player information.

    Each row (`tr`) consists of 6 cells. We are skipping over 1st and last cell data."""
    soup = BeautifulSoup(club_page, features="lxml")
    rows = soup.findAll("tr")
    header, *content = [[cell.getText().strip() for cell in row][1:5] for row in rows]
    for row in content:
        row[0] = reverse(row[0])
    return header, content

def conversion_df(url):
    page = fetch_page(url)
    header, content = get_club_information(page)
    print(header, content)
    table = pd.DataFrame(data=content, columns=header)
    return table

LINKS = (
    "https://www.lnb.fr/fr/espa/equipe/espoirs-nanterre-61344.html",
    "https://www.lnb.fr/fr/espa/equipe/espoirs-monaco-61343.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-boulazac-61331.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-boulogne-levallois-61332.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-bourg-en-bresse-61333.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-chalon-saone-61334.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-cholet-61336.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-chalons-reims-61335.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-dijon-61337.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-gravelines-dunkerque-61338.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-le-mans-61339.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-le-portel-61340.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-orleans-61345.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-pau-lacq-orthez-61346.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-roanne-61347.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-strasbourg-61348.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-limoges-61341.html",
    "https://www.lnb.fr/fr/espoirs/equipe/espoirs-lyon-villeurbanne-61342.html",
)

def main():
    # Creating Excel Writer Object from Pandas
    writer = pd.ExcelWriter("liste_espoir.xlsx", engine="xlsxwriter")
    workbook = writer.book
    writer.sheets["Espoirs"] = workbook.add_worksheet("Espoirs")
    row_count = 0
    for club in map(conversion_df, LINKS):
        club.to_excel(writer, sheet_name="Espoirs", startrow=row_count, startcol=0)
        row_count += 20
    writer.save()

if __name__ == "__main__":
    main()

Additionally, you can use asyncio to fetch those pages in parallel, to reduce the runtime for your program.


Stack class in Ruby to calculate maximum number, get the average, pop, push Is the class optimized enough to accept 1 million items in the stack?

class Stack

attr_accessor :stacks

def initialize()
    @stacks = []
end

def max ()
    #first we could Think by
    #sort the stack
    #then take the last element
    #but I think it is not efficent
    if @stacks.size == 0
        return puts"Error no input"
    elsif @stacks.size == 1
        @stacks.first
    else
    return @stacks.max
    end
end

def push(value)
    @stacks.push(value)
end

def pop()
    if @stacks.size == 0
        return puts"Error no input"
    else
        @stacks.pop
    end
    return @stacks
end

end

class Extras < Stack

def average()
    if @stacks.size == 0
        return puts"Error no input"
    elsif @stacks.size == 1
        return @stacks.first
    else
        #used inject instead of loop to get more familiar with ruby
        sum = @stacks.inject(0, :+)
        lenght = @stacks.size
        averages = sum / lenght
        return averages
    end
end
end

users = Extras.new
puts users.push(1)
puts users.push(5)
puts users.push(3)
users.pop
puts users.push(10)
puts users.average

Your code here is really just wrapping the Array class from the ruby library.  As such, your performance will be exactly the same as just using Array itself.  The array can hold as many items as you can hold in RAM and the performance will be determined by whatever machine you are running on.

The ruby array class is written in C and performs fairly well.  You ask about optimization but you don't given any criteria, so I can't really speculate beyond that.  If you need something that is extremely fast or needs to scale across millions or billions of items, you may want to look at a different language or even a database.

Looking at your code, you have several instances of the pattern if-elsif-else.  The last two are redundant and can be combined.  For example, the max of an array with one item will be the one item, you don't have to break it out.  Likewise for doing the average; you can average a list with one item and you will still get the correct answer

def max ()
    # returning 'puts' is _very_ unusual.  It would be better to return nil, which Array#max would do anyway
    return puts "Error no input" if @stacks.empty?
    @stacks.max     # the last expression is returned automatically
end

def average()
    return puts"Error no input" if @stacks.empty?

   sum = @stacks.inject(0, :+)
   sum / @stacks.size # note that if you sum an array of integers this way, the final average will also be an integer unless you convert something to a float first
end


Basic sign-up method, testable and with try/catch I am learning how to implement testing and try/catch statements into my code. I have a Laravel application with the following method. I am looking for advice on where I should add in try/catch statements and what phpunit unit-test(s) I would create to ensure this is functioning properly. Tips on comment blocks are also appreciated.

// User sign up
public function postSignup()
{
    $validator = Validator::make(Input::all(), User::$rules);

    $confirmation_code = MyHelper::encrypt_decrypt('encrypt',Input::get('email'));

    if($validator->passes()) {
        $user = new User;
        $user->email = Input::get('email');
        $user->password = Hash::make(Input::get('password'));
        $user->emailVerifiedToken = $confirmation_code;
        $user->firstName = null;
        $user->lastName = null;
        $user->shipAddressId = null;
        $user->billAddressId = null;
        $user->phone = null;
        $user->save();

        $userdata = array(
            'token' => $confirmation_code
        );

        //Send the user an email requesting verification
        $toEmail = $user->email;
        $toName = $user->email;
        $subject = "Email Verification - example.com";

        Mail::queue('emails.account.account-creation', $userdata, function($message) use ($toEmail,$toName,$subject)
        {
            $message->to($toEmail,$toName)->subject($subject);
        });

        if(Auth::attempt(array('email' => Input::get('email'), 'password' => Input::get('password')), false))
        {
            return Redirect::to('/home/')
            ->with('login_message', 'Thank you for creating a new account, please sign in');
        }
    }

    return Redirect::to('/account/sign-up')
            ->with('signup_message',  'Something went wrong')
            ->withErrors($validator)
            ->withInput();
}

For try/catch, statements you should ask yourself: what are you trying to achieve? Do you need to handle the error differently in one controller compared to the rest of the application? If yes, then wrap the entire contents of your method.

If no, Laravel implements a global error handler. You simply need to throw your exceptions and Laravel handles the display of the errors.

As for you unit testing, I am only learning PHPUnit at the moment. Judging by your code, I believe you should take some particular emphasis on learning the S.O.L.I.D principles.

When you abide by these principles you will ensure your code is testable. To be testable your methods and objects need to have a single responsibility, such as taking requests (controller), validation (Request objects in Laravel and saving data (Models).

As for Doc blocks, refer to something like this.

Comment you code often. For methods I always specify in the first line a description of what the method does. (it should have a single responsibility).
Then any parameters/arguments it takes (@param). Then any data it returns (@return). Specify the data type or 'void' if it returns nothing. Then finally any exceptions the method throws.


Simple chart-viewing JavaFX example on Nashorn I'm hacking together some first examples on JavaScript (Nashorn) for viewing a simple chart. Since my background is "Java programmer" I'm sure there are ways where I can have more benefit from JavaScript syntax than I'm using.

I'm open to criticism to get it more shorter, more elegant, more idiomatic than a Java coder writes it down in his first attempts.

load("fx:base.js");
load("fx:controls.js");

var Button = javafx.scene.control.Button;
var StackPane = javafx.scene.layout.StackPane;
var Scene = javafx.scene.Scene;

function start(primaryStage) {
    primaryStage.title = "Hello World!";

    var xAxis = new NumberAxis();
    xAxis.setLabel("Number of Month");
    var yAxis = new NumberAxis();

    var lineChart = new LineChart(xAxis, yAxis);
    lineChart.setTitle("Stock Monitoring 2010");

    var series1 = new XYChart.Series();
    series1.setName("Reference portfolio");

    [new XYChart.Data(1,2),
     new XYChart.Data(2,1),
     new XYChart.Data(3,9),
     new XYChart.Data(4,5),
     new XYChart.Data(5,7),
     new XYChart.Data(6,6)
    ].forEach(function(item){
        series1.data.add(item);
    });

    lineChart.data.add(series1);

    var series2 = new XYChart.Series();
    series2.setName("My portfolio");

    [new XYChart.Data(1,12),
    new XYChart.Data(2,11),
    new XYChart.Data(3,19),
    new XYChart.Data(4,15),
    new XYChart.Data(5,17),
    new XYChart.Data(6,16)].forEach(function(item){
        series2.data.add(item);
    })

    lineChart.data.add(series2);

    var scene = new Scene (lineChart, 800, 600);
    primaryStage.scene = scene;
    primaryStage.show();
}

Interesting question,

from a high level, I think it will be hard to write idiomatic JavaScript using libraries that are straight ports out of Java..

In my mind (meaning, this is not canonical), JS tends to do more during initialization so that this

 var data1 = [
     new XYChart.Data(1,2),
     new XYChart.Data(2,1),
     new XYChart.Data(3,9),
     new XYChart.Data(4,5),
     new XYChart.Data(5,7),
     new XYChart.Data(6,6)
];

 var series1 = new XYChart.Series( "Reference portfolio" , data1 );

seems more natural than this

var series1 = new XYChart.Series();
series1.setName("Reference portfolio");

[new XYChart.Data(1,2),
 new XYChart.Data(2,1),
 new XYChart.Data(3,9),
 new XYChart.Data(4,5),
 new XYChart.Data(5,7),
 new XYChart.Data(6,6)
].forEach(function(item){
    series1.data.add(item);
});

Also, from a var declaration perspective, you either group all your var statements on top or you declare them to where you are going to use them. You have a mixture of both approaches. Especially the declaration of lineChart looks needlessly odd, I would simply deal with lineChart at the very end.

var lineChart = new LineChart(xAxis, yAxis); //Could have been on top
lineChart.setTitle("Stock Monitoring 2010");
lineChart.data.add(series1);
lineChart.data.add(series2);

The same goes for primaryStage, I would have set title in the same space as primaryStage.scene = scene which most JS people would have written as  primaryStage.scene = new Scene (lineChart, 800, 600); if you are going to use the variable only once, and you are not in danger of stretching too much horizontally, just do a direct assignment.

This very much looks like sample code, I think your Java'isms will show more once you get into the nitty gritty. So I am looking forward to you posting some of your real JS code.


Scrape company data from Google search I have written this code for crawling and scraping search results from Google. But it's super slow when I run it.

What it does: read a pandas column, search values in Google and take the first result link then extract the siret number in link
Ex.

  https://www.infogreffe.fr/entreprise-societe/380760702-entreprise-de-decoration-et-d-innovation-sarl-340591B001420000.html
  the siret is 380760702. In every iteration I save this number in found_results.

To do that, there are three functions:

rechercher_google() to get the html
parse_results() to parse the url where the digit is
recherche_google() to wrap to these two functions above.

Could it be less greedy in complexity or be better?

# Function pour recupere la html
USER_AGENT = {'User-Agent':'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36'}

def rechercher_google(search_term, number_results, language_code):
    '''
    input: Nom de l'entreprise à siretiser, le nombre de page et la langue
    output: La page resultat dde google en html stocker au format raw html
    '''
    assert isinstance(search_term, str), 'Il doit etre en string'
    assert isinstance(number_results, int), 'DOit etre en chiffre'
    escaped_search_term = search_term.replace(' ', '+')

    g_url = 'https://www.google.fr/search?q={}&num={}&hl={}'.format(escaped_search_term, number_results, language_code)
    response = requests.get(g_url, headers = USER_AGENT)
    response.raise_for_status()

    return response.text, search_term

# function pour recuperer ce qu'on a besoin dans la page
def parse_results(html, keyword):
    '''
    Input: la sortie de la fortion fetch_results()
    output: le nom de l'entreprise et le siret
    '''
    soup = bs(html, 'html.parser')

    found_results = []
    rank = 1
    result_block = soup.find_all('div', attrs={'class': 'g'}) # g indique q'on est bien dans un resultat normal
    for result in result_block:
        link = result.find('a', href=True)
        title = result.find('h3')
        if link and title:
            link = link['href']
            title = title.get_text()
            if link != '#':
                try:
                    found_results.append({'siret':link.split('/')[4].split('-')[0]})
                except Exception as list_error:
                    print('error in fund_results {}'.format(list_error))

                rank += 1
    return found_results

# wrap up, le tout dans un global function qui appelle les deux premiere.
def scrape_google(search_term, number_results, language_code):
    '''
    input: il mange les deux premier functions
    output: une liste de dict des nom d'entreprise avec leur siret et club
    '''
    try:
        html, keyword = rechercher_google(search_term, number_results, language_code)
        results = parse_results(html, keyword)
        return results
    except AssertionError:
        raise Exception("Probleme d'argument")
    except requests.HTTPError:
        raise Exception(" :/ Google a bloqué le ip")
    except requests.RequestException:
        raise Exception(" :/ Probleme de connexion/proxy")

To run it

# Siret à partir de google
start_time = time.time()
data = []
for keyword in liste_partenaire.Partenaire:
    keyword = keyword+ ' ' + 'infogreffe Montpellier'
    name.append(keyword)
    try:
        results = scrape_google(keyword, 1, "fr")
        for result in results:
            data.append(results[0]['siret'])
    except Exception as e:
        print(' :( Error in results:: [ {} ]'. format(e))
    finally:
        time.sleep(11)
print("--- %s seconds d'éxecution ---" % (time.time() - start_time)) # 48 minutes
# save

Your escaping for the search term is not sufficient. Other characters than spaces might be in the user entered string, which also need to be escaped. Luckily, requests can do it all for you:

def rechercher_google(search_term, number_results, language_code):
    '''
    input: Nom de l'entreprise à siretiser, le nombre de page et la langue
    output: La page resultat dde google en html stocker au format raw html
    '''
    assert isinstance(search_term, str), 'Il doit etre en string'
    assert isinstance(number_results, int), 'DOit etre en chiffre'

    g_url = 'https://www.google.fr/search'
    params = {"q": search_term, "num": number_results, "hl": language_code}
    response = requests.get(g_url, params=params, headers=USER_AGENT)
    response.raise_for_status()
    return response.text, search_term

Note that I also followed Python's official style-guide, PEP8, and did not use spaces around the = for keyword arguments.

But the obvious improvement is not to parse the Google search results at all, but directly query the website you are interested in. If I understand it correctly, what you actually want to parse is:

url = "https://www.infogreffe.fr/recherche-entreprise-dirigeants/resultats-entreprise-dirigeants.html"
params = {"ga_cat": "globale", "ga_q"=search_term}
response = requests.get(url, params=params, header=USER_AGENT)

That website is not particularly fast either (some pages take 5s to load). But they will probably have worse bot protection than Google, which is a bit evil, but if your scraping is evil, you should not be doing it in the first place. Always read the terms & conditions of websites you want to scrape, so at least you know what you are doing. The Wikipedia page Search engine scraping has quite a lot of information about what Google is doing to prevent you from scraping their search result page too aggressively. In the same vein, you could try optimizing the wait time between the queries, which is currently 11 seconds. With some trial & error you might find a shorter time that also works.


Graceful way to iterate through two arrays and build a master array? params[:controller_id].each do |cid|
  params[:zone_number].each do |pzn|
    zone_ids << Zone.find_by_controller_id_and_number(cid,pzn).id
  end
end

I have multiple controllers and multiples zone_numbers, and I would like to build a single array of zone_ids by iterating through controller and then zone per controller.

The code I wrote works - but there has to be a more eloquent way!

If you need to do the combination in Ruby (and I don't think you do; read on), you could do something like this, using Array#product

# just extracting these 2 params for clarity
controller_ids = params[:controller_id]
zone_numbers = params[:zone_number]

# product will give you every id/number combo,
# which is then mapped to a Zone record's ID
zone_ids = controller_ids.product(zone_numbers).map do |controller_id, number|
  Zone.find_by_controller_id_and_number(controller_id, number).id
end

However! I'd suggest less Ruby and more SQL for this.

# line-breaks are for clarity
zone_ids = Zone.where(
  'controller_id IN (?) AND number IN (?)', controller_ids, zone_numbers
).pluck(:id)

That should pluck just the id values from the Zones table without loading entire records.

While you can generate an array that contains every combination of controller ID and zone number, you'd still (with your current code) do separate DB queries for each of those combinations. And doing SQL queries in a loop is a sure sign you should look for another approach altogether (namely doing more in the DB layer, if possible).


Read Twitter users from a text file I have this code for reading Twitter users from a text file represented by integers in the format "int (space) int", where the first int is the ID of
the user doing the following and the second int is the user being followed. It works for small files but takes a really long time with larger files. What can I do to make it more efficient?

users is an ArrayList

public void open() {
    try {
        if (file.exists()) {
            FileReader fileReader = new FileReader(file);
            BufferedReader bufferedReader = new BufferedReader(fileReader);
            String line;
            String[] strings;
            while ((line = bufferedReader.readLine()) != null) {
                strings = line.split(" ");
                String user1 = strings[0];
                String user2 = strings[1];

                if (!users.toString().contains(user1)) {
                    TwitterUser tUser1 = new TwitterUser(
                            Integer.parseInt(user1));
                    users.add(tUser1);
                    TwitterUser tUser2 = new TwitterUser(
                            Integer.parseInt(user2));
                    tUser1.follow(tUser2);
                } else {
                    TwitterUser target = null;
                    for (TwitterUser i : users)
                        if (i.toString().equals(user1))
                            target = i;
                    TwitterUser tUser3 = new TwitterUser(
                            Integer.parseInt(user2));
                    target.follow(tUser3);

                }

            }
            fileReader.close();
        }

    } catch (IOException e) {
        e.printStackTrace();
    }
}

users.toString().contains(user1)

This is not a very optimal way of checking if a user from the file is in your users object. For starters, ArrayList.toString() will generate the String representation every time, and you may run into false positives. For example, if the toString() representation of a TwitterUser contains a comma, which is the delimiter used by ArrayList:

String user1 = user1.toString(); // = "John, Doe"
String user2 = user2.toString(); // = "Jane, Doe"
// Just to make sure we are using the ArrayList implementation
List<String> users = new ArrayList<>(Arrays.asList(user1, user2));
String allUsers = users.toString(); // = "[John, Doe, Jane, Doe]"
allUsers.contains("Doe, Jane"); // = true

Sure, you are actually comparing numeric values, but consider this then:

String user11 = user11.toString(); // = "11"
String user22 = user22.toString(); // = "22"
// Just to make sure we are using the ArrayList implementation
List<String> users = new ArrayList<>(Arrays.asList(user11, user22));
String allUsers = users.toString(); // = "[11, 22]"
allUsers.contains("1"); // = true

The point here is that ideally, your TwitterUser class should be constructed first so that you can use one of its method to test for equivalence, using equals(Object) for example:

public class TwitterUser {

    // ...

    public boolean equals(Object o) {
        // assuming id is a primitive value
        return o instanceof TwitterUser && ((TwitterUser) o).id == id;
    }
}

Again, you should avoid using toString()-based comparison below for testing equivalence due to the 'false positives' point. Furthermore, you should not skimp on braces ({ ... }), and break early once you have a match:

for (TwitterUser i : users) {
    TwitterUser twitterUser = // ...
    if (i.equals(twitterUser)) {
        target = i;
        break;
    }
}

Hold your horses! If you have implemented TwitterUser.equals() to compare by the ID, you wouldn't even need the for-loop:

TwitterUser user = new TwitterUser(Integer.parseInt(user1));
if (!users.contains(user)) {
    users.add(user);
    user.follow(new TwitterUser(Integer.parseInt(user2)));
} else {
    TwitterUser currentUser = users.get(users.indexOf(user));
    currentUser.follow(new TwitterUser(Integer.parseInt(user2)));
}

To make the performance better, you can consider using a Set instead of a List.

To avoid the huge nested if-block after if (file.exists()), consider return-ing early:

public void open() {
    if (!file.exists()) {
        return;
    }
    try {
        // ...
    }
}

Ideally, the File object should be passed to this method so that the method callers know what exact file is being opened here.

edit: If you claim that the processing still tends to infinity, consider reworking your process:

Read all the lines from the file (even if it runs into the millions, it's just two numbers per line).
Use a number of background threads to process the lines concurrently. You'll need a Runnable/Callable implementation, an ExecutorService and a thread-safe Collection implementation. This is probably more suited for another question/answer though...


AES encryption wrapper After taking into account answers for my questions here and here I created (well may-be) improved version of my wrapper. The key issue was what if an attacker is knowing what is encoded - he might then find the key and encode another messages. So I added XOR before encryption. I also in this version prepend IV to the data as was suggested.

SHA256 on key is only for making sure the key is as long as needed for the AES algorithm, but I know that key should not be plain text but calculated with many iterations to prevent dictionary attack

function aes192ctr_en($data,$key) {
    $iv = mcrypt_create_iv(24,MCRYPT_DEV_URANDOM);
    $xor = mcrypt_create_iv(24,MCRYPT_DEV_URANDOM);
    $key = hash_hmac('sha256',$key,$iv,true);
    $data = $xor.((string)$data ^ (string)str_repeat($xor,(strlen($data)/24)+1));
    $data = hash('md5',$data,true).$data;
    return $iv.mcrypt_encrypt('rijndael-192',$key,$data,'ctr',$iv);
}

function aes192ctr_de($data,$key) {
    $iv = substr($data,0,24);
    $data = substr($data,24);
    $key = hash_hmac('sha256',$key,$iv,true);
    $data = mcrypt_decrypt('rijndael-192',$key,$data,'ctr',$iv);
    $md5 = substr($data,0,16);
    $data = substr($data,16);
    if (hash('md5',$data,true)!==$md5) return false;
    $xor = substr($data,0,24);
    $data = substr($data,24);
    $data = ((string)$data ^ (string)str_repeat($xor,(strlen($data)/24)+1));
    return $data;
}

$encrypted = aes192ctr_en('secret text','password');
echo $encrypted;
echo aes192ctr_de($encrypted,'password');

Another question is: Is CTR mode is okay in this context, would it be better if I use CBC mode instead?

Again, by safe I mean, could an attacker derive the password if he knows which exact text was encrypted, and knows above method. I assume a random and long password here.

Maybe instead of XOR, will it be safer to use random initial data with another run of AES or other simpler algorithm like TEA or trivium?

Your XOR-step is nonsense, it doesn't add any security.

The reason is the way the CTR mode works: It encrypts a counter (thus the name) using the key (and the IV as starting point), and XORs the plaintext with the result.
XORing the plaintext with some other constant before encryption can be undone by XORing it with the same value after encryption.

Still, CTR mode (with or without this XOR) should be enough for protection, as long as your IV is non-repeating. To break it with some known plaintext, you basically have to brute-force encrypt the counter with different keys, until you get the same output as plaintext XOR ciphertext (for one block).

Also, you are using a simple MD5 hash for integrity protection. While MD5 might be fine in this case, it now has the reputation of being broken (i.e. there are quite easy collision attacks), which makes it not safe. Use a MAC algorithm for integrity protection, for example with hash_hmac (and a strong hash function like sha256, which you used before).

Other than that, PHP's rijndael-192 is not one of the AES algorithms. AES is Rijndael with block size of 128 bits (and key sizes of 128, 192 or 256 bits), and you are using an 192-bit block size version here. (This does not necessarily mean it is less secure, you just shouldn't call it AES. It did receive less cryptoanalysis than the variant that got standardized as AES, though.)


Generate a CSV file from a multidimensional array in php I can tell by looking at this code that I've gone about this in the wrong way.  Provided the string is correctly formatted (each index ofCSVData[][], would the following generate a CSV file?

There must be a more elegant way of taking my queries and building a CSV.  Looking at this code kinda makes me sick in the stomach.

class ShippingCSVGenerator
{
    public function generateShippingCSV($invoiceArray)
    {
        $invoiceArray = $this->filterInvoices($invoiceArray);
        if(empty($invoiceArray))
        {
            // no parcels to fetch
            return;
        }
        $dispatchDataRes = \DispatchDataQuery::create()
            ->where('DispatchData.printed = ?', $invoiceArray)
            ->find();

        $index = 0;
        $CSVData = [];
        foreach($dispatchDataRes as $orders)
        {
            $parcelsRes = \ItemsInDispatchQuery::create()
                ->where('ItemsInDispatch.orderNumber = ?', $orders->orderNumber)
                ->find();

            $parcelIndex = 0;
            foreach($parclesRes as $parcel)
            {
                $CSVData[$index][$parcelIndex] =
                    $orders->orderNumber() . ',' .
                    $orders->date() . ',' .
                    $orders->toName() . ',' .
                    $orders->destinationBuilding() . ',' .
                    $orders->destinationStreet() . ',' .
                    $orders->suburb() . ',' .
                    $orders->unused_7() . ',' .
                    $orders->postcode() . ',' .
                    $orders->state() . ',' .
                    $orders->country() . ',' .
                    $orders->email() . ',' .
                    $orders->phone() . ',' .
                    $orders->itemName() . ',' .
                    $orders->itemPrice() . ',' .
                    $orders->instructions() . ',' .
                    $parcel->weight() . ',' .
                    $orders->shippingMethod() . ',' .
                    $parcelIndex . ',' .
                    $orders->SKU() . ',' .
                    $orders->quantity() . ',' .
                    $orders->unused_21() . ',' .
                    $orders->unused_22() . ',' .
                    $orders->unused_23() . ',' .
                    $orders->unused_24() . ',' .
                    $parcel->height() . ',' .
                    $parcel->width() . ',' .
                    $parcel->length() . ',' .
                    $orders->courier() . ',' .
                    $orders->unused_29() . ',' .
                    $orders->unused_30();
                $parcelIndex++;
            }
            $index++;
        }

        header("Content-Type: text/csv");
        header('Content-disposition: attachment;filename=shipping.csv');
        $fp = fopen("php://output", "w");
        fputcsv ($fp, $header, "\t");
        foreach($CSVData as $order)
        {
            foreach($csvOrder as $csvParcel)
            {
                fputcsv($fp, $csvParcel, "\n");
            }
        }
        fclose($fp);
    }

    private function filterInvoices($invoiceArray)
    {
        if(empty($invoiceArray))
        {
            // no parcels to fetch
            return;
        }
        $filteredInvoices = [];
        $dispatchDataRes = \DispatchDataQuery::create()
            ->where('DispatchData.printed = ?', $invoiceArray)
            ->find();
        if($dispatchDataRes->isEmpty())
        {
            // no parcels to fetch
            return;
        }
        else
        {
            foreach($dispatchDataRes as $invoicesToFetch)
            {
                $filteredInvoices = $invoicesToFetch->getInvoiceNumber();
            }
        }
        return $filteredInvoices;
    }
}

Encapsulate the csv line generation in another function.

function GenerateCSVlineFromArray($array){
    $line = "";
    foreach ($array as $value){
        $line .= $value .",";
    }
    //cut last comma
    $line = substr($line, 0, strlen($line) -1);
    return $line;
}

Assuming you have already the right order of values.


Iterating through each grid row and matching elements to largest height I have a grid that lists products. The grid is three columns wide, but it could grow to be four or five columns wide. I've written my script to count the columns. The markup is something like this:

...

...

...

...

...

...

In the project, I can't change the markup. I can however write JavaScript/jQuery any which way I please. What I have is working, but I'm using four loops. Since my JS chops aren't too hot, I'm not convinced that what I have is particularly fast or concise.

jQuery(document).ready(function ($) {
"use strict";

var maxItemHeight,
    firstItem = $('.item.first'),
    gridRow = [],
    i;

firstItem.each(function () {
    gridRow.length = 0;
    maxItemHeight = 0;
    gridRow.push($(this));
    $(this).nextUntil(firstItem).each(function () {
        gridRow.push($(this));
    });
    for (i = 0; i < gridRow.length; i++) {
        maxItemHeight = (gridRow[i].find('h2').outerHeight() > maxItemHeight) ? gridRow[i].find('h2').outerHeight() : maxItemHeight;
    };
    for (i = 0; i < gridRow.length; i++) {
        gridRow[i].find('h2').css('height', maxItemHeight);
    };
});
});

How could this be improved?

Can't say if this is much faster, but it's more concise

jQuery(document).ready(function($) {
    "use strict";

    var firstItems = $('.item.first');

    firstItems.each(function() {
        var $first, row, headings, heights, maxHeight;

        $first = $(this);

        // Get collection of "columns"
        row = $first.add($first.nextUntil(firstItems));

        // Find each column's heading
        headings = row.find("h2");

        // Get the heights of the headings
        heights  = headings.map(function () {
            return $(this).outerHeight();
        }).toArray();

        // Get the maximum
        maxHeight = Math.max.apply(null, heights);

        // Set the heading heights to the maximum
        headings.css("height", maxHeight);
    });
});

--Edit--

In my particular instance, the above script had conflict issues with prototype, since it lives inside a Magento install. I sought help and found this improvement.

/Jezen


Computing pairwise bitwise OR in a list of bit-strings I am trying to solve this problem using Clojure:

  You are given a list of \$N\$ people who are attending ACM-ICPC World
  Finals. Each of them are either well versed in a topic or they are
  not. Find out the maximum number of topics a 2-person team can know.
  And also find out how many teams can know that maximum number of
  topics.

  Input Format:

  The first line contains two integers \$N\$ and \$M\$ separated by a single
  space, where \$N\$ represents the number of people, and M represents the
  number of topics. \$N\$ lines follow. Each line contains a binary string
  of length \$M\$. In this string, 1 indicates that the ith person knows a
  particular topic, and 0 indicates that the ith person does not know
  the topic.

  Output Format:

  On the first line, print the maximum number of topics a 2-people team
  can know.  On the second line, print the number of 2-person teams that
  can know the maximum number of topics.

  Constraints:

  \$2 ≤ N ≤ 500\$

  \$1 ≤ M ≤ 500\$

  Sample Input:

4 5

10101

11100

11010

00101

  Sample Output:

5

2

However, my implementation is too slow to pass all the test cases. What are some suggestions to optimize this implementation? Basically, I have a list of bit-strings, and I am trying to find a faster way to bitwise-OR every pair of strings, and find the maximum number of set bits that could be obtained from ORing a pair.

(let [[N _] (clojure.string/split (read-line) #" ")
      N (Integer/parseInt N)
      strings (vec (repeatedly N read-line))
      known
      (for [i (range N) j (range (inc i) N)
            :let [s1 (strings i)
                  s2 (strings j)]]
        (apply + (map #(if (or (= %1 \1)
                               (= %2 \1))
                         1
                         0)
                      s1
                      s2)))
      maximum (apply max known)]
  (println maximum)
  (println (count (filterv #(= % maximum) known))))

I was able to get it accepted by optimizing the inner loop by using BitSets instead of plain strings. I then just used the BitSet's or and bit counting method (cardinality) to do the work. Here's the resulting code:

(import [java.util BitSet])

(defn string->bitset [str]
  (let [bitset (BitSet. (count str))]
    (doall (map-indexed #(when (= \1 %2)
                           (.set bitset %1)) str))
    bitset))

(defn bitset-or [s1 s2]
  (let [tmp (.clone s1)]
    (.or tmp s2)
    tmp))

(let [[N _] (clojure.string/split (read-line) #" ")
      N (Integer/parseInt N)
      bitsets (mapv #(string->bitset %) (repeatedly N read-line))
      known
      (for [i (range N) j (range (inc i) N)
            :let [s1 (bitsets i)
                  s2 (bitsets j)]]
        (.cardinality (bitset-or s1 s2)))
      maximum (apply max known)]
  (println maximum)
  (println (count (filterv #(= % maximum) known))))


Search for row number of maximum value in matrix column Background

I'm writing code that searches for the row number of the maximum value in a partial column of a matrix.  The partial column corresponds with the non-zero values of an equivalent lower triangular matrix.  That is, starting from the input row index idx through the final row. (This is my own version of subroutine for Gaussian Elimination with Partial Pivoting, for those who are interested).

Basically, the behavior of MaxLocColumnWise is the search of columnwise maximum location in the submatrices of A. Passing by reference, MaxLocColumnWise changes the value of k.

For example, for the following matrix
$$
A = \begin{bmatrix}1 & 2 & 3 \\
4 & 5 & 6 \\
7 & 8 & 9
\end{bmatrix}
$$

when k=0, MaxLocColumnWise(A,k) converts k to the index of maximum element in [1,4,7], which is 2, which is the index of 7 i.e. we get 2 as a result of

    int k = 0;
    `MaxLocColumnWise(A,k)`;
    std::cout << k <<"\n";

when k=1, MaxLocColumnWise(A,k) converts k to the index of maximum element in [5,8], which is 2, which is the index of 8. (This case is highlighted in red here)
when k=2, MaxLocColumnWise(A,k) converts k to the index of maximum element in [9], which is 2, which is the index of 9

Code

Interestingly it turns out that the following code takes quite long time.

void MaxLocColumnWise(Matrix A, int &idx){
    int n = A.size();
    int col = idx;

    double currentAbsMax = abs(A[col][col]);

    for (int i=(col+1); i<n; i++){
        double currentVal = abs(A[i][col]);
        if (currentVal > currentAbsMax){
            currentAbsMax = currentVal;
            idx = i;
        }
    }
}

I have implemented this subroutine to the following Gaussian elimination with partial pivoting routine:

void GaussElimPartialPivot(Matrix& A, Vector& b){
    int n = A.size();

    for (int j=0; j<(n-1); j++){
        int index = j;
        MaxLocColumnWise(A, index);
        SwapMatrixRows(A, j, index);
        SwapVector(b, j, index);

        // main loop
        for (int i=(j+1); i<n; i++){
            double m = A[i][j]/A[j][j];
            b[i] -= m*b[j];
            for(int k=j; k<n; k++){
                A[i][k] -= m*A[j][k];
            }
        }
    }
}

But when A gets large, the program gets slower exactly due to the subroutine MaxLocColumnWise, which was verified from excluding each subroutine in the main code.

But I'm not sure exactly where in MaxLocColumnWise() to blame. Any help will be appreciated.

(An excuse for the exclusion of the code : Matrix is just from typedef std::vector<Vector> Matrix;, and the Vector is typedef std::vector<double> Vector;)

I see a number of things that may help you improve your code.

Pass by const reference where practical

The first argument to MaxLocColumnWise is a Matrix but that causes the entire input matrix to be duplicated.  Better would be to make it const Matrix & because it is not modified and it doesn't need to be duplicated.  This is very likely the crux of your code's performance problem.   On my machine with a matrix of size 1000, that single change drops the execution time down from 3.1 seconds to 11 milliseconds.

Prefer return value over reference

Instead of modifying one of the passed parameters, it's often better to return a value instead.  So in this case, the function would be

int MaxLocColumnWise(const Matrix &A, int idx);

Check parameters before use

If idx is a negative number or beyond the end of the Matrix, your program will invoke undefined behavior and it could crash or worse.  Better would be to verify the value is in a valid range before use.

Use appropriate data types

An index into an array is never negative, so instead of int, I'd recommend using std::size_t.

Provide complete code to reviewers

This is not so much a change to the code as a change in how you present it to other people.  Without the full context of the code and an example of how to use it, it takes more effort for other people to understand your code.  This affects not only code reviews, but also maintenance of the code in the future, by you or by others.  One good way to address that is by the use of comments.  Another good technique is to include test code showing how your code is intended to be used.  I split your code into a header Gauss.h and implementation file Gauss.cpp and then created a test driver.  These are the resulting files, after applying all of the suggestions above:

Gauss.h

#ifndef GAUSS_H
#define GAUSS_H
#include <vector>
typedef std::vector<double> Vector;
typedef std::vector<Vector> Matrix;
std::size_t MaxLocColumnWise(const Matrix &A, std::size_t idx);
void GaussElimPartialPivot(Matrix& A, Vector& b);
#endif // GAUSS_H

Gauss.cpp

#include "Gauss.h"
#include <cmath>

std::size_t MaxLocColumnWise(const Matrix &A, std::size_t idx){
    auto maxIndex{idx};
    const auto col{idx};
    for (double maxValue{-1}; idx < A.size(); ++idx)  {
        auto currentVal{std::abs(A[idx][col])};
        if (currentVal > maxValue){
                maxValue = currentVal;
                maxIndex = idx;
        }
    }
    return maxIndex;
}

void GaussElimPartialPivot(Matrix& A, Vector& b) {
    const std::size_t n{A.size()};
    for (std::size_t j{1}; j < n; ++j) {
        auto maxcol{MaxLocColumnWise(A, j)};
        SwapMatrixRows(A, j, maxcol);
        SwapVector(b, j, maxcol);
        for (std::size_t i{j-1}; i < n; ++i) {
            double m{A[i][j]/A[j][j]};
            b[i] -= m*b[j];
            for (auto k{j}; k < n; ++k){
                A[i][k] -= m*A[j][k];
            }
        }
    }
}

main.cpp

#include "Gauss.h"
#include <iostream>
#include <numeric>

int main() {
    constexpr size_t n{1000};
    Matrix m;
    m.reserve(n);
    int startval{1};
    for (size_t i{0}; i < n; ++i) {
        Vector v(n);
        std::iota(v.begin(), v.end(), startval);
        m.push_back(v);
        startval += n;
    }
    for (int i=0; i < n; ++i) {
        auto max{MaxLocColumnWise(m, i)};
        if (max != n-1) {
            std::cout << "Error:" << i << ", " << MaxLocColumnWise(m, i) << '\n';
        }
    }
}


Calling lodash _.defaults() before Object.defineProperty() In reviewing an older project of mine, I noticed that in a particular class definition I'm doing this:

var defaultOptions = { ... object of default options ... }

var SDKClient = function(options) {
    var self = this

    ...

    _.defaults(self, options, defaultOptions)

    Object.defineProperty(self, 'clientId', {
        enumerable: false
    })

    Object.defineProperty(self, 'clientSecret', {
        enumerable: false
    })
}

If I initialize the client object like this:

var client = new SDKClient({clientId: "x", clientSecret: "y"})

Should I expect there to be problems calling defineProperty after the property has already been merged in with _.defaults()? I don't see any errors or problems thrown when running tests, but this is a large project, and I can't be certain there isn't something obscuring errors. If it's not OK, why is it not OK?

I don't believe so _.defaults. The properties will be set on this but be enumerable. The call to Object.defineProperty  will just set those properties to not enumerable. If the property didn't exist then the value would be set as undefined and default descriptors.

Let's break out only the necessary.

Empty object.

var x = {};
console.log(Object.getOwnPropertyDescriptor(x, 'clientId'));

Object with clientId set.

var x = {
  clientId: 1
};
console.log(Object.getOwnPropertyDescriptor(x, 'clientId'));

Empty object.

var x = Object.defineProperty({}, 'clientId', {
  enumerable: false
});
console.log(Object.getOwnPropertyDescriptor(x, 'clientId'));

Note that the default descriptors are set.

Object with clientId set.

var x = Object.defineProperty({
  clientId: 1
}, 'clientId', {
  enumerable: false
});
console.log(Object.getOwnPropertyDescriptor(x, 'clientId'));

So if anything were to give you a problem then it would be this situation.

var x = Object.defineProperty({}, 'clientId', {
  enumerable: false
});
console.log(Object.getOwnPropertyDescriptor(x, 'clientId'));
x.clientId = 1;
console.log(Object.getOwnPropertyDescriptor(x, 'clientId'));


Draw an outline of an upward-pointing triangle I am beginner in C++ and I was given this assignment by my teacher.  The assignment is to print the following shape:

      *
     * *
    *   *
   *     *
  *       *
 *         *
*************

The following is what I have tried and basically it works, but I doubt my code is not simple, there's another way simpler than that. How can I make my code more simpler? Any hints are appreciated.

#include <iostream>

using namespace std;

int main() {
  /**
      *
     * *
    *   *
   *     *
  *       *
 *         *
*************
  */
  int spaces_before = 6; // spaces before printing the stars
  int spaces_after = 0; // spaces after printing the first star
  int stars = 1;
  for (int i = 1; i <= 7; i++) {
    for (int j = 1; j <= spaces_before; j++) {
      cout << ' ';
    }
    if (i == 1) {
      cout << "*";
    } else if (i != 7) {
      cout << '*';
      for (int j = 1; j <= spaces_after; j++) {
        cout << ' ';
      }
      cout << '*';
    } else {
      for (int j = 1; j <= stars; j++) {
        cout << '*';
      }
    }
    spaces_before--;
    if (i == 1) {
      spaces_after++;
    } else {
      spaces_after += 2;
    }
    cout << '\n';
    stars += 2;
  }
}

First of all, please avoid using namespace std;.  It is considered bad practice and will cause many problems.  See Why is using namespace std; considered bad practice? for more information.

Now let's try to simplify the logic.  The following loop:

for (int j = 1; j <= spaces_before; j++) {
  cout << ' ';
}

is a very verbose way to write

std::cout << std::string(spaces_before, ' ');

(this happened more than one time.)

And you don't need to keep the variables spaces_before, spaces_after, and star.  Just calculate them on demand.  Also, don't include the special cases in the loop:

#include <iostream>
#include <string>

int main()
{
    std::cout << "      *\n";
    for (int i = 0; i < 5; ++i)
        std::cout << std::string(5 - i, ' ') << '*'
                  << std::string(2 * i + 1, ' ') << "*\n";
    std::cout << std::string(13, '*') << '\n';
}

This should be enough since you are a beginner.  After this is done, it would be very nice if you extract the magic numbers into named constants.  And the whole process can be wrapped in a function for reuse:

#include <iostream>
#include <string>

void print(std::ostream& os, int rows)
{
    os << std::string(rows - 1, ' ') << "*\n";
    for (int i = 1; i < rows - 1; ++i)
        os << std::string(rows - i - 1, ' ') << '*'
                  << std::string(2 * i - 1, ' ') << "*\n";
    os << std::string(2 * rows - 1, '*') << '\n';
}

int main()
{
    print(std::cout, 7);
}


Creating a Table with Tkinter I am using a code similar to the following example to create a table with tkinter using Python 3:

Each cell is individually created as a ttk.Label and displayed using .grid(row, column)

Some of my values get updated while the program is running with tk.StringVar().set()

Each value is unique and has it's own variable name count1 count2

This example seems short, but in my program I have a few more rows being displayed and each row has 2 StringVar() that I need to keep updated. Overall, it's a very large for loop because it has to check if it's in the correct row & column combination (row 1 column 0 or row 2 column 0) before creating a StringVar().

Any suggestions on how to reduce the size of my for loop?
Are there better ways of creating tables that need a few values consistently updated?

Thank you for your time and Happy Holidays!
import time
import tkinter as tk
import tkinter.ttk as ttk
from threading import Thread

gui = tk.Tk()
gui.geometry('360x270')
gui.configure(bg='white')

style = ttk.Style()
style.theme_create('custom', settings={
    'header.TLabel': {'configure': {
        'background': 'white',
        'foreground': 'dark green',
        'font': 'Times 16 bold',
        'padding': (10, 0)}},
    'TLabel': {'configure': {'background': 'white', 'font': 'Times 12'}},
    'TFrame': {'configure': {'background': 'white'}}})
style.theme_use('custom')

table_frame = ttk.Frame(gui)
table_frame.pack(pady=(36, 0))

values = [('Count', 'Date', 'Time', 'Phrase'),
          ('5', '12/12/10', '03:15', 'blue car'),
          ('13', '09/09/98', '16:20', 'red door')]

total_rows = len(values)
total_columns = len(values[0])

for i in range(total_rows):
    for j in range(total_columns):
        if i == 0:
            label = ttk.Label(table_frame, text=values[i][j], style='header.TLabel')
            label.grid(row=i, column=j)
        elif i == 1:
            if j == 0:
                count1 = tk.StringVar()
                count1.set(values[i][j])
                label = ttk.Label(table_frame, textvariable=count1)
                label.grid(row=i, column=j)
            else:
                label = ttk.Label(table_frame, text=values[i][j])
                label.grid(row=i, column=j)
        elif i == 2:
            if j == 0:
                count2 = tk.StringVar()
                count2.set(values[i][j])
                label = ttk.Label(table_frame, textvariable=count2)
                label.grid(row=i, column=j)
            else:
                label = ttk.Label(table_frame, text=values[i][j])
                label.grid(row=i, column=j)

def increment_count():
    increment_count.status = 'run'

    while increment_count.status == 'run':
        new_minute1 = int(count1.get()) + 1
        count1.set(str(new_minute1))

        new_minute2 = int(count2.get()) - 1
        count2.set(str(new_minute2))

        time.sleep(1)

Thread(target=increment_count).start()

gui.mainloop()
increment_count.status = 'exit'

As I suspected, there's already a thing for this - a TreeView. Strongly prefer built-in widgets that get the job done, leaving the construction of individual elements as a last resort if the built-in widget doesn't at all meet your needs.
There's plenty of examples on how to use this as a table, but if I were you I'd start with the reference documentation.
Remove the first row of your values and use it as a dedicated heading, instead. Break your i/j loop into two halves - a heading half, and a body half.


Length of the longest common sub sequence bottom up Could I get some feedback on this code? I included a test case as well.

This code computes the longest common sub sequence given paired data, it was not part of any challenge I just did it to learn about dp.

To my understanding it runs in \$\mathcal{O}(m*v)\$ where \$m\$ is length str1 and \$v\$ len str2.

def lcs(word1, word2):

  x = len(word1)
  y = len(word2)
  return _lcs(word1, word2, x, y)

def _lcs(word1, word2, x, y):

  matrix = [[-1]*(x) for val in range (0,y)]

  for i in range(0, y):
    for j in range(0, x):

      if word1[j] == word2[i]:
        if i-1 < 0 or j-1 < 0:
          matrix[i][j] = 1
        else:
          matrix[i][j] = 1 + matrix[i-1][j-1]

      else:
        val1 = 0
        val2 = 0
        if i-1 >= 0:
          val1 = matrix[i-1][j]
        if j-1 >= 0:
          val2 = matrix[i][j-1]
        matrix[i][j] = max(val1,val2)

  return matrix[y-1][x-1]

a = 'ABC'
b = 'ABCD'
print(lcs(a,b))

x and y are terrible variable names.  length1 and length2 would be better.

matrix = [[-1]*(x) for val in range (0,y)]

You are not using val in this list comprehension.  Convention is to use _ for throw-away, variables needed for syntax reasons, but not actually used.

0 is the default minimum in range() statements.  If you loop from 0 to some limit, you don't need to mention the 0; you can just use range(y).

You are never reading the -1 value anywhere.  The value is always overwritten by another value before it is read.  To make this clearer, store None instead of -1 in the matrix you are creating.

matrix = [ [None] * x  for _ in range(y) ]

Using i-1 < 0 is an awkward way of writing i == 0.  Similarly, i-1 >= 0 can be written simply as i > 0, or perhaps even i, since non-zero values are "Truthy".

The following is awkward and hard to understand.  6 statements, 4 assignments, two conditionals.  What does it do?  What does it mean?

    val1 = 0
    val2 = 0
    if i-1 >= 0:
      val1 = matrix[i-1][j]
    if j-1 >= 0:
      val2 = matrix[i][j-1]

Python has a x if cond else y trinary operator, which may help simplify and clarify the code.

    val1 = matrix[i-1][j] if i > 0 else 0
    val2 = matrix[i][j-1] if j > 0 else 0

That a lot more concise.  Two statements which look the similar; the differences should be clear, and it should be easier to understand what those differences mean.

  for i in range(y):
    for j in range(x):

      if word1[j] == word2[i]:
        matrix[i][j] = 1
        if i > 0 and j > 0:
          maxtrix[i][j] += matrix[i-1][j-1]

      else:
        val1 = matrix[i-1][j] if i > 0 else 0
        val2 = matrix[i][j-1] if j > 0 else 0
        matrix[i][j] = max(val1, val2)

The statement return matrix[y-1][x-1] returns the last column of the last row.  You don't actually need to know the dimensions of the matrix for this.  Simply return matrix[-1][-1].

After you generate row 1, you no longer need row 0 of the matrix.  After you generate row 2, you no longer need row 1 of the matrix.  After you generate row 3, you no longer need row 2 of the matrix.  This means you could solve the problem in \$O(m)\$ memory, instead of \$O(m*v)\$ memory, by simply maintaining a prev_row and next_row, instead of an entire matrix.


How to optimize this C# code The below logic is working fine. But is there any way to optimize this logic ?

I have a string1, string2,...,stringn
and values of each strings are

string1 = "s1k1-s1k2-s1k3-....-s1kn | s1v1-s1v2-s1v3-....-s1vn";

string2 = "s2k1-s2k2-s2k3-....-s2kn | s2v1-s2v2-s2v3-....-s2vn";

                .............

stringn = "snk1-snk2-snk3-....-snkn | snv1-snv2-snv3-....-snvn";

Now, I want to populate the Dictionary<string, string>() with the key and values as follows.

var dict = new Dictionary<string, string>();
dict.Add(s1k1, s1v1);
dict.Add(s1k2, s1v2);
         :
         :
dict.Add(snkn, snvn);

Below is my sample code:

        string string1 = "s1k1-s1k2-s1k3-s1kn|s1v1-s1v2-s1v3-s1vn";
        string string2 = "s2k1-s2k2-s2k3-s2kn|s2v1-s2v2-s2v3-s2vn";
        string stringn = "snk1-snk2-snk3-snkn|snv1-snv2-snv3-snvn";

        var dict = new Dictionary<string, string>();

        new List<string> { string1, string2, stringn }.ForEach(str =>
        {
            var strSplit = str.Split('|');
            var strKeys = strSplit[0].Split('-');
            var strValues = strSplit[1].Split('-');
            strKeys.Zip(strValues, (key, value) => new { key, value }).ToList().ForEach(filed => dict.Add(filed.key, filed.value));
        });

Is it possible to optimize this code interms of Speed and Readability.

If you want to optimize for speed and possibly memory, just use regular loops to build up your dictionary, though it may hurt readability.

The idea is simple, scan the string for keys and values simultaneously.  Don't extract substrings until you know what you need.  Scan each token until you hit a delimiter and add the corresponding keys and values to your dictionary.

Assuming none of your keys or values do not contain any of your delimiters and everything is well formed and you have the same amount of values as you do keys for every string.

var myStrings = new[]
{
    "1-2-3-4-5|a-b-c-d-e",
    "10-20-30-40-50|aaa-bbb-ccc-ddd-eee",
};
var delims = new[] { '-', '|' };
var dict = new Dictionary<string, string>();
foreach (var str in myStrings)
{
    var sepIndex = str.IndexOf('|');

    var keyStart = 0;
    var valueStart = sepIndex + 1;
    while (keyStart < sepIndex)
    {
        // scan for the end of the token
        var keyEnd = str.IndexOfAny(delims, keyStart);
        var valueEnd = keyEnd < sepIndex
            ? str.IndexOfAny(delims, valueStart)
            : str.Length;

        // extract and add the tokens
        var key = str.Substring(keyStart, keyEnd - keyStart);
        var value = str.Substring(valueStart, valueEnd - valueStart);
        dict.Add(key, value);

        keyStart = keyEnd + 1;
        valueStart = valueEnd + 1;
    }
}

For readability, a declarative style works well and LINQ is king, just use it better.

var dict =
    (from str in myStrings
    let split = str.Split('|')
    let keys = split[0].Split('-')
    let values = split[1].Split('-')
    from pair in keys.Zip(values, (key, value) => new { key, value })
    select pair).ToDictionary(x => x.key, x => x.value);

As a side note, don't throw things into a list just for the sake of using the ForEach() method, you don't need the overhead of the list just to be able to interate over a collection through that method.


Placing and removing a dynamic tooltip I'm making a calendar and this is one part of the larger project. This adds a tooltip with more information when a calendar event is clicked. It then disappears after 10 seconds, or earlier if a click is detected. The exception being if the click in on the tooltip itself. This code works but feels kind of "cheaty" to me. I wanted a fresh pair of eyes to let me know if there's a better way of doing this all or a specific part I could do differently.

setUpCalendar: function() {
    var self = this; //Refers to the main Calendar Object
    //Render Calendar
    this.elem.fullCalendar({
        height:     this.options.elemHeight,
        events:     this.entries,
        eventClick: function(event, jsEvent, view) {
            var left = parseInt($(this).css("left")) - 4;
            var top =  parseInt($(this).css("top")) - 80;
            self.placeTooltip(left, top, event);
            return false;
        }
    });
},

placeTooltip: function(left, top, event) {
    var toolTip = $(".tooltip"),
        date = $.fullCalendar.formatDate(event.start, "MM dd yyyy hh TT");

    //Format date
    date = date.replace(" ", "/");
    date = date.replace(" ", "/");

    toolTip
        .css({
            "display": "block",
            "position": "absolute",
            "z-index": 10,
            "left": left,
            "top": top
        })
        .html(event.title + "<br />" + date)
        .on("click", function(e) {
            e.stopPropagation(); //Stop click from closing tooltip
        });

    //Remove it later
    $(document).on("click", function() {
        toolTip.hide(); //Hide if clicked anywhere
    });

    if(this.closeTool)
        clearTimeout(this.closeTool);

    this.closeTool = setTimeout(function() {
        toolTip.fadeOut(); //Run this timeout to hide tooltip if not clicked
    }, 10000);
}

You could change the close tooltip fade out to use the jquery delay method (assuming you're using jQuery 1.4+).

Instead of this:

if(this.closeTool)
    clearTimeout(this.closeTool);

this.closeTool = setTimeout(function() {
    toolTip.fadeOut(); //Run this timeout to hide tooltip if not clicked
}, 10000);

You can do this:

if (toolTip.queue().length > 0) {
    toolTip.finish();
} else {
    toolTip.delay(10000).fadeOut();
}

When the user clicks the first time, the effects queue will be empty (length will be 0), so we queue up the delay and the fade out.

When the user clicks the second time, the effects queue length will be more than one, so we use finish to stop the current animation (either the delay or the fade out) and to remove all effects in the effects queue (which will be the delay or both the delay and fade out).


Managing Angular subscriptions There are different methods to get data from server in Angular application:

Get Observable from the service and subscribe to it at component
Create Subject at the service and subscribe to the Subject at component

Both of this methods work for me but I can't understand which should I use.

First method. Get Observable from the service and subscribe to it at component:

article.service.ts

import { Injectable } from '@angular/core';
import { Article } from '../models/article';
import { AngularFirestore } from '@angular/fire/firestore';
import { map, take } from 'rxjs/operators';
import { Observable, Subject } from 'rxjs';

@Injectable({
  providedIn: "root"
})
export class ArticleService {
  public articlesChanged: Subject<Article[]> = new Subject<Article[]>();
  articles: Article[];

  constructor(private db: AngularFirestore) {}

  get() {
    return this.db.collection('articles').valueChanges({ idField: 'id' });
  }
}

home.component.ts

import { Component, OnInit } from '@angular/core';
import { ArticleService } from 'src/app/services/article.service';
import { Observable, Subscription } from 'rxjs';
import { Article } from 'src/app/models/article';

@Component({
  selector: 'app-home',
  templateUrl: './home.component.html',
  styleUrls: ['./home.component.scss']
})

export class HomeComponent implements OnInit {
  articles: Article[];

  constructor(private articlesService: ArticleService) { }

  ngOnInit() {
    this.articlesService.get().subscribe(articles => this.articles = articles as Article[]);
  }
}

Second method. Create Subject at the service and subscribe to the Subject at component:

article.service.ts

import { Injectable } from '@angular/core';
import { Article } from '../models/article';
import { AngularFirestore } from '@angular/fire/firestore';
import { map, take } from 'rxjs/operators';
import { Observable, Subject } from 'rxjs';

@Injectable({
  providedIn: "root"
})
export class ArticleService {
  public articlesChanged: Subject<Article[]> = new Subject<Article[]>();
  articles: Article[];

  constructor(private db: AngularFirestore) {}

  get(): void {
    this.db
      .collection('articles')
      .valueChanges({ idField: 'id' }).subscribe(articles => {
        this.articles = articles as Article[];
        this.articlesChanged.next(this.articles);
      });
  }
}

home.component.ts

import { Component, OnInit, OnDestroy } from '@angular/core';
import { ArticleService } from 'src/app/services/article.service';
import { Observable, Subscription } from 'rxjs';
import { Article } from 'src/app/models/article';

@Component({
  selector: 'app-home',
  templateUrl: './home.component.html',
  styleUrls: ['./home.component.scss']
})

export class HomeComponent implements OnInit, OnDestroy {
  articlesSubscription: Subscription;
  articles: Article[];

  constructor(private articlesService: ArticleService) { }

  ngOnInit() {
    this.articlesSubscription = this.articlesService.articlesChanged.subscribe(articles => this.articles = articles);
    this.articlesService.get();
  }

  ngOnDestroy(): void {
    this.articlesSubscription.unsubscribe();
  }
}

Is there a best practice which I should use?

I think at the end it depends on the use case.
For a simple loading of data, i would use the approach of getting the Observable directly.
When the backendcall is not anymore the single source of changes (for example when i could change the data in the frontend and persist it later), or when i want to "store" the data, so that i do not have to call the backend for each request, then i would start thinking about providing an extra observable.

But i would not make the "storage" public, because then others would be able to emit values on it. I would make it private and provide a method that gives access to the data.

// ...
private articles$ : BehaviorSubject<Article[]>([]);

public getArticles(): Observable<Article[]> {
   return this.articles$.pipe(
     distinctUntilChanged();
   )
}

public loadArticles(): void {
  this.db.collection('articles').valueChanges({ idField: 'id' })
  .subscribe(
    (articles: Article[]) => this.articles.next(articles)
  );
}

In my experience i worked mostly with a direct connected backend call. Perhaps with a pipe to map the backend data format into my frontend data format, but thats it.
The other approach i use mostly when i want to implement a central data source.

By the way, i have the tendency to type the return types of my methods, so that the transpiler can provide me some support. :-)


Nginx Alias vs Rewrite I have two working configurations that, on the surface, seem to do the same thing. They both accomplish the functionality that I need:

location ~ ^/(css|images|js)/ {
    location ~ '^/(css|js)/[0-9]{8}-(.*)$' {
        alias /$1/$2;
    }
    root /server/path/to/web/root;
}

AND

location ~ ^/(css|images|js)/ {
    rewrite '^/(css|js)/[0-9]{8}-(.*)$' /$1/$2 break;
    root /server/path/to/web/root;
}

They both take a URL like /css/87654321-styles.css and deliver the file /css/styles.css. I lean toward the second solution because it's more succinct, but I don't know if one is better than the other for performance reasons, unintended side-effects, etc.

Here's my original SO post, for reference/context.

After being directed to the documentation for the Nginx rewrite module, I found these interesting lines:

  The ngx_http_rewrite_module module directives are processed in the following order:

  the directives of this module specified on the server level are executed sequentially;
  repeatedly:

  a location is searched based on a request URI;
  the directives of this module specified inside the found location are executed sequentially;
  the loop is repeated if a request URI was rewritten, but not more than 10 times.

So, if I used the rewrite directive without the break flag, there could be at least one additional loop of the server level directive.

alias is like root, so you need to use full server path
you can add break flag to rewrite for avoid internal redirect. added without break rewrite will make loop (internal redirect) for process all locations again; but break tell to rewrite do not loop and run current location.

I don't think here big difference in performance. Main difference is changing internal request uri. But in case of static files serve both work well.


Finding products of all other members of an array I had the following interview question: Given an array of integers, for each member of the array find the product of all the other members of the array.  So for instance, if you have this array:

{3, 1, 2, 0, 4}

you should end up with this:

{0 0 0 24 0}

I wrote this code which uses two loops, but I couldn't come up with a solution with just one loop.  Can anyone help me?

public int[] findProducts(int[] arr) {
    int[] products = new int[arr.length];

    for (int i = 0; i < arr.length; i++) {
        int product = 1;

        for (int j = 0; j < arr.length; j++) {
            if (i != j) {
                product *= arr[j];
            }
        }

        products[i] = product;
    }

    return products;
}

This could be done with two consecutive loops (instead of nested loops), making it O(N) instead of O(N^2).

In the first loop you calculate the total product of all non-zero elements (and the number of zeros).

If there are more than one zero, return result as is, since all elements already default to zero.

In the second you either set the result element to zero or to total product divided by the current element depending on the number of zeros in the data and the value of the current element.

BTW, There are a lot of blog posts and even some research written about the pointlessness of this kind of interview questions.


HMAC algorithm implementation I wanted to understand the HMAC algorithm. So, I implemented it as a Jupyter Notebook with all intermediate steps being output to console. My objective now is to improve it so it may serve as a reference to anyone trying to implement or understand it.

The Jupyter Notebook on github is easier to read and I would like feedback on the notebook rather than the code below.

Jupiter notebook on GitHub

import hashlib
import hmac

### Define key, message, Block size, hashing algorithm
hash_algo = "sha1"
B = 64
message = b'hello!'
key = b'key'

### hash () for an easy to read code
def hash(M):
    H = hashlib.new(hash_algo)
    H.update(M)
    b_hash = H.digest()
    return b_hash

### XOR Translation table
trans_5C = bytes((x ^ 0x5C) for x in range(256))
trans_36 = bytes((x ^ 0x36) for x in range(256))

### Zero pad key to block length B
K_zpad=key.ljust(B,b'\0')
# Xor with ipad and opad sequence
K_ipad=K_zpad.translate(trans_36)
K_opad=K_zpad.translate(trans_5C)

print("Key XOR ipad(0x363636...):\n"+K_ipad.hex())
print("\n\nKey XOR opad(0x505050...):\n"+K_opad.hex())

### concatinate message
part1=K_ipad+message
print("key XOR ipad || message:\n"+part1.hex())

### hash (key xor ipad || message )
hash1=hash(part1)
print("hash( key XOR ipad || message):\n"+hash1.hex())

### hash (key xor opad || hash ( (key xor ipad)||message ) )
part2 = K_opad + hash1
print("hash (key xor opad || hash ( (key XOR ipad)||message ) ):\n"+part2.hex())
hash_final = hash(part2)
print("\n\nFinal Hash:\n"+hash_final.hex())

# Compare with Python's HMAC Implementation
pyH=hmac.new(key,message,digestmod=hash_algo)
pyH.hexdigest()

My objective now is to improve it so it may serve as a reference to anyone trying to implement or understand it.

In light of that goal I would have thought it made more sense to use the documented test vectors of RFC 2202 rather than make up your own one.

### hash (key xor opad || hash ( (key xor ipad)||message ) )
part2 = K_opad + hash1
print("hash (key xor opad || hash ( (key XOR ipad)||message ) ):\n"+part2.hex())
hash_final = hash(part2)
print("\n\nFinal Hash:\n"+hash_final.hex())

The comment and the first print describe hash_final, not part2. IMO this is a bug, albeit a minor one.


String substitutions in ruby and coding style I wrote a short ruby script to generate various screenshots, and event though I am not completely new to ruby I rarely use it.

Full script (41 LOC): https://gist.github.com/1229115

The string substitutions irritate me. They don't look quite right - much too ugly for a beautiful and concise language like ruby. Am I overdoing it? What alternatives are there?

Here is the lines I am concerned about (for context, please take a look at the gist):

# assemble file name
fname = "#{directory}/#{prefix}-#{timestamp}-#{commit}-#{size[:width]}x#{size[:height]}.png"

# create command line stuff
call = "#{macruby_exe} #{snapper} \
        --width #{size[:width]} --height #{size[:height]} #{url} #{fname}"

# now call it
system call

# just to show the progress (and the file sizes)
puts "#{fname} (#{File.size?(fname) / 1024}kB)"

Just to illustrate the script, the output usually looks like:

screenies/screenshot-1316535022-0b5c7967887481deb17bcd9039cd3fe0ac4540d4-640x960.png (95kB)
screenies/screenshot-1316535022-0b5c7967887481deb17bcd9039cd3fe0ac4540d4-800x480.png (62kB)
screenies/screenshot-1316535022-0b5c7967887481deb17bcd9039cd3fe0ac4540d4-800x600.png (78kB)
   ^          ^          ^                          ^                       ^
   |          |          |                          |               ________|______
   |          |          |                          |              |               |
directory   prefix     timestamp                  commit    size[:width] x size[:height]

There are other ways you could write this but I'm not sure if you'd find any of them cleaner. One option is plain old sprintf. For example:

# assemble file name
fname = sprintf("%s/%s-%i-%s-%ix%i.png", directory, prefix, timestamp,
          commit, size[:width], size[:height])

You could shorten it a little by giving size its own to_s method:

class << size; def to_s; "#{self[:width]}x#{self[:height]}" end end

fname = sprintf("%s/%s-%i-%s-%s.png", directory, prefix, timestamp,
          commit, size)

...and perhaps create other convenience methods along the same lines. But still, very small gains.

Another option would be to make your own super-small template engine, something like this:

def tiny_subst template, bndg
  param_exp   = /:(\w+)[^\w:]+/i
  tmpl_params = template.scan(param_exp).flatten

  proc {
    out_str = template.clone

    tmpl_params.reduce(out_str) do |str, p|
      str.gsub ":#{p}", eval(p, bndg).to_s unless p.empty?
    end
  }
end

tmpl   = ':directory/:prefix-:timestamp-:commit-:size.png'
subber = tiny_subst(tmpl, binding)

directory = 'screenies'
prefix    = 'screenshot'
timestamp = Time.now.to_i
commit    = '0b5c7967887481deb17bcd9039cd3fe0ac4540d4'
size      = '640x960'

subber.yield # => screenies/screenshot-1316535022-0b5c7967887481deb17bcd9039cd3fe0ac4540d4-640x960.png

commit  = 'foobarbazquux'
size    = '9000x9000'

subber.yield # => screenies/screenshot-1316535022-foobarbazquux-9000x9000.png

So, that's kinda nice, but is it really worth it? Not for me, but I guess if you really hate "#{this}" it might be.