Optimizing unboxed array operations in Haskell

Okay, first let's establish a baseline. I'm on a 32-bit system now, so the allocation figures and also the timings are somewhat different than on a 64-bit system, but the trends are mostly the same. To see what we deal with, let's not compile for profiling, but just plain with optimisations (-O2).

I have created a repository with all versions of the code I ran. I took the liberty and removed all superfluous seqs, and slightly changed applyLoop, to

applyLoop i v
    | i == n    = v
    | otherwise = applyLoop (i + 1) $ f i v

which changes the result if apply is initially called with a zero argument (the new one immediately returns the original argument, while the old looped until i reaches -1 after wrap-around of the counter), but for positive arguments the semantics remain unchanged. For very small positive n, the one extra comparison the new version does might make a measurable difference if f is inlined and trivial, but for nontrivial n or f, the extra comparison is negligible, and the new version avoids code duplication that happens with the old.

That gives

$ ./test3 +RTS -s
array (0,1) [(0,0),(1,1)]
    560,049,112 bytes allocated in the heap
        32,160 bytes copied during GC
        42,632 bytes maximum residency (2 sample(s))
        22,904 bytes maximum slop
            1 MB total memory in use (0 MB lost due to fragmentation)

                                    Tot time (elapsed)  Avg pause  Max pause
Gen  0      1083 colls,     0 par    0.02s    0.02s     0.0000s    0.0001s
Gen  1         2 colls,     0 par    0.00s    0.00s     0.0002s    0.0002s

INIT    time    0.00s  (  0.00s elapsed)
MUT     time    1.35s  (  1.35s elapsed)
GC      time    0.02s  (  0.02s elapsed)
EXIT    time    0.00s  (  0.00s elapsed)
Total   time    1.37s  (  1.37s elapsed)

%GC     time       1.3%  (1.3% elapsed)

Alloc rate    415,944,041 bytes per MUT second

Productivity  98.6% of total user, 98.4% of total elapsed

in contrast to

$ ./test4 +RTS -s
(0,1)
        47,492 bytes allocated in the heap
        1,756 bytes copied during GC
        42,632 bytes maximum residency (1 sample(s))
        18,808 bytes maximum slop
            1 MB total memory in use (0 MB lost due to fragmentation)

                                    Tot time (elapsed)  Avg pause  Max pause
Gen  0         0 colls,     0 par    0.00s    0.00s     0.0000s    0.0000s
Gen  1         1 colls,     0 par    0.00s    0.00s     0.0002s    0.0002s

INIT    time    0.00s  (  0.00s elapsed)
MUT     time    0.04s  (  0.04s elapsed)
GC      time    0.00s  (  0.00s elapsed)
EXIT    time    0.00s  (  0.00s elapsed)
Total   time    0.04s  (  0.04s elapsed)

%GC     time       0.5%  (0.6% elapsed)

Alloc rate    1,182,455 bytes per MUT second

Productivity  98.9% of total user, 103.1% of total elapsed

from test4. The difference is enormous. Well, let's look at the core to find out why, and how we can reduce it.

First, the core for using test4:

Rec {
Main.main_$s$wapplyLoop [Occ=LoopBreaker]
  :: GHC.Prim.Int#
     -> GHC.Prim.Word#
     -> GHC.Prim.Word#
     -> (# GHC.Word.Word32, GHC.Word.Word32 #)
[GblId, Arity=3, Caf=NoCafRefs, Str=DmdType LLL]
Main.main_$s$wapplyLoop =
  \ (sc :: GHC.Prim.Int#)
    (sc1 :: GHC.Prim.Word#)
    (sc2 :: GHC.Prim.Word#) ->
    case sc of wild {
      __DEFAULT ->
        Main.main_$s$wapplyLoop
          (GHC.Prim.+# wild 1)
          (GHC.Prim.plusWord# sc2 (__word 1))
          (GHC.Prim.minusWord# sc1 (__word 1));
      10000000 -> (# GHC.Word.W32# sc1, GHC.Word.W32# sc2 #)
    }
end Rec }

You can't beat that without short-cutting. You get a tight loop using only unboxed values in registers, it does no allocation. But note that seqing x and y in test4 is necessary here to aid the strictness analyser. Without it, you get a loop using boxed Word32s building up huge thunks. The seqs that originally were in apply resp. in the function (\_ x -> x `seq` testN x) passed to it make no difference whatsoever.

So, we have a lean and mean unboxed loop to aim for, and a sluggish alternative allocating tons. What is allocating so much, and taking so much time there?

The applyLoop worker in this case begins with

case GHC.ST.runSTRep
       @ (Data.Array.Base.UArray GHC.Types.Int GHC.Word.Word32)
       (\ (@ s) (s :: GHC.Prim.State# s) ->
          let {
            n# [Dmd=Just L] :: GHC.Prim.Int#
            [LclId, Str=DmdType]
            n# = GHC.Prim.sizeofByteArray# ww4 } in
          case GHC.Prim.newByteArray# @ s n# s of _ { (# ipv, ipv1 #) ->
          case {__pkg_ccall array-0.4.0.1 memcpy forall s.
                              GHC.Prim.MutableByteArray# s
                              -> GHC.Prim.ByteArray#
                              -> GHC.Prim.Word#
                              -> GHC.Prim.State# GHC.Prim.RealWorld
                              -> (# GHC.Prim.State# GHC.Prim.RealWorld, GHC.Prim.Addr# #)}

allocating a new ByteArray# - which is pretty fast, GHC's runtime is very good at such things, but nevertheless, 10 million allocations sum up to non-negligible time - and copying the contents of the old array over. The __pkg_ccall is a quite fast C call, but it is considerably slower than a Haskell call, and calling over to C for copying 8 bytes is wasteful (the call takes much longer than the copying). I think - though I can't verify at the moment - that on a 64-bit system that is done without calling out to C, which would be much faster. Replacing the thaw with a manual copying loop for an unsafeNewArray_ of the appropriate size (manual_copy.hs) brings down the time to 0.65 seconds - but increases the allocation figures:

$ ./manual_copy +RTS -s
array (0,1) [(0,0),(1,1)]
    760,049,112 bytes allocated in the heap
        41,424 bytes copied during GC
        42,632 bytes maximum residency (2 sample(s))
        22,904 bytes maximum slop
            1 MB total memory in use (0 MB lost due to fragmentation)

                                    Tot time (elapsed)  Avg pause  Max pause
Gen  0      1469 colls,     0 par    0.02s    0.02s     0.0000s    0.0001s
Gen  1         2 colls,     0 par    0.00s    0.00s     0.0003s    0.0003s

INIT    time    0.00s  (  0.00s elapsed)
MUT     time    0.62s  (  0.63s elapsed)
GC      time    0.02s  (  0.02s elapsed)
EXIT    time    0.00s  (  0.00s elapsed)
Total   time    0.64s  (  0.65s elapsed)

%GC     time       3.3%  (3.2% elapsed)

Alloc rate    1,226,997,169 bytes per MUT second

Productivity  96.6% of total user, 95.3% of total elapsed

Note that it would be different for a large array, then calling to memcpy of the C standard library would pay off.

Using unsafeThaw here (unsafeThaw.hs) avoids allocating new ByteArray#s (the underlying raw arrays that carry the payload of UArrays) each iteration, and the involved copying, so it unsurprisingly speeds up the program, and reduces allocation:

$ ./unsafeThaw +RTS -s
array (0,1) [(0,0),(1,1)]
    400,049,112 bytes allocated in the heap
        27,596 bytes copied during GC
        42,632 bytes maximum residency (2 sample(s))
        22,904 bytes maximum slop
            1 MB total memory in use (0 MB lost due to fragmentation)

                                    Tot time (elapsed)  Avg pause  Max pause
Gen  0       764 colls,     0 par    0.01s    0.01s     0.0000s    0.0001s
Gen  1         2 colls,     0 par    0.00s    0.00s     0.0003s    0.0003s

INIT    time    0.00s  (  0.00s elapsed)
MUT     time    0.28s  (  0.28s elapsed)
GC      time    0.01s  (  0.01s elapsed)
EXIT    time    0.00s  (  0.00s elapsed)
Total   time    0.29s  (  0.29s elapsed)

%GC     time       4.4%  (4.3% elapsed)

Alloc rate    1,448,146,787 bytes per MUT second

Productivity  95.5% of total user, 95.6% of total elapsed

So the speedup is about 4.5× compared to the original - not bad, but still a far cry from the test4 time - and the allocation is reduced by 160 million bytes (10 million times 8 bytes for the two Word32 plus 8 for bookkeeping [size] of the ByteArray#), but it is still huge.

That is because

of _ { (# ipv4, ipv5 #) ->
(# ipv4,
    Data.Array.Base.UArray
    @ GHC.Types.Int @ GHC.Word.Word32 ww1 ww2 ww3 ipv5 #)
}

in each iteration it creates a new UArray Int Word32. It allocates a new UArray constructor, and pointers to the components, lower bound, upper bound, number of elements and the ByteArray#. We can avoid that by returning the argument array - after we sneakily changed the contents of its ByteArray# (unsafeThaw2.hs):

test3 :: UArray Int Word32 -> UArray Int Word32
test3 arr = runST (change arr) where
    change a' = do
        a <- Data.Array.Unsafe.unsafeThaw a' :: ST s (STUArray s Int Word32)
        x0 <- unsafeRead a 0
        x1 <- unsafeRead a 1
        unsafeWrite a 0 (x1 + 1)
        unsafeWrite a 1 (x0 - 1)
        return a'

which results in

$ ./unsafeThaw2 +RTS -s
array (0,1) [(0,0),(1,1)]
    80,049,100 bytes allocated in the heap
        6,828 bytes copied during GC
        42,632 bytes maximum residency (2 sample(s))
        22,904 bytes maximum slop
            1 MB total memory in use (0 MB lost due to fragmentation)

                                    Tot time (elapsed)  Avg pause  Max pause
Gen  0       151 colls,     0 par    0.00s    0.00s     0.0000s    0.0001s
Gen  1         2 colls,     0 par    0.00s    0.00s     0.0003s    0.0003s

INIT    time    0.00s  (  0.00s elapsed)
MUT     time    0.17s  (  0.17s elapsed)
GC      time    0.00s  (  0.00s elapsed)
EXIT    time    0.00s  (  0.00s elapsed)
Total   time    0.17s  (  0.17s elapsed)

%GC     time       1.7%  (1.7% elapsed)

Alloc rate    480,150,792 bytes per MUT second

Productivity  98.1% of total user, 98.0% of total elapsed

another nearly twofold speedup and allocation figures reduced by 80%.

It takes so much longer and allocates so much more than the tight loop mostly because each iteration costs a GHC.ST.runSTRep, which is NOINLINE and inhibits a worker-wrapper transform of test3 that would allow cheaper access to the ByteArray#.

You can get much better performance if you change the type of apply to be monadic, and use a monadic version of test3, e.g. (cleanST.hs)

import Data.Word
import Data.Array.Unboxed
import Data.Array.ST
import Data.Array.Base
import Control.Monad.ST

test0 :: STUArray s Int Word32 -> ST s (STUArray s Int Word32)
test0 arr = do
    x0 <- unsafeRead arr 0
    x1 <- unsafeRead arr 1
    unsafeWrite arr 0 (x1+1)
    unsafeWrite arr 1 (x0 - 1)
    return arr

applyM :: Monad m => Int -> (Int -> a -> m a) -> a -> m a
applyM n f v0 = do
    let loop i v
            | i < n    = do
                v' <- f i v
                loop (i+1) v'
            | otherwise = return v
    loop 0 v0

main :: IO ()
main = let arr = listArray (0,1) [0,1] :: UArray Int Word32
       in print $ runSTUArray $ do
                    marr <- thaw arr
                    applyM 10000000 (\_ a -> test0 a) marr

that comes close to the tight loop:

$ ./cleanST +RTS -s
array (0,1) [(0,0),(1,1)]
        49,148 bytes allocated in the heap
        1,756 bytes copied during GC
        42,632 bytes maximum residency (1 sample(s))
        18,808 bytes maximum slop
            1 MB total memory in use (0 MB lost due to fragmentation)

                                    Tot time (elapsed)  Avg pause  Max pause
Gen  0         0 colls,     0 par    0.00s    0.00s     0.0000s    0.0000s
Gen  1         1 colls,     0 par    0.00s    0.00s     0.0002s    0.0002s

INIT    time    0.00s  (  0.00s elapsed)
MUT     time    0.05s  (  0.06s elapsed)
GC      time    0.00s  (  0.00s elapsed)
EXIT    time    0.00s  (  0.00s elapsed)
Total   time    0.06s  (  0.06s elapsed)

%GC     time       0.4%  (0.4% elapsed)

Alloc rate    905,470 bytes per MUT second

Productivity  99.2% of total user, 97.3% of total elapsed

but is of course still slower because the array reads and writes go to the L1 cache, while test4 operated completely in registers.

If you don't want to change the type of apply, I know no clean way of coming close to the performance of the tight loop.

If you are prepared to get your hands really dirty, you can use low-level primitives (low_level.hs),

{-# LANGUAGE MagicHash, UnboxedTuples, BangPatterns #-}

import Data.Word
import Data.Array.Unboxed
import Data.Array.Base
import GHC.Base

test3 :: UArray Int Word32 -> UArray Int Word32
test3 arr = change arr where
    swp mbarr s =
        case readWord32Array# mbarr 0# s of
          (# s1, w0 #) ->
            case readWord32Array# mbarr 1# s1 of
              (# s2, w1 #) ->
                case writeWord32Array# mbarr 0# (w1 `plusWord#` 1##) s2 of
                  s3 -> writeWord32Array# mbarr 1# (w0 `minusWord#` 1##) s3
    change arr@(UArray _ _ _ ba)
        = case swp (unsafeCoerce# ba) realWorld# of
            !s -> arr

which gives more or less the same figures as cleanST,

$ ./low_level +RTS -s
array (0,1) [(0,0),(1,1)]
        49,112 bytes allocated in the heap
        1,756 bytes copied during GC
        42,632 bytes maximum residency (1 sample(s))
        18,808 bytes maximum slop
            1 MB total memory in use (0 MB lost due to fragmentation)

                                    Tot time (elapsed)  Avg pause  Max pause
Gen  0         0 colls,     0 par    0.00s    0.00s     0.0000s    0.0000s
Gen  1         1 colls,     0 par    0.00s    0.00s     0.0002s    0.0002s

INIT    time    0.00s  (  0.00s elapsed)
MUT     time    0.05s  (  0.06s elapsed)
GC      time    0.00s  (  0.00s elapsed)
EXIT    time    0.00s  (  0.00s elapsed)
Total   time    0.06s  (  0.06s elapsed)

%GC     time       0.4%  (0.4% elapsed)

Alloc rate    928,001 bytes per MUT second

Productivity  99.0% of total user, 95.1% of total elapsed

and more-or-less equivalent core (cleanST's is cleaner), but is really as dirty as it looks. I cannot recommend going down that route except in extreme circumstances.

Note: new (better) timings, the machine was busier when the original runs were made


What I like:

usage of f-strings, but you are not using them everywhere eg: URL = 'https://remote.co/remote-jobs/search/?search_keywords='+pos.replace(' ', '+')
usage of if __name__ == '__main__':
code is fairly easy to grasp
effort to provide a decent working script with a minimum of code (70 lines)

What I like less:

usage of exception handling: I would be more specific and not catch any type of exception, only handle those that are relevant to the operation being performed in the try/catch block (selenium.common.exceptions)
lack of global exception handling in the rest of the code
also think about logging full exception details to a file so you don't have to guess what has gone wrong
I would also avoid nesting of try/catch blocks, try to separate each operation from each other (see below)
mixing lower case and upper case in variable names: remember that variable names are case-sensitive in Python. jobPD and jobpd are two different variables that can get assigned different values so this could be a nasty source of bugs
variable names should be more descriptive and meaningful: jobPD does not give a clear hint about what it represents. More descriptive names would be job_title, job_posted_time etc

Regarding the scraping process, make sure that the DOM elements your are expecting are really present: websites change their layout more or less often and you must spot changes that you could break your application. You can either check with Selenium or BS4 if you have already retrieved the HTML. But it seems logical to use Selenium. If you use BS, note the behavior of the different functions:

From the BS4 doc (emphasis is mine):

  If find_all() can’t find anything, it returns an empty list. If find()
  can’t find anything, it returns None

You have this block (the nested try/catch block):

    try: # can't locate element - click the close on the popup add
        WebDriverWait(driver, 5).until(
            ec.presence_of_element_located((By.CLASS_NAME, 'portstlucie-CloseButton'))
            )
        addClose = driver.find_element_by_xpath('//*[@id="om-oqulaezshgjig4mgnmcn-optin"]/div/button')
        addClose.click()
    except: # Timeout / can't locate add - break
        break

Is is better to anticipate and avoid exceptions, rather than handling them. So it seems to me that you should verify the existence of the element if possible, rather than trigger an exception.

Instead, you could use the more generic findElements function. Note that findElement is different than findElements. The difference:

  findElements will return an empty list if no matching elements are
  found instead of an exception (NoSuchElementException)

Reference: Find Element and FindElements in Selenium WebDriver

However, if you stick to the current approach, you should not blindly catch all exceptions: in this context the one relevant exception that may occur is: NoSuchElementException.

There is one thing that is not okay: you are using the requests module in parallel to Selenium. That is unnecessary since you already have an instance of Selenium that you can use. To fetch the whole HTML just use:

html = driver.page_source

then feed it to BS

Final thought: have you thought about logging the results to a file, a table or CSV perhaps ? The console buffer may be too small if you retrieve lots of results.


To start off, I can't see a single semicolon being used here. You should always close your statements with a semicolon.

Also I can't quite understand what you've done here:

'click .hint-text': function(e) {
    e.preventDefault()

    var currentHintId = $(e.target).attr('id')
    Session.set('currentHintId', currentHintId)

    $('.hint-popup').show()
},

Is this a property in an object? If so why have you named it 'click .hint-text'? Give it a proper name such as togglePopup or something along those lines to make it more obvious what it does. Now it's just confusing.

And for the other function you could name it submit(whatever you're submitting).

If you included some additional markup it would help your question a bit.


Is this a reasonable binary search implementation?

Your implentation can search integers, we expect to search in an array of any comparable objects.

I think you have a bug if the element is not found. It will return 1 instead of -1 /  throwing an exception. Add unit tests.

Also, Java is now open source, you can compare your implementation with the one from the JDK (Arrays.binarySearch).

Also, in code style:

it is recommended to use {} in if block, even if they are one-liner


Given an array of numbers, return array of products of all other numbers

The Time Complexity is indeed: \$\mathcal{O}(n^2)\$.

You can optimize it to \$\mathcal{O}(n)\$ by calculating the total product of all elements in the source array, and then looping over the array and dividing the product by each element to get the product excluding that particular element.

Code:

public static BigInteger[] calcArray(int[] input) throws Exception {
      if (input == null) {
        throw new IllegalArgumentException("input is null");
      }

      BigInteger product = calculateProduct(input);
      BigInteger result[] = new BigInteger[input.length];
      for (int i = 0; i < input.length; i++) {
        result[i] = product.divide(BigInteger.valueOf(input[i]));
      }
      return result;
}

private static BigInteger calculateProduct(int[] input) {
      BigInteger result = BigInteger.ONE;
      for (int i = 0; i < input.length; i++) {
          result = result.multiply(BigInteger.valueOf(input[i]));
      }
      return result;
}

EDIT: The above code assumes that none of the numbers is 0 in the input array.


Retrieving HKLM nodes of Windows10 Registry

It looks to me, one of your biggest problems is working directly with the Nodes of the TreeView.  This necessitates calling Paint() everytime you add a node.  If you start with a new TreeNode as root, and build the tree on that, you can add the whole thing to the TreeView by adding the root to the Nodes.

I also found that running VS as administrator greatly reduced the number of errors, which also sped it up.

By using a custom TreeNode class that inherits from Forms.TreeNode, you can recursively fill the tree inside the class.  This also allows you to store the actual RegistryKey object in each node.  Here's a relatively simple way of doing it:

Public Class RegistryTreeNode
    Inherits TreeNode
    Private Class SecurityExceptionLogger
        Private Shared Property Exceptions As New List(Of (String, SecurityException))
        Private Shared TempFileName As String = Path.GetTempFileName()
        Private Shared writer As StreamWriter
        Private Shared reader As StreamReader
        Public Shared Async Sub Add(exception As SecurityException)
            Exceptions.Add((Now.ToUniversalTime, exception))
            Using writer = New StreamWriter(TempFileName, True)
                Await writer.WriteLineAsync($"{Now.ToUniversalTime} - {exception.Message}")
            End Using
        End Sub
        Public Shared Sub PrintExceptions(stream As StreamWriter)
            Using stream
                Exceptions.ForEach(Async Sub(x)
                                       Await stream.WriteLineAsync($"{x.Item1} - {x.Item2.Message}")
                                   End Sub)
            End Using
        End Sub

    End Class
    Private _RegKey As RegistryKey

    Private Property RegKey As RegistryKey
        Get
            Return _RegKey
        End Get
        Set
            _RegKey = Value
            Name = _RegKey.Name
            Text = Name
            For Each key In _RegKey.GetSubKeyNames
                Try
                    Nodes.Add(New RegistryTreeNode(_RegKey.OpenSubKey(key)))
                Catch ex As SecurityException
                    SecurityExceptionLogger.Add(ex)
                Catch ex As Exception
                    MsgBox(ex.Message)
                End Try
            Next
        End Set
    End Property

    Public Sub New()
        MyBase.New()
        RegKey = Registry.LocalMachine
    End Sub
    Public Sub New(newRegistryKey As RegistryKey)
        MyBase.New()
        RegKey = newRegistryKey
    End Sub
End Class

I hardcoded the default key to Registry.LocalMachine for your uses.  Simply Dim root = New RegistryTreeNode() and the whole tree will build itself.

You'll notice I made a simple logger that writes to file as well as keeping a record in memory, instead of just in memory, just as an exercise.

Another way to speed things up, is to only load the first sub level of nodes on start up.  You can then use the AfterSelect event handler to load the sub nodes of the key you want to expand.  Something like this:

Public Class Form1
    Dim root = New RegistryTreeNode(Registry.LocalMachine)
    Public Sub New()

        ' This call is required by the designer.
        InitializeComponent()

        ' Add any initialization after the InitializeComponent() call.
        TreeView1.Nodes.Add(root)
    End Sub

    Private Sub TreeView1_AfterSelect(sender As Object, e As TreeViewEventArgs) Handles TreeView1.AfterSelect
        Dim node = DirectCast(TreeView1.SelectedNode, RegistryTreeNode)
        For Each key In node.GetSubKeyNames
            Try
                Dim regKey = New RegistryTreeNode(node.OpenSubKey(key))
                If Not regKey Is Nothing Then
                    node.Nodes.Add(regKey)
                End If

            Catch ex As SecurityException
                SecurityExceptionLogger.Add(ex)
            Catch ex As Exception
                MsgBox(ex.Message)
            End Try
        Next

    End Sub

End Class
Class SecurityExceptionLogger
    Private Shared Property Exceptions As New List(Of (String, SecurityException))
    Private Shared TempFileName As String = Path.GetTempFileName()
    Private Shared writer As StreamWriter
    Private Shared reader As StreamReader
    Public Shared Async Sub Add(exception As SecurityException)
        Exceptions.Add((Now.ToUniversalTime, exception))
        Using writer = New StreamWriter(TempFileName, True)
            Await writer.WriteLineAsync($"{Now.ToUniversalTime} - {exception.Message}")
        End Using
    End Sub
    Public Shared Sub PrintExceptions(stream As StreamWriter)
        Using stream
            Exceptions.ForEach(Async Sub(x)
                                   Await stream.WriteLineAsync($"{x.Item1} - {x.Item2.Message}")
                               End Sub)
        End Using
    End Sub

End Class
Public Class RegistryTreeNode
    Inherits TreeNode

    Private _RegKey As RegistryKey

    Private Property RegKey As RegistryKey
        Get
            Return _RegKey
        End Get
        Set
            _RegKey = Value
            Name = _RegKey.Name
            Text = Name.Substring(Name.LastIndexOf("\"c) + 1)

        End Set
    End Property

    Public Sub New()
        MyBase.New()
        RegKey = Registry.LocalMachine
    End Sub
    Public Sub New(newRegistryKey As RegistryKey)
        MyBase.New()
        RegKey = newRegistryKey
    End Sub
    Public Function GetValueNames() As List(Of String)
        Return _RegKey.GetValueNames().ToList()
    End Function
    Public Function GetValueKind(name As String) As RegistryValueKind
        Return _RegKey.GetValueKind(name)
    End Function
    Public Function GetValue(name As String) As Object
        Return _RegKey.GetValue(name)
    End Function
    Public Function GetSubKeyNames() As String()
        Return _RegKey.GetSubKeyNames()
    End Function
    Public Function OpenSubKey(key As String) As RegistryKey
        Return _RegKey.OpenSubKey(key)
    End Function
End Class

I moved the SecurityExceptionLogger outside the RegistryTreeNode class.  You can also load the values for the keys, if needed, by looping through them after the sub nodes are added.


Cache DOM selectors

You select the target container over and over again. You should get it once outside the function, cache it in a variable and reuse it:

const target = document.getElementById('quotes');

Naming

Try to use descriptive names instead of myarray, myVar and quote:

const quotes = [];
function changeQuoteRandomly() {}

Style

It's easier to read, if you use single quotes on the strings, as you don't have to escape the double quotes:

quotes.push('"Important words" by Someone');

You sometimes use a semicolon at the end of the line and sometimes you don't – use only one way and be consistent:

var myarray = []
var myVar = setInterval(quote, 15000);

I would prefer using push to add elements to the array instead of setting the index manually. Or you could even add all elements during initialization:

const quotes = [
    '"Important words" by Someone',
    '"Other important words" by Someone'
];

Both ways make it easier to re-arrange elements later as you don't have to keep track of the index yourself.

Semantics

Currently your markup looks like this:

    "The single best piece of advice: Constantly think about how you could be doing things better and questioning yourself."

     Elon Musk

You could improve this, by using blockquote, cite and even figure elements. Here you can find more infos and examples on W3C: 4.4.5. The blockquote element.

User experience

While this is a nice addition on your website, there are a few downsides:

There's no introduction to this section, the visitor has no clue why you're showing them. Try to relate them to your business.
The first quote is shown after 15s, until then only an empty blue container ist visible.
15s between quotes is a really long time. There's no indicator that more is coming, so there's a great chance that a user will not see a second one.


Last five non-zero digits of a factorial in base b

Note that the constraint in this problem is \$n \leq 10^{18}\$. In other words, the factorials you are calculating can be as large as \$10^{18}!\approx 10^{1.7\cdot 10^{18}}\$. In other words, if you aren't running into a time limit, you will most likely run into a memory limit.

Your function treeFactor calculates:

\$10000!\$ in 0.06 seconds
\$100000!\$ in 2.57 seconds
\$200000!\$ in 9.34 seconds
\$400000!\$ in 29.73 seconds
\$1000000!\$ in 129.58 seconds

In other words, even \$1000000!\$ can't be calculated within the time limit. Futher, \$(10^7)!\$ will probably take at least an hour, \$(10^8)!\$ will probably take several days. Even with a lot of optimizations to calculating factorials, it isn't going to make it, unfortunately. Further, I can't think of a faster method now (I tried a few things, but I got recursion limit exceeded).

For this problem, you only need the last five non-zero digits. For this, you'll need to do a few things with modular arithmetic. Only read the text under the spoiler if you want more hints.

   For example, create an array with the five last non-zero digits. Take away all factors that cause a factor of the base \$b\$. Also, note that multiplying by a number larger than \$10^5\$ is the same as multiplying by the same number modulo \$10^5\$.

I will not write the solution for you. This problem is quite hard - considering that only four people solved it out of over a hundred attempts. I do not know whether this solution is fast enough (also because there is no time limit specified, and because I didn't code it), but it should at least run within a couple of minutes, if you optimalize enough. The calculation of \$10^{18}!\$ can't be done with any method in any reasonable time.


You should "lift" your functions instead.

For example, instead of defining:

def getSomething(input: V): Future[FinalType] = {
   val intermediate: IntermediaryType = complicatedFunctionReturningFuture(input)
   mappingFunction(intermediate)
}

use

complicatedFunctionReturningFuture(input).map(mappingFunction)

directly in your code.

You can think of map as a method that is lifting a function IntermediaryType => FinalType to a function Future[Intermediary] => Future[FinalType].

If you have a Future[Try[Type]], you can do a "double lifting" (I'm claiming the trademark on that term!):

complicatedFunctionReturningFutureTry(input).map(_.map(mappingFunction))

which lifts the same function IntermediaryType => FinalType to a function Future[Try[Intermediary]] => Future[Try[FinalType]].


I'll just get this out the way: The method should work fine. The problems aren't with the logic.

Indentation and whitespace are... awful. Seriously, it's a total mess. Did multiple double-takes before I was able to make sense of where tests began and ended. The method itself is more readable, but still inconsistent. Considering it's so simple there's really no excuse for messy whitespace.

Second, don't call a variable by its value. What I mean is don't do this

fizz_3 = fizzbuzz(3)
expect(fizz_3).to eq("Fizz")

What if you wanted to check with the number 6 instead? Now the variable would have to change name too to make sense (in two places, even), which means it's not really variable, is it? Besides, for this you can just write

expect(fizzbuzz(x)).to eq y

and skip the variable.

Third, why return nil? As I said in a comment, a solution typically prints/returns either "Fizz", "Buzz", "FizzBuzz", or the number. That said, there's no "official" FizzBuzz specification (closest you find is probably this), because that's not the point. Similarly, there's no single "correct" solution, because that's not the point either. All FizzBuzz does is (firstly) test whether someone is able to solve the task at all, and (secondly) how they went about it.

You solved it (mostly - or with some atypical requirements), which is good. But really, that's the ground floor here.
There are tests, which is good. Not that the tests are important in and of themselves for something so trivial, but the ability to write tests is a plus.
But the code is an almost unreadable mess, which is bad.

FizzBuzz was conceived as an interviewer's tool. Provided the person being interviewed is able to program at all, the solution provides something to examine and discuss.

So if I were to pretend to be an interviewer, I'd say thank you for your time, and show you the door. This is just too sloppy.

But assuming the code was the same, just using proper whitespace, I might ask the following questions (just concerning the method itself):

How would you refactor this to avoid the duplication of logic? This is a practical concern.
How would you refactor this to only use the method's implicit return? Having multiple explicit returns is not the most Ruby-like thing to do, so this is more stylistic concern.
What might you use in place of the if statements? This is a general competence question.

And provided I'd given you the task without specifying what should be returned by default, I'd also ask:

Why return nil? I want to hear the reasoning behind this, because it effective means the method can have two return types, meaning callers may have more work to do in checking the returned value. I want to know if you considered the method's usage.

Of course, this is CodeReview, not CodeInterview, and I don't expect answers to those questions. It's just food for thought - which is real purpose of FizzBuzz. For some inspiration, see this question.


Dynamic programming solution for cross river algorithm (part 2)

You still have the remnant of the same bug.

Consider the river **.*..*...*....** The last slot exists but is not reachable. Yet you can jump from the penultimate slot to cross the river.

Or consider **.*..*...*....*. The slot just past the end is not reachable but something further to its right is.

This can be dealt with by modifying your code thus:

def can_cross_river(river):
    dp = set()
    dp.add((0,0))
    for i,v in enumerate(river):
        if v != '*':
            continue
        for speed in range(0,i+1):
            if (i-speed, speed) in dp or (i-speed, speed-1) in dp or (i-speed, speed+1) in dp:
                dp.add((i, speed))
                if i+speed+1 >= len(river):
                    return True
    return False


Retrieve a city list for a country and state

Utilize the query object, this type of querying is more suitable there than as params.
Use object destructuring.
Dry your code, see how we're mapping over [countryId, stateId] to check the validity of the input. Ideally, you would have been using a module like Joi for that.
Maintain a happy path and break out of the function early when erroring.
Use async/await instead of .then().
Return a 400 when the input is invalid.
User proper camelCase (in both the DB and the backend).
Check the array length instead of checking the existence of the first element. So if (arr.length) not if (arr[0]). The reason for that is at some cases the first element of the array could be 'falsy' (such as '' and 0) which can be valid sometimes.
When erroring, return an error field, not a msg one.
Study javascript's array methods such as map, reduce, filter, and every. They will save your life more often than you thought.

router.get("/citylist", async (req, res) => {
  try {

    const { countryid, stateId } = req.query

    const validInput = [countryid, stateid].map(mongoose.Types.ObjectId.isValid).every(Boolean)
    if (!validInput) return res.status(400).json({ error: "Country or State Id is not valid" })

    const cities = await CityModel.find({ countryId, stateId })
    if (!cities || cities.length) return res.status(404).json({ error: "City not found" })

    res.json(cities);

  } catch (error) {
    res.status(500).json({ error })
  }
});


This test, on its own, is not enough. Additionally, the test is called testUpdated, but it does not actually test that the value is updated.

What it does is:

find an object based on a key value
it modifies some non-key content
it 'persists' the changes
it finds an object for the same key as before
it tests that the object it found the second time is the identical object to the first.

At no point in this process have you actually tested that the values you think you changed on the object, actually did change.

Additionally, you never tested whether those changes were persisted to the database/store.

What I would expect a test like this to do, is to:

get the object using the key
make the changes
assert that the values have changed to the new values.
persist the changes
assert that a persist happened (some timestamp in the database maybe)?
use an independent mechanism to check that the values in the database match the values in the object (a different database connection, a different DAO instance, something).
ensure that finding the same key value finds the same instance as before.

That is a lot of things to change, which means the test, on it's own, is not enough, and should be included with a bunch of other tests, each one testing different parts of the process.


Calculating the joint probability of n events from a sample sequence of occurrences

I'm just going to look at cond_probs here. You'll see that there's plenty in this function for one answer.

1. Comments on your code

The line return probs has the wrong indentation. Copy/paste error?
There are no docstrings. What do these functions do and how do I call them? (The text from your question would be a good starting-point for docstrings.)
There are no test cases. This is the kind of code that would benefit from some unit tests and/or doctests.
You write, "cond_probs() produces a dictionary {'AT': n1, 'AG': n2, 'AA': n3, 'AC': n4, ... , 'TC': n16}" but when I run it the dictionary has tuples for keys, not strings:

>>> cond_probs(['AAC'])
{('A', 'C'): 0.5, ('A', 'A'): 0.5}

I don't understand the behaviour when reverse=True. You write, "In reverse, the transitional probabilities are calculated with the sequences in reverse" but this doesn't seem to be the case:

>>> cond_probs(['CAA'], reverse=True)
{('A', 'A'): 1.0}

What has happened to C? This looks wrong to me.

If I were writing this, I'd implement cond_probs to run only in the forward direction; to do the reverse direction I would pass in reversed sequences.
Prefer using True and False for Booleans, not 1 and 0. So the second parameter to cond_probs should be reverse=False.
Instead of:

counts = {}
# ...
counts[X] = counts.get(X, 0) + 1

use collections.Counter and write:

counts = Counter()
# ...
counts[X] += 1

(But see below for how to use collections.Counter.update.)
Instead of:

range(len(seq))[::-1][:-1]

write:

range(len(seq) - 1, 0, -1)

(But see below for how to avoid this.)
Prefer iteration over elements of sequences instead of iteration over their indexes. So instead of:

for i in range(len(seq) - 1):
    counts[seq[i],seq[i + 1]] = counts.get((seq[i], seq[i + 1]), 0) + 1

use zip:

for t in zip(seq, seq[1:]):
    counts[t] += 1

or, even more simply, using collections.Counter.update:

counts.update(zip(seq, seq[1:]))

(If you are concerned about memory use, then you could use  itertools.islice and write islice(seq, 1, None) instead of seq[1:] to avoid the copy.)
In this code:

for s in states:
    sCounts = dict((k,v) for (k,v) in counts.items() if k[0] == s)

you iterate over the whole of counts for every state, collecting the ones you want. It would be more efficient to iterate only over the transitions from s. See below for how this would look.

2. Revised code

def cond_probs_2(sequences):
    """Calculate 1-step transitional probabilities from sequences.
    Return dictionary mapping transitions to probabilities.

    >>> sequences = ['ACABC', 'ABCAB']
    >>> sorted(cond_probs_2(sequences).items())
    ... # doctest: +NORMALIZE_WHITESPACE
    [(('A', 'B'), 0.75),
     (('A', 'C'), 0.25),
     (('B', 'C'), 1.0),
     (('C', 'A'), 1.0)]

    """
    counts = Counter()
    entries = set()
    for seq in sequences:
        entries.update(seq)
        for a, b in zip(seq, islice(seq, 1, None)):
            counts[a, b] += 1
    probs = {}
    for a in entries:
        suma = float(sum(counts[a, b] for b in entries))
        if suma != 0:
            probs.update(((a, b), counts[a, b] / suma) for b in entries
                         if counts[a, b])
    return probs

This is about 30% faster than your code:

>>> from timeit import timeit
>>> from random import choice
>>> data = ''.join(choice('ACGT') for _ in range(1000000))
>>> cond_probs([data]) == cond_probs_2([data])
True
>>> timeit(lambda:cond_probs([data]), number=1) # yours
1.4706195190083236
>>> timeit(lambda:cond_probs_2([data]), number=1) # mine
0.9721288200234994

3. Rewriting in NumPy

Here's how I'd go about this kind of task in NumPy. NumPy works best with fixed-size numeric data, so I'd encode the input as small integers:

>>> import numpy as np
>>> data = 'TTAGCAGTCAGTTCAGCGTTCGACTGA'
>>> distinct = set(data)
>>> coding = {j:i for i, j in enumerate(distinct)}
>>> coding
{'A': 0, 'C': 1, 'T': 2, 'G': 3}
>>> coded_data = np.fromiter((coding[i] for i in data), dtype=np.uint8)
>>> coded_data
array([2, 2, 0, 3, 1, 0, 3, 2, 1, 0, 3, 2, 2, 1, 0, 3, 1, 3, 2, 2, 1, 3, 0,
       1, 2, 3, 0], dtype=uint8)

(I've chosen the type np.uint8 here because there are only four different characters in the input strings. You might need to choose a different type.)

Then you can encode adjacent pairs of numbers from 0–3 as a single number from 0–15, count the number of occurrences of each pair using numpy.bincount, and decode the result using numpy.reshape:

>>> n = len(distinct)
>>> pairs = coded_data[:-1] + n * coded_data[1:]
>>> counts = np.bincount(pairs, minlength=n*n).reshape(n, n)
>>> counts
array([[0, 3, 1, 2],
       [1, 0, 3, 2],
       [0, 1, 3, 3],
       [4, 2, 1, 0]])

This array represents the transition table:

          source
       │ A  C  T  G
     ──┼───────────
     A │ 0, 3, 1, 2
dest C │ 1, 0, 3, 2
     T │ 0, 1, 3, 3
     G │ 4, 2, 1, 0

You can then compute the transition probabilities by dividing each element by the sum of its column:

>>> counts / counts.sum(axis=0)
array([[ 0.        ,  0.5       ,  0.125     ,  0.28571429],
       [ 0.2       ,  0.        ,  0.375     ,  0.28571429],
       [ 0.        ,  0.16666667,  0.375     ,  0.42857143],
       [ 0.8       ,  0.33333333,  0.125     ,  0.        ]])

Here's the above as a function:

from itertools import chain
import numpy as np

def cond_probs_np(sequences):
    """Calculate 1-step transitional probabilities from sequences.
    Return dictionary mapping transitions to probabilities.

    >>> sequences = ['ACABC', 'ABCAB']
    >>> sorted(cond_probs_np(sequences).items())
    ... # doctest: +NORMALIZE_WHITESPACE
    [(('A', 'B'), 0.75),
     (('A', 'C'), 0.25),
     (('B', 'C'), 1.0),
     (('C', 'A'), 1.0)]

    """
    distinct = set(chain.from_iterable(sequences))
    n = len(distinct)
    assert(n < 256)             # so that they will fit in an np.uint8
    coding = {j:i for i, j in enumerate(distinct)}
    counts = np.zeros((n, n))
    for seq in sequences:
        coded_seq = np.fromiter((coding[i] for i in seq), dtype=np.uint8)
        pairs = coded_seq[:-1] + n * coded_seq[1:]
        counts += np.bincount(pairs, minlength=n*n).reshape(n, n)
    totals = counts.sum(axis=0)
    totals[totals == 0] = 1     # avoid division by zero
    probs = counts / totals
    return {(a, b): p for a, b in product(distinct, repeat=2)
            for p in (probs[coding[b], coding[a]],) if p}

And this is about seven times faster than your original code:

>>> cond_probs([data]) == cond_probs_np([data])
True
>>> timeit(lambda:cond_probs_np([data]), number=1)
0.2206433709943667


Re-architect to do lots of reading, then lots of writing - then loop.

OP's code presently reads file A, writes file A, reads file B, writes file B, ...

Reading file A, file B,.... partial file Z and then writing those into a large buffer is certainly faster.  Of course it is more complicated as code needs to pick-up where it left off in the middle of file Z.

I first noticed this huge saving BITD when XCOPY *.* came out as it read many files first versus the old COPY *.*.  This paradigm has been consistently true with my subsequent coding experiences.

A central issue, that unless you get deeper into the DISK I/O, code is limited to trying to do what it thinks the file system wants best in order to perform fast.

Using multiple threads (one for reading, one for writing) may help too.


Converting a 12 hour time string to a 24 hour time string

Here's how to parse dates and times using built-in .Net libraries:

static void Main()
{
    string strTime = "01:00:10AM";
    DateTime time;
    if (DateTime.TryParseExact(strTime, "hh:mm:sstt", null,
        System.Globalization.DateTimeStyles.None, out time))
    {
        Console.WriteLine("Parsed \"{0}\" as {1:HH\\:mm\\:ss}", strTime, time);
    }
    else
    {
        Console.WriteLine("Could not parse \"{0}\" as a DateTime", strTime);
    }
    Console.ReadLine();
}

Some points to note:

I'm not reinventing the wheel.  The .Net DateTime library is the best way to handle points in time.  (Use TimeSpan for duration)
I'm using custom DateTime format strings for both parsing the string and displaying the parsed DateTime result.
I'm using the TryParse pattern the .Net supplies for converting strings to various built-in types: e.g., int.TryParse, double.TryParse, etc.
In the output, I want to display the parsed time as HH:mm:ss (note that capital H is 24 hour time and lowercase h is 12 hour time).  I have to escape the colons in there because of how string.Format works.  You escape colons using backslashes like this HH\:mm\:ss.  The problem is that you then have to escape the backslashes (or use verbatim strings).  You do that with an extra backslash like this HH\\:mm\\:ss


A bunch of random observations.  I'd rethink the way you approach "classes".  I'd start with something like

classes = attr['class'] || []
classes = [classes] if typeof classes == 'string'

And then write or delete it back at the end.

Creating hashStartRe

hashStartRe = /^[.#]/
elemType = if hashStartRe.test elem then "div" else pieces.shift()

Will probably be faster than the current rewriting and calling elem.charAt(0).  It's definitely easier to read.  However, it causes problems with "pos".  (Solveable problems, but you do need to be careful.)

At this point, the big question is whether you want to allow multiple ids or ids that aren't in the front position.  If you restrict your attention to a#id.class1.class2 and don't process a.class1#id1#id2, you can simplify the for loop as well.

A final performance point, since regular expressions are so fast on modern browsers, you may want to consider using the regular expression /(.#)([^.#)+)/ to do your parsing.  But you'd need to sit down and do some proper analysis on what was faster.  My gut tells me it will be.


It takes advantages of the fact that all primes from 5 and above can be written as 6X-1 or 6X+1,

I don't think it does, really. It structures the code around that fact, but to take advantage of it, at a minimum you should replace

    while p <= twoOrThree {
        primes.append(p)
        var q = p * p
        let step = p * (p - 1)
        while q <= n {
            composites[q] = true
            q += step
        }
        p += 1
    }

with

    while p <= twoOrThree {
        primes.append(p)
        p += 1
    }

which in my testing gives a significant speedup.

To maximise the advantage, you could reduce composites to only store flags for \$6X \pm 1\$. Proof of concept code (could be tidier):

    var pidx = 1
    p = 5
    while p <= squareRootN {
        if !composites[pidx] {
            primes.append(p)

            var qidx = 3 * pidx * (pidx + 2) + 1 + (pidx & 1)
            let delta = p << 1
            let off = (4 - 2 * (pidx & 1)) * pidx + 1
            while qidx < composites.count {
                composites[qidx - off] = true
                composites[qidx] = true
                qidx += delta
            }
            if qidx - off < composites.count {
                composites[qidx - off] = true
            }
        }

        pidx += 1
        p += 2 + 2 * (pidx & 1)
    }

    while p <= n {
        if !composites[pidx] { primes.append(p) }
        pidx += 1
        p += 2 + 2 * (pidx & 1)
    }

This gives a moderate speedup in my testing.


One of the most important concept of Object Oriented Programming (and programming in general) is information hiding and encapsulation. A class should never expose its attributes unless there are some very valid reasons to do so (for example to share a constant). The fields of a class should be made available providing getters and setters. The reasons of that are many. For example if you have that field finalGrade public and a day you will want to change some details of that field, e.g. use a subclass of Grade, you will cause a long cascade of errors wherever you made a reference to that object without a getter method. We could spend a lot of time talking about how information hiding is important in OOP, but in short...your peer is right!


Shorten code that gathers files from directories

Unless I'm misinterpreting your code, you should be able to get by with something like this:

def spec_context
  directories = spec_repo.select { |path| File.directory?(path) }
  files = directories.map do |dir|
    spec_type.map { |type| Dir["#{dir}/**/*.#{type}"].sort }
  end
  files.flatten.uniq
end

That should be well within the line-limit (you could actually write that in one line, but that'd be kinda' annoying to read).

Point is, we filter to get the directories first, rather than in an if...else, and we map rather than push stuff to an array.

By the way, the title of question is a little confusing. Yes, the code "sorts file names", but only within their directory structure. Perhaps you want to put that sort at the very end, after the flatten, when you got all the files in a single array?
Also, as far as I can tell, the call to uniq isn't actually necessary; the files you'll find will either be in different directories or have different types, so there shouldn't be any repeats. But, if the addition of user configuration might cause repeats, I'd suggest handling that filtering elsewhere, so spec_repo and spec_type are sure to always return arrays with unique items only. Again, you should be able to get rid of the call to uniq here. Perhaps something similar could be done to make sure the spec_repo array only contains directories (assuming that's all it's supposed to contain) so we avoid doing that filtering in the method above.

Speaking of those methods, I'd suggest plural names for those two methods, since they return arrays. Similarly, the name spec_context doesn't quite communicate that the method returns an array of file names. It might be OK within the jargon of your code, but taken out of context, I might prefer something like spec_files as it's slightly more descriptive.

You also seem to be prefixing all your methods (the ones shown here, at least) with spec_. You might consider creating another class for this logic, since prefixing method names is indicative of methods that should be separately encapsulated.

Responding to the questions posed in the comments:

re method names: No, a method name doesn't need to follow any conventions or declare at its return type. In fact, if you overdo it, the code just becomes weirdly stilted. E.g. a method that returns a path ought to just be called path, not path_string - it's just redundant. Besides, Ruby doesn't have strict types, so trying to pseudo-declare types is going against the grain.

But hinting at what something is is very useful, and helps self-document the code. For example, I usually use plural for array variables or methods that return array. Also, somewhat particular to Ruby, a method call without arguments looks like any other variable, since we don't need to add () at the end. For instance, a line like directories.map ... looks the same whether directories is a local variable or a method. So if you were to refactor the code above and extract the first line into a separate method, you might as well call that method directories, since, hey, the name's worked well so far and it's pretty descriptive:

def directories # same name, but now a method
  spec_repo.select { |path| File.directory?(path) }
end

def spec_context # besides removing a line, nothing needs to change here. Yay!
  files = directories.map do |dir| # looks just like before
    spec_type.map { |type| Dir["#{dir}/**/*.#{type}"].sort }
  end
  files.flatten.uniq
end

You could go with directory_paths to further indicate that it's paths we're talking about, but... meh, not necessary. The simpler the better in my opinion. That said, it's always a judgment call.

You certainly shouldn't go for something like setup_spec_context; if it's already in a module called Setup, it's totally redundant to repeat that in the individual method names. For instance, the uniq method isn't called array_uniq, because, well, we know it's an array already.

re the word context: From the code and what you're describing, it sounds like a "context" in your case is actually more than just a list of files. It's also the directories to search, and the file types to search for (if not more). So you may want something like a Context class. Then you could have two instances of that class, spec and execution, each containing all the relevant stuff (spec.files, spec.types, spec.directories, etc.). Basic OOP encapsulation; modelling the notion of a "context".

If you find yourself prefixing the names of a bunch of methods, what you actually want is likely a class to contain those methods, so they're properly encapsulated.


users.Where(user => cond && cond2) is equivalent to users.Where(user => cond).Where(user => cond2)

So you can do the following:

IEnumerable<User> resultStream = users;

if(searchTerm.Length > 0)
    resultStream = resultStream.Where(user.Name == searchTerm);

switch(selectedValue){
    case 1:
        return resultStream.ToList();
    case 2:
        return resultStream.Where(user=>user.Street == "StreetA").ToList();
    case 3:
        return resultStream.Where(user=>user.Street == "StreetB").ToList();
}


I have no idea what this does:

if (i == 3) {
    temp = temp1;
    temp1 = temp2;
    temp2 = temp;
}

Please don't say if (test == true) or if (test == false)

but just if (test) or if (!test)

and you can just say else instead else if (test == false)

But your code in overly complex anyway. How about just this:

    bool[] doors = new bool[101];
    for (int step = 1; step <= 100; ++step) {
        for (int door = step; door <= 100; door += step) {
            doors[door] = !doors[door];
        }
    }

    for (int door = 1; door <= 100; ++door) {
        if (doors[door]) Console.WriteLine(door);
    }

You'll see that the output is all the square numbers: 1, 4, 9, 16, 25, 36, 49, 64, 81 and 100


Optimize sorting of Array according to Distance (CLLocation)

Winston's answer points out some improvements to your algorithm.  There are other ways in which we can slightly improve the speed of this.

In Objective-C, for loops (and while and do...while) loops are handled one iteration at a time and the exit condition is checked on each iteration of the loop.  Meanwhile, a forin loop is handled in batches.  When we're dealing with an array for example, a forin loop might cache the first 8 objects and work with them before it goes back and grabs more objects.  In this way, we save time by calling fewer messages on the collection.  Moreover, when it caches these objects, you've already got a reference to the object, so you don't have to waste yet another message to the collection to grab a reference to the object.  Long story short, forin loops are faster than for loops in Objective-C, and you should use them when you can, even if you don't need the reference to the objects in the collection...

In this case though, we're actually wasting tons of execution time making calls back to the collection.

Moreover, NSString has a method called doubleValue which shouldn't have any trouble pulling a double value out of a string...

So let's clean up the first loop to look like this:

for (id object in originalData) {
    double latitude = [[object latitude] doubleValue];
    double longitude = [[object longitude] doubleValue];
    CLLocation* currentLoc = [[CLLocation alloc] initWithLatitude:latitude
                                                        longitude:longitude];

    CLLocationDistance distance = [searched distanceFromLocation:currentLoc];
    NSNumber* num = [NSNumber numberWithInt:distance];
    [keysArray addObject:num];
}

There are some things that are curious to me however.

What's the data type of the objects in the array?  Why does it have properties latitude and longitude which you clearly need to do math things with yet they're stored as NSString objects?  These should be stored as NSNumber objects or just double primitives preferably.

CLLocationDistance is a typedef for double.  Why are we then taking this double and instantiating an NSNumber object with numberWithInt:?


Why would I want to always have to explicitly call a "base" method if I just want to use base functionality?

The problem with building a good base class is that ideally, you should know exactly what kind of functionality will the derived classes need from the base class.

If you think they'll need more than is actually necessary, you'll write more code unnecessarily and it will also make the base class harder to use. On the other hand, if you think the derived classes will need less, your base class will be hard (or impossible) to use for those advanced scenarios.

For example, in your specific case, imagine you would need to call the base Get() with different IRepositorys. With your implementation, that's hard to do (though not impossible, you use a hack like IRepository wrapper where the wrapped IRepository can be switched, but ugh). With the architect's implementation, that's a trivial thing to do: just pass different IRepository to each call of InternalGet().

But again, choosing between the two depends on expectations of what the derived classes will need. If a scenario like I just described is unlikely, then your implementation is better, because it's simpler. But if that scenario happens, your implementation is clearly insufficient and the one from the architect is clearly better.


I'm sure someone else can provide a more comprehensive answer than I could, Just a few things:

string::at throws an out-of-range exception, since you're not actually catching anywhere, I'd use [] since at() is slower and you're already checking the string length, not that it really matters that much for this simple code though.
I would factor out the std::wstring(argv[1]) and similar into local variables so there are fewer brackets.
if statements without braces are less maintainable.
wmain is windows specific, so this code won't compile elsewhere.


I solved the problem. Here is the code:

Set rngSearch = Sheets("Sheet1").Range("D:D")
For Each rngSearch In Drng
    'all activities are searched in Arng:
    Set rFnd = Sheets("Sheet1").Range("D2:D" & lastRowD).Find(What:=sText, LookAt:=xlPart)
    Set rngFound = Arng.Find(What:=rngSearch, LookIn:=xlValues, LookAt:=xlWhole, SearchOrder:=xlByRows, SearchDirection:=xlNext, MatchCase:=False, SearchFormat:=False)

    If Not rFnd Is Nothing Then
        rngSearch.Offset(0, 0) = rngSearch.Offset(-1, 0) 'if found multiple strings, don't loop column A but copy the row above
    Else
        'if activity is found:
        If Not rngFound Is Nothing Then
            rngSearch.Offset(0, 0) = rngFound.Offset(0, 1)
        Else
            rngSearch.Offset(0, 0) = "0"
        End If
    End If
Next


Design

Since duplicates are allowed (no Set required) and you don't need to fetch books by name (no Map required), ArrayList will do just fine.

Overview Collection Types

Guard public entrypoints

Make sure to checks arguments on all publically available code that takes input from calling code.

if (novels == null) throw new IllegalArgumentException("novels must be set");

Use plural for variable names that represent collections

private List<Novel> novels = new ArrayList<>();
private List<ArtAlbum> artAlbums = new ArrayList<>();

Initialize object members only once

Don't create the ArrayList instances twice. You can use addAll to add a range of items to a list. Perhaps you also want to create public methods to perform these actions.

public class LibraryCatalog {
        private List<Novel> novels = new ArrayList<>();
        private List<ArtAlbum> artAlbums = new ArrayList<>();

        public LibraryCatalog(List<Novel> novels, List<ArtAlbum> artAlbums) {
            if (novels == null)
                throw new IllegalArgumentException("novels must be set");
            if (artAlbums == null)
              throw new IllegalArgumentException("argument must be set");
            this.novels.addAll(novels);
            this.artAlbums.addAll(artAlbums);
        }
    }


Invert bits of a number in clojurescript

First, fixing a few of the errors that I noted in the comments and altering the formatting a bit, I have:

(defn invert-number [n]
  (+ 1 (* -1 n)))

(defn number-complement [num]
 (let [numstr (.toString num 2)
       bits (map (comp invert-number js/parseInt) numstr)
       complement-bits (reduce str bits)]

   (js/parseInt complement-bits 2)))

(println (number-complement 10)
         (number-complement 12)
         (number-complement 15)
         (number-complement 993061001)
         (number-complement 123))

; 5 3 0 80680822 4

I prefer to have defn function definitions on a separate line, and have at least one empty line between function definitions.

invert-number can be fixed up a bit. - can actually be used as an unary operator to do a negation, and inc is arguably more idiomatic than + 1 unless you think you may need to add to the equation later. I changed it to:

(defn invert-number2 [n]
  (inc (- n)))

Since the entire purpose of the function though seems to be just toggling between 0 and 1, I'd just write it as a more explicit toggle:

(defn invert-number2 [n]
  (if (zero? n) 1 0))

I feel like that conveys the purpose much clearer. I'd expect it to perform similarly too.

You can get rid of the first call to js/parseInt by just using int:

(map (comp invert-number2 int) numstr)

Honestly, I don't know why this works. int here is not acting like it does in Clojure (which is what I'm familiar with; I don't actually know Cljs). I'm guessing this is due to some weirdness on Javascript's end. If I plug that into a Cljs transpiler, I get some weird clues:

(int "192837465")

becomes

("192837465" | (0));

Because... Javascript happened?

The second js/parseInt is a little harder to deal with though because of the radix argument. I'd just stick with what you have.

(reduce str bits) can be changed to (apply str bits). Many variadic functions automatically manually reduce over their arguments (like str and +), so you you can alternatively apply the list directly to the function. (apply str makes a little more sense to me, but that's likely because I've written that many times before.

In the end, I ended up with:

(defn invert-number2 [n]
  (if (zero? n) 1 0))

(defn number-complement2 [num]
 (let [numstr (.toString num 2)
       bits (map (comp invert-number2 int) numstr)

       complement-bits (apply str bits)]

   (js/parseInt complement-bits 2)))

(println (number-complement2 10)
         (number-complement2 12)
         (number-complement2 15)
         (number-complement2 993061001)
         (number-complement2 123))

; 5 3 0 80680822 4


Formatting milliseconds as days, hours, minutes, and seconds

The Decimal Point

Exponential notation is by far the better for large numbers.

However when writing numbers in this format the convention is to have the decimal point after the first digit, as the exponent represents the magnitude of the value which is obscured if you have to locate the decimal point.

d = Math.floor(t / 8.64e7),
h = Math.floor(t % 8.64e7 / 3.6e6),
m = Math.floor(t % 3.6e6 / 6e4),
s = Math.floor(t % 6e4 / 1e3);

As constants

But.. These are magic numbers, and you repeat some of them several times, which is prone to error. Also assuming that this function would be part of a set of such functions declaring these constants as named variables would be much better.

DAY_MS = 8.64e7;
HOUR_MS = 3.6e6;
MIN_MS = 6e4;
SEC_MS = 1e3;

Derived values

As they are derived from each other you can then write it as

SEC_MS = 1e3;
MIN_MS = SEC_MS * 60;
HOUR_MS = MIN_MS * 60;
DAY_MS = HOUR_MS * 24;

Encapsulate

If you then encapsulate them you can drop the _MS and your function would look like.

"use strict";
const dhms = (()=>{
    const SEC = 1e3;
    const MIN = SEC * 60;
    const HOUR = MIN * 60;
    const DAY = HOUR * 24;
    return time => {
        const ms = Math.abs(time);
        const d = ms / DAY | 0;
        const h = ms % DAY / HOUR | 0;
        const m = ms % HOUR / MIN | 0;
        const s = ms % MIN / SEC | 0;
        return `${time < 0 ? "-" : ""}${d}Day(s) ${h}Hour(s) ${m}Minute(s) ${s}Second(s)`;
    };
})();

Notes

That t is made positive ms to avoid a negative sign on each number. This also lets you use the shorter and quicker | 0 (bit-wise OR 0) to floor the values, which you should only use for positive integers.
The use of a template string to format the output.
You could also define it as a module and thus avoid the need to encapsulate the constants as a module has its own local scope.


Return all anagrams for a word matched against a list of words in a file

I'm going to review your Main code and let someone else address the Test code.

I'll start with a few minor issues.

Breaking up long lines often makes them easier to scan/parse/comprehend.

def isAnagram(word: String, anagram: String) : Boolean =
  word.toLowerCase.toSeq.sorted.unwrap == anagram.toLowerCase.toSeq.sorted.unwrap

Also, you're using .toSeq to get to the non-deprecated .sorted method, but there's no reason to .unwrap it to get back to a String. The == comparison will work on a WrappedString.

Using.apply() returns a Try[A], which your code ignores. If an exception is thrown (words.txt isn't where it should be?) then your app silently returns without reporting any errors. You might want to consider Using.resource() instead. It won't try to catch any exceptions.

Using pattern matching just to determine if anagrams is populated or not seems a bit bizarre. In fact, it doesn't need to be a List at all.

val anagrams = source.getLines()
                     .filter(isAnagram(word, _))
                     .mkString(", ")

if (anagrams.isEmpty) println("none")
else                  println(anagrams)

A big, big, problem with your algorithm is that the user-input, word, is processed for comparison (toLowerCase and toSeq and sorted) for every word in the dictionary. (*Ouch!*) It should only be done once.

But my biggest issue with the design is your choice of user experience (UX). Prompting for user input has many pitfalls. I won't go into the whole rigamarole about using the IO monad in an FP context, instead I'll just point out that simplifying the user I/O can greatly enhance the app's utility.

What if the user wants to count the number of anagrams for a given word? If your app had a small and simple usage profile (usage: anagrams <word>) then the task would be easy.

%%> anagrams Star | wc -w
5
%%>

From there you can imagine simple feature enhancements to make it more useful.

%%> anagrams Star bets ENDING
Star: arts, rats, tars, tsar
bets: best
ENDING: ginned
%%>


I'd improve two things:

take advantage of ability to specify multiple clauses for anonymous function,
use underscore for big numbers to improve code readability.

My take on your code:

{0, 1}
|> Stream.unfold(fn {a, b} -> {a, {b, a + b}} end)
|> Enum.reduce_while(0, fn
  value, acc when value < 4_000_000 and rem(value, 2) == 0 -> {:cont, acc + value}
  value, acc when value < 4_000_000 -> {:cont, acc}
  _value, acc -> {:halt, acc}
end)


You should structure your code a bit better and improve your naming conventions.

The first loop in main() apparently scans 6 strings into ip. From that I might deduce that ip is supposed to hold your input. So name it input or even inputProductions. Current IDEs support code completion so not using longer names for the sake of less typing is not a valid argument.

You could also put that code into a separate function readInput(char **result). Later you can adjust how you read your input without changing main.

Then you have a couple of nested loops which compute some values to be stored in b and a value c. Using i and j as loop counter is accepted practice so that's ok but what are b and c supposed to represent? Give them names which express their meaning.

Also your outer loop counter i can go up to 5 and j < i + 1 means j can go up to 5 as well. b is only an array with 4 elements, so the last index you are allowed to access is 3 and b[j] would be out of bounds for any j > 3.

If that nested for loop is supposed to do some pre-processing then put it into a function and give it a sensible name which expresses what it is doing.

You have a variable called cnt - what exactly is it that you are counting? Name the variable accordingly.

Your functions access global variables like cnt or ip. You should pass all data those functions require as parameters. There is not need to create implicit dependencies.

Organize your code so its concerns are separated: input parsing, data processing, output. Your main function should read something like this:

int main()
{
    char input[7][7];
    char b[4];
    int count;

    if (!readInput(input))
    {
         puts("Failed to read input.");
         return -1;
    }

    count = preProcessInput(input, b);
    if (count < 0)
    {
        puts("Failed to pre-process input");
        return -2;
    }

    if (!findLeadingSet(input, b, leadingSet, count))
    {
        puts("Failed to process input");
        return -3;
    }

    printLeadingSet(leadingSet);

    return 0;
}

This makes the structure immediately clear and explains what the steps are without having to read all the code and trying to understand it (apparently preProcessInput should be named as to what it is actually pre-processing).


Notify user of credential failure and make another attempt

I found the solution to removing the do. I needed to move the error presentation into the flatMap and use materialize so that a network error wouldn't break the observable chain.

// MARK: - function Under Test
func checkCredentials(login: Login, network: Network) -> Observable<Token> {
    let credentials = login.getCredentials()
    let token = credentials.flatMap { (credential) -> Observable<Token> in
        let token = network.getAuthToken(credentials: credential)
            .materialize()
        let error = token.map { $0.error }.filter { $0 != nil }.map { $0! }
        login.presentAuthFailure(error)
        return token.map { $0.element }.filter { $0 != nil }.map { $0! }
    }
    return token
}


Actually, I find the structure of the program quite complicated and difficult to read and understand. I think that the reason for this is that the program structure is patterned on the shape parameter, whose limitation to three elements is, in my opinion, rather arbitrary.

Why do not eliminate such limitation, and make the function more similar to the original numpy one, in which shape can be any list, even an empty list?

The elimination of this restriction could seem to complicate the problem, but instead it simplifies the function if we use a recursive approach on the structure of shape:

if shape is empty, then generate a single random number
if shape is a list, get the first element, which is a number n, apply n times the function to the rest of the shape and collect the result in a list.

In this way the function is greatly simplified:

(defun randint (&key (start 0) (end (1+ start)) (shape '()))
  (if (null shape)
      (+ start (random end))
      (loop as i below (car shape) collect (randint :start start :end end :shape (cdr shape)))))

Finally, I don't like the fact that a function has a key parameter which, if omitted, causes an error (end). So I would prefer to give it an optional value (for instance start + 1). Finally, to complete the similarity with the original function, I think we can default the shape parameter to the empty list, so that the call (randint :start n :end m) produce a single integer in the range specified.


Project Euler - Largest Product In A Grid

Stylistic it is mostly nice, and I would possibly change two things. First of I would put all of the code within a function allowing for reuse, and secondly I would change some of the names, i.e. backward_diag and forward_diag.

From an algorithmic point of view there are however some more issues with your code:

Use range(x) when you don't care about the value – Usually it is good to do for column_value in row, but as you dereference using indexes, and don't use the column value at all, I would rather using pure indexing, and let the double loop be:

for y in range(len(grid)):
    for x in range(len(grid[y])):

What's the point with used? – I don't understand why you store the starting value of the largest value. In itself this value is not useful, as any value could be found multiple places, and you don't store which direction the largest value was used.

If storing something I would rather store the indices and direction, something similar to (x, y, "left") where you vary the direction accordingly to current search
Don't repeat the value if a > biggest else biggest – Instead of doubling the if ... else I would rather use a full blown if statement. In other words instead of

left = reduce(operator.mul, grid[y][x - 4:x], 1)
used = grid[y][x - 4:x] if left > biggest else used
biggest = left if left > biggest else biggest

I would rather use:

left = reduce(operator.mul, grid[y][x - 4:x], 1)
if left > biggest:
    used = (x, y, "left")
    biggest = left

Don't repeat calculation – As Zack commented upon I wouldn't calculate both the left and right, and similarily the up and down, as these are redundant.
Strange validation of ranges – The only range you check is the x + 4 < 20, but you don't do any validation on other ranges. This means that you calculate the left even though you don't have 4 values to the left of current value, and so on. This is kind of sloppy, and could easily be avoided.


Finding minimum scalar product using ST Monad

Somewhat old question, but still it's a pity it hasn't been answered :). Putting aside the asymptotically better solution suggested in the comments, and focusing just on the code:

Pluses: Top-level types for all funtcions, that's very good, as well as comments. The code shows good understanding of various Haskell functions and idioms.

My most general comment is that there is disproportion between code verbosity and complexity. Usually more complex code should be more verbose and more descriptive. Visually, most of the code is just simple read/write/swap ST operations, but the important part is squeezed into not-so-easy-to-understand and uncommon combinations of folds/scans/sequence/>=>/>>= etc. I'd probably separate the simple parts (read/swap) into helper functions and expand/simplify the complex parts a bit.

Function adjust somewhat mixes pure and mutable approaches. While it mutates its first argument, it keeps the scalar product pure and passes it as an argument in and out. This is somewhat confusing and complicates the computation of the minimum (scanl with >>=). If the product were a ST variable too, the code gets simpler (untested):

adjust :: MVector s Int -> Vector Int -> STRef s Int -> (Int, Int) -> ST s Int
...
msp = ...
  x <- newSTRef $ foldl' (+) 0 $ zipWith (*) u v
  fmap minimum . traverse (adjust u' v x) . transpositions $ n

Alternatively you could even keep the result in an ST variable.

Also foldr with >=> can be replaced with foldM (untested):

foldM (flip $ extend p q) [] [2..n]

(Then it'd make sense to refactor the type of extend to get rid of the flip.)

Some questions:

Is replaying all ts in extend necessary? As it's executed repeatedly, it'd make sense to compute the combined permutation of ts separately or once and then just apply it.
Which variant of the algorithm is actually used? Using two vectors doesn't seem to match anything in the linked Wikipedia article. While the general idea of the algorithm is clear, it'd be easier for someone not familiar with it to get more precise guidelines.

Nit: The order of functions is somewhat unnatural: transpositions uses extend, msp uses transpositions and adjust. There is no visible order in this. One good option is that functions are always defined before being used (with the obvious exception of mutually-recursive functions). Or the other way around - the main/exported functions are first and helper functions follow. Or, separate functions into (possibly nested) sections, then ordering of functions doesn't really matter, ordering of sections becomes important.


Generating the largest decent number of given number of digits

Since you're asking where the code is failing:

Your logic is flawed for at least the case n % 5 == 0 in which case you assume the whole number must always be made of 3's but that is wrong since for example for n = 15 the number consisting of all 5's would be larger and still fulfill the requirements.

Also your code invokes undefined behaviour since flag might be used before it is initialized in the case n % 5 == 0.

Ultimately what you are looking for is a solution to:

3 * f + 5 * t = n

where f is the number of 5's and t is the number of 3's and n is the known length.

One simple way to solve this is to transform it into:

f = n/3 - 5/3 * t

Now all you need to do is to check if there is a solution for t with 0 <= t <= n where f is a whole number which ultimately boils down to finding the smallest t where

n % 3 == (5 * t) % 3

So you have the three cases. Together with the knowledge that 5 % 3 == 2 we can deduce:

n % 3 == 0: (5 * t) % 3 == 0 for t = 0, 3, 6, 9, 12, 15, 18
n % 3 == 1: (5 * t) % 3 == 1 for t = 2, 5, 8, 11, 14, 17, 20
n % 3 == 2: (5 * t) % 3 == 1 for t = 1, 4, 7, 10, 13, 16, 19

Since we're interested in the smallest t (so the resulting number has the maximum number of 5's and is therefor the largest matching one) this leaves you with roughly this pseudo-code algorithm

switch (n % 3):
    case 0: t = 0;
    case 1: t = 2;
    case 2: t = 1;

if (5 * t <= n):
    print "5".repeat(n - 5 * t) + "3".repeat(5 * t)
else
    print "-1"


Getting client timezone name to use with PHP date_default_timezone_set()

On the javascript side:

I am not seeing any value in creating the string format for the offset that you are creating, as you just end up spending more code in PHP trying to get to a usable format. This is all more work than necessary. Consider keeping offset in minute-based javascript format or converting to second-based format.
You seem to use your cookie management library inconsistently here.  You write using a jQuery extension like $.cookie() while reading the actual cookie string from document. Why not use your library for both (or does it not support that?)  Also, not sure what you what your library does with expires: null. Hopefully this set to session-length cookie?

On PHP side:

I would strongly consider using PHP's DateTime and related classes (in this case specifically, DateTimeZone) rather than the procedural date_* functions.  Over time I think you will find them much more powerful and flexible in usage.  This also can help move you away from the concept of resetting your default system timezone, which I don't think you want to overwrite here, but rather use as fallback.  There may be other parts of your code that do things like insert records into database datetime fields or something like that where you want to use a consistent system time, not the end users timezone.  I don't think you have an appropriate use case for changing this setting (at least from the context shown).

Let's show an example of how this might look using DateTimeZone:

$system_tz = date_default_timezone_get();

// assume $_COOKIE['hp_time_offset'] contains a minute based offset like
// output by javascript Date().getTimezoneOffset()
if (!empty($_COOKIE('hp_time_offset') &&
    is_numeric($_COOKIE['hp_time_offset'])) {

    $offset_min = (int)$_COOKIE['hp_time_offset'];
    $offset_sec = $offset_min * 60;
    // is current system in daylight savings?
    $is_dst = (int)date('I');
    // get timezone name
    $tz = timezone_name_from_abbr('', $offset_sec, $is_dst);
    try {
        $date_timezone = new DateTimeZone($tz);
    } catch (Exception $e) {
        error_log('Invalid timezone string of "' . $tz . '" passed from cookie.');
        $tz = $system_tz;
    }
}

if(empty($timezone)) {
    // I am not wrapping this in try-catch as if there is a failure here
    // I don't know what you want to do to recover (as this should not happen).
    // Feel free to catch if needed for your implementation
    $date_timezone = new DateTimeZone($tz);
}

return $date_timezone;
// or if you just want to return the timezone string
// return $date_timezone->getName();

This proposed functionality gives you a valid DateTimeZone object representing either the cookie-derived timezone or, as a fallback, the system timezone.  You can use this object to instantiate any DateTime object you may need on your system with those objects automatically taking on the expected timezone behavior.


Adding arbitrarily long integers C++ (string represented)

Prepending zeros one at a time is quite inefficient.  It could be improved by making a string of the appropriate length in one go:

{
    auto l1 = n1.length();
    auto l2 = n2.length();
    if (l1 < l2)
        n1 = std::string{"0", l2-l1} + n1;
    else if (l1 > l2)
        n2 = std::string{"0",  l1-l2} + n2;
}

Reassembling the string afterwards character by character is also inefficient.  This can be improved by telling the string how long it will need to be:

std::string o = "";
o.reserve(sums.size());

The index variables i should be an unsigned type to compare against string length or vector size - I recommend using the same type as those methods return.

Greater gains can be made by keeping the long integer as a chain of digits without adding '0' to make them printable characters at each stage; it also helps to store them in little-endian order (so that any carry gets appended rather than prepended).  See Digit-by-digit addition using vectors for a start.  You could reduce storage requirements by using base-100 instead of base-10 (char is guaranteed to hold ranging at least from 0 to 127).


Avoid redundancy of HTML entities in .NET framework

Why not just use the Server.HtmlEncode function?

Here is a brief description of what it does:

  Any ASCII code character whose code is greater-than or equal to 0x80
  is converted to &#<number>, where <number> is the ASCII character
  value.

It also converts common ascii characters such as <, >, &, and " to their html character references.


Using Array.prototype.filter() and Array.prototype.map()

You have a bug in your code. Filter expects true or false. Your still works because implicit type conversion of Javascript (object => true, undefined => false).

Correct would be:

var categories = this.information.filter(function (item) {
    return item.type === 'genre';
});

Filter/Map could be combined together if you like.

You're using variable from outside. You could improve testability and readability by using of arguments and return the result.

this.flattenCategories = function (information, separator) {
    // flatten in a single string all categories so to be easily visaulised in the dom
    return information
        .filter(function (item) { return item.type === 'genre'; })
        .map(function (item) { return item.value; })
        .join(separator);
}


Changes quantities that are displayed in a shopping cart page

You don't do a good job of validating $_POST['pid']. You just assume the value passed will be good (a bad mistake when working with user input).

May I suggest something like:

$filterOptions = array('options' => array('min_range' => 1));
$pid = filter_input(INPUT_POST, 'pid', FILTER_VALIDATE_INT, $filter_options);
if(is_null($pid) || $pid === false) {
     // parameter was either not set (null) or failed filter (false)
     // you should handle this somehow
     exit();
}

The two different code examples are not the same in that the cart array is build differently in each.  The first would build a numerically-indexed structure at $_SESSION['cart_array'] like this:

[
    0 => [
        'item_id' => {{pid}},
        'quantity' => {{quantity}}
    ],
    1 => [
        ...
    ],
    ...
]

Whereas the second would build an array index by item id like this:

[
    {{pid}} => [
        'item_id' => {{pid}},
        'quantity' => {{quantity
    ],
    {{pid2}} => [
        ...
    ],
    ...
]

Did you really intend to change this structure? If so, are there other areas of code that might need to change to accommodate this changed structure?

$result = print_r(array_keys('cart_array',$pid))

This makes no sense. cart_array is not an array suitable for use with array_keys().  Why would you use print_r() (an output function) here at all?

If you want to use the second form of the cart data shown above, this is how you would extract pid values from it:

$pidsInCart = array_keys($_SESSION['cart_array']);

With the numerically-indexed cart array, it might be:

$pidsInCart = array_map(
    function($item) {
        return $item['item_id'];
    },
    $_SESSION['cart_array']
);

I would also would be careful in evaluating the result directly, as you could get unexpected truthy/falsey results there I might use:

$pidsInCart = ...;
if(!empty($pidsInCart)) {
     ...
}

Also, use meaningful variable names. $result here is a good example of a not very meaningful name in this context.

Indent your code properly.


Upgrade script for MediaWiki website with one external addon (wave 1)

Make the process approach atomic

The script first wipes out ${war}/${domain},
then populates it with content from wget commands,
and restores content from backup.

The wget commands are a serious risk here.
If any of the source sites is down or experiencing instability at the time the script runs,
your site can end up in an inconsistent state.

At the minimum, it would be better to fetch all external resources before you start rewriting your content.

An even better way would be to try to make the process as atomic as possible:
build up the new content in a dedicated directory isolated from production,
but on the same partition,
and when all is ready, make the switch in fast directory rename operations.

Separate your backups from your production environment

The $war directory looks clearly in the heart of the production environment.
In that directory, you create backup directories side by side with the $domain directory.
I suggest to create the backups elsewhere.
Ideally on a different filesystem partition.

Closely related to this, the backup directories are somewhere in $war/...,
and the original content is also somewhere in $war/....
I think this invites mistakes.
If you had distinct variable name prefixes such as $backup_... and $content,
that would reduce the risk of mistakes.

Syntax error

I don't understand what this is doing in a Bash script:

db-username_and_db-name="DB_CREDENTIALS"

This is an invalid statement in all Bash versions that I know.

Bash scripts should have a shebang

The posted code has no shebang.
Since the question is tagged bash,
maybe it's implied it's really there in your version,
but just for the record, every Bash script should start with a shebang.

Watch out for things that may go wrong

Toby already mentioned this in his review,
but I think it deserves to be stressed.
Many things can go wrong in this script:

If mysqldump fails for some reason, the script will keep going without a backup created. That can lead to data loss, and it's to be avoided.
If any of the wget commands fail for some reason, the script will keep going and update the sitemap and the DB. That can lead to inconsistent state of your site that's difficult to debug.

My suggestion is similar to Toby's:
put set -euo pipefail near the top of the script,
to make it stop executing when something goes wrong unexpectedly.

Double-quote variables used in command arguments

You did that correctly for the most part,
you missed just one here:

cp "${war}/${domain}"/${domain}.png               "${war}/mediawiki_specific_backups"/${domain}.png

Use more variables

The term ${war}/mediawiki_general_backups comes up repeatedly.
It would be better if it was in a variable.
Editors with Bash support will make it possible to rename, see, find easily all occurrences.

Make long pipelines easier to read and understand

Take for example this:

wget "${latest_mediawiki_core}" -O - | tar -xzv --strip-components 1 -C "${war}/${domain}"

I think it's better to write flags before arguments.
The text closer to the left is naturally easier to read.
And the flags pack a lot of important details in little space,
and the URL argument is long and not very interesting to read.

Secondly, it's easier to understand code when there is a single statement per line.
A good way to achieve that is by splitting the line at each command in the pipeline.

That is, I suggest writing like this:

wget -O- "${latest_mediawiki_core}" \
| tar -xzv --strip-components 1 -C "${war}/${domain}"

Simplify

Instead of this:

rm "${war}"/mediawiki_specific_backups/*
rm "${war}"/mediawiki_specific_backups/.* # If I won't run this, a specific backup of .htaccess, in that directory, won't get deleted;

Why not simply:

rm -r "${war}"/mediawiki_specific_backups


"Increment" a string of uppercase characters

A slight simplification of the if else structure is possible:

def increment_str(s):
    lpart = s.rstrip('Z')
    num_replacements = len(s) - len(lpart)
    new_s = lpart[:-1] + increment_char(lpart[-1]) if lpart else 'A'
    new_s += 'A' * num_replacements
    return new_s


Efficiently index rows of numpy array by exclusion

Since the result's buffer will have a gap, compared to the original, it will have to be a copy.  I believe delete takes different approaches depending on the inputs.

One approach is a boolean index, e.g.

ind = np.ones((10,), bool)
ind[n] = False
A1 = A[ind,:]

Another is to do the equivalent with index values

ind = range(n) + range(n+1:A.shape[0]] # using list concatenate
A1 = A[ind,:]

And as you note, using that index with take may be faster than direct indexing.  I doubt if the difference is big, but I haven't timed it recently.

ind could also made by concatenation of 1d arrays.  Alternatively, index the two parts, and concatenate them:

 np.concatenate([A[:n,:],A[n+1:],axis=0)

The inputs to concatenate are slices, but result is a copy.

 np.r_[0:n, n+1:A.shape[0]]

is a convenient way of generating the integer index list - but not necessarily a speed solution.

Why is the difference time difference between a view and a copy significant?  If you do it a few times in the program, it shouldn't matter.  If you do this deletion repeatedly, I'd question the larger program design.  Could, you for example, accumulate the deletion indices, and perform the deletion step just once?

A few timings:

In [17]: arr=np.arange(1000)

In [18]: timeit arr[np.r_[:500,501:1000]].shape
10000 loops, best of 3: 55.7 us per loop

In [19]: timeit arr.take(np.r_[:500,501:1000]).shape
10000 loops, best of 3: 44.2 us per loop

In [20]: timeit np.r_[:500,501:1000]
10000 loops, best of 3: 36.3 us per loop

In [23]: timeit ind=np.ones(arr.shape[0],bool);ind[500]=False;arr[ind].shape
100000 loops, best of 3: 12.8 us per loop

Oops, the boolean index is faster in this test case.

Best yet:

In [26]: timeit np.concatenate((arr[:500],arr[501:])).shape
100000 loops, best of 3: 4.61 us per loop


Ensuring user input is an integer in a range

The second sample is definitely better. I would be inclined to continue to improve the code by separating mechanisms from policies. The policy is the code that actually expresses the meaning of the program; the mechanism is the code that expresses what specific operations implement the policy.

This idea comes from security design; you don't want the code that computes "is Bob allowed to open this door?" and the code that computes "is that really Bob's card key?" to be the same code. The first is a policy, the second is a mechanism. But it applies generally to all code.

By separating mechanism from policy we enable both to be understood more easily. The "main line" of your code should read like the specification. If you had to describe your code in English you'd probably say something like "present the user with a choice of gestures. If the user chooses an invalid gesture, keep trying until they choose a valid gesture."  That's the meaning of your program, but that's not what your code looks like. Rather, your code looks like the most important things in the world are integer and bool variables, list counts, and so on.

Let's identify a mechanism: parsing an integer and testing whether it is in range is the mechanism behind the policy of "the user must choose a valid gesture". So let's isolate that mechanism into a purely mechanistic method, that knows nothing about your policy domain:

static class Extensions
{
    public static int? BoundedParse(this string str, int lower, int upper)
    {
        if (str == null)
            return null;
        int result;
        bool success;
        success = int.TryParse(str, out result);
        if (!success)
            return null;
        if (result < lower)
            return null;
        if (result > upper)
            return null;
        return result;
    }
}

Let's now rewrite your loop as a do-while instead of a while. Look how much ugly mechanism we removed, leaving your policy shining brightly through:

gameSetup(listOfGestures);
int? choice;
do
{
    Console.WriteLine("Please choose your Gesture ");
    choice = Console.ReadLine().BoundedParse(1, listofGestures.Count);
}
while(choice == null);
playerGesture = listOfGestures[choice.Value - 1];

And as a bonus, we now have a reusable method that we can apply to any task that requires that a string be parsed as a bounded integer.

We can take this even further. Isn't that loop a mechanism? Move it into a helper method:

static int PromptForNumber(string prompt, int lower, int upper)
{
    int? choice;
    do
    {
        Console.WriteLine(prompt);
        choice = Console.ReadLine().BoundedParse(lower, upper);
    }
    while(choice == null);
    return choice.Value;
}

And now your method is:

gameSetup(listOfGestures);
int choice = PromptForNumber("Please choose your gesture", 1, listofGestures.Count);
playerGesture = listOfGestures[choice - 1];

Now the code is super clear what is going on at the policy level because all the stuff you don't care about is now the responsibility of some mechanism somewhere. (And again we have a great mechanism for prompting for a number that we can re-use later.)

Even better: maybe there is a better way to display a prompt and get a number. If we decided that we wanted to write a GUI instead, we can change the mechanism code without changing the policy code. The policy code doesn't care how the valid gesture gets chosen, just that it does.


Calculating the connectance of a matrix iteratively

Without installing bipartite and getting into the exact details of what the actual calculations are looking to do, there are a few things that jump out at me that might help you.

Growing the Object (connectMatrix)

It looks like you're in Circle 2 - Growing Objects of the R Inferno.
Specifically, the line where you've got:

#Stitch matrix together
connectMatrix[k,] <- c(site, method, subSize, con)

In that scenario, every time you add another row to the connectMatrix, you're expanding and fragmenting the memory you're using, which as you've observed starts out okay but then starts to really drag. Here's another resource on that same issue, and a much less intricate Stack Overflow question.

nested for loops

You've got three layers of nested for loops

for(i in 1:length(unique(df$SiteMethod))){

    #subsetting dataset
    ...
    ...
    ...

    for(l in 1:numRep){
        #Beginning calculation loop
        for(j in 1:length(size)){

        ...
        ...

        }
    }

The outside for loop jumps out at me as something the plyr package might be good for -- essentially it looks like you're splitting the main dataframe into subsets, then doing things to it (the meat of your second and third for loops). Basically, plyr was designed for splitting-applying-and combining, and there's a really great paper on how to implement it.

You may be able to do two (or maybe even three) layers of splitting-applying-combining.

If the data lend themselves to grouping operations, those tend to help, too, but it doesn't seem like it based on the comments in the code.

Parallel processing

If, after you've optimized for not growing the object and chunked the code for splitting/combining (or another method of getting to the same outcome), you're still not at the level of performance you're looking for, you can journey into parallel application.

Basically, parallel application works well when you're using apply or plyr approaches and you can convert over to mcmapply or one of the other parallel approaches.

For example, we use mcmapply in production at work to take a 30 minute process and trim it down to about 6 minutes. Parallel processing tends to be worth it (in my personal experience) only once you've done everything else you can to get to the best speed you can without parallelizing - it tends to be more work to get it right, and, as a bonus, once you've done the other things it tends to make it easier to convert the process to run on multiple cores.

Finally, Efficient R is a good general resource on optimizing R processes.


Looks fine to me, but I do have a few recommendations:

Instead of calling the onclick handler inline, consider doing the following:

$('#close').click(function(e) { fadeBox(e); });

Then I would pass e to fadeBox() and do e.preventDefault() so that the click event doesn't throw the user to the top of the page.

Also, you do not need to create the function hideLightBox() if the only thing it is doing is calling fadeBox().


Naming index and nextIndex as i and j would have been OK.

There are no comments, which is not necessarily a problem. But a one-line javadoc for the function would be helpful. I know, I know, from the signature it's kind of obvious, but still. For example, what if caller passes in null? Well, we throw a fatal exception, and that seems appropriate, caller gets what he deserves, he violated the contract spelled out in the javadoc. But only if there is a javadoc explaining the caller's responsibility.

The inner while loop would be more naturally expressed as a for loop.

The inner while loop might become a helper function. Then you'd have the opportunity to give it an informative name, perhaps copy.

That said, overall the function as written is reasonably clear.