Verifying user accounts on SMTP servers I've been writing a basic script to enumerate SMTP users (via a user dictionary) on poorly configured SMTP servers. In scripts like this, I usually see arguments handled as follows:

if (len(sys.argv) != 3:
    print "Usage: ..."
    sys.exit(0)

I found the if and print statement approach irritating, but without a concrete reason why in my mind. I decided to use assertions and combine argument handling with general setup. The following is the complete script.

#!/usr/bin/python

import socket
import sys
import os

SMTP_PORT = 25

#
# Helper Functions
#

def create_connection(host, port):
    smtp_server = sys.argv[1]
    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    sock.connect((host, port))

    return(sock)

def smtp_verify(username, smtp_conn):
    smtp_conn.send('VRFY ' + username + '\r\n')
    smtp_reply = smtp_conn.recv(1024)

    return(smtp_reply)

#
# Arg checking, connection setup
#

try:
    assert len(sys.argv) == 3, \
    "Usage: smtp_enum.py <smtp_server_ip> <username_list_file>"

    users_file = sys.argv[2]
    assert os.path.isfile(users_file), "Cannot open file %s" % users_file

    smtp_server = sys.argv[1]
    smtp_conn = create_connection(smtp_server, SMTP_PORT)

    banner = smtp_conn.recv(1024)
    print banner
except Exception as e:
    print "Error: %s" % e
    exit(1)

#
# Perform SMTP enumeration
#

with open(users_file, 'r') as users:
    for user in users:
        print smtp_verify(user.strip(), smtp_conn)

smtp_conn.close()

I'm wondering if I may later regret the assertion approach if the script grows. Is this an abuse of assertions, or somehow problematic?

What you really want is a robust argument parsing. This calls for argparse:

import argparse
import os.path

def is_file(f):
    if os.path.isfile(f):
        return f
    raise OSError("{} does not exist".format(f))

parser = argparse.ArgumentParser(description="SMTP user checker")
parser.add_argument('server', help="IP address of server")
parser.add_argument('usernames', type=is_file, help="Path to list of user names")
parser.add_argument('--port', '-p', type=int, default=25, help="SMTP port of server")
args = parser.parse_args()

smtp_conn = create_connection(args.server, args.port)
print smtp_conn.recv(1024)

Note that it is already standard practice of Python to print any error and exit afterwards.

Additionally, you probably want to close your SMTP connection even in the case of an exception. For this, you could use a contextmanager:

from contextlib import contextmanager

@contextmanager
def create_connection(host, port):
    try:
        sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        sock.connect((host, port))
        yield sock
    except socket.error:
        # Handle specific exception here, or not and re-raise
        raise
    finally:
        # Ensure sock is closed under all circumstances
        try:
            sock.close()
        except UnboundLocalError:
            # Did not even succeed in creating the variable, nothing left to do
            pass

And then use it like this:

with create_connection(args.server, args.port) as smtp_conn:
    print smtp_conn.recv(1024)
    with open(args.usernames) as users:
        for user in users:
            print smtp_verify(user.strip(), smtp_conn)

Note that Python uses read mode by default when opening a file, so there is no need to use 'r'.

Final code:

import argparse
import socket
import os.path
from contextlib import contextmanager

@contextmanager
def create_connection(host, port):
    try:
        sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        sock.connect((host, port))
        yield sock
    except socket.error:
        # Handle specific exception here, or not and re-raise
        raise
    finally:
        # Ensure sock is closed under all circumstances
        try:
            sock.close()
        except UnboundLocalError:
            # Did not even succeed in creating the variable, nothing left to do
            pass

def smtp_verify(username, smtp_conn):
    smtp_conn.send('VRFY {}\r\n'.format(username))
    return smtp_conn.recv(1024)

def is_file(f):
    if os.path.isfile(f):
        return f
    raise OSError("{} does not exist".format(f))

def parse_args():
    parser = argparse.ArgumentParser(description="SMTP user checker")
    parser.add_argument('server', help="IP address of server")
    parser.add_argument('usernames', type=is_file, help="Path to list of user names")
    parser.add_argument('--port', '-p', type=int, default=25, help="SMTP port of server")
    return parser.parse_args()

if __name__ == "__main__":
    args = parse_args()
    with create_connection(args.server, args.port) as smtp_conn:
        print smtp_conn.recv(1024)
        with open(args.usernames) as users:
            for user in users:
                print smtp_verify(user.strip(), smtp_conn)

Alternatively, you could use the docopt module for this:

"""SMTP user checker.
Usage:
  argparser_comp.py SERVER USERNAMES [(-p <p>| --port=<p>)]
  argparser_comp.py (-h | --help)
Options:
  -h --help          Show this screen.
  -p <p> --port=<p>  SMTP port of server [default: 25].
"""

import socket
import os.path
from contextlib import contextmanager
from docopt import docopt

def is_file(f):
    if os.path.isfile(f):
        return f
    raise OSError("{} does not exist".format(f))

def parse_args():
    args = docopt(__doc__)
    return args["SERVER"], is_file(args["USERNAMES"]), int(args["--port"])

@contextmanager
def create_connection(host, port):
    try:
        sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        sock.connect((host, port))
        yield sock
    except socket.error:
        # Handle specific exception here, or not and re-raise
        raise
    finally:
        # Ensure sock is closed under all circumstances
        try:
            sock.close()
        except UnboundLocalError:
            # Did not even succeed in creating the variable, nothing left to do
            pass

def smtp_verify(username, smtp_conn):
    smtp_conn.send('VRFY {}\r\n'.format(username))
    return smtp_conn.recv(1024)

if __name__ == "__main__":
    server, usernames, port = parse_args()
    with create_connection(server, port) as smtp_conn:
        print smtp_conn.recv(1024)
        with open(usernames) as users:
            for user in users:
                print smtp_verify(user.strip(), smtp_conn)


Setting flags to show three buttons I have this loop that iterates and assigns a variable to true depending on the different conditions

for (const element of actionsReferences) {
  if (element === 'accept') {
    this.showAcceptButton = true
  } else if (element === 'reject') {
    this.showRejectButton = true
  } else if (element === 'transfer') {
    this.showTransferButton = true
  }
}

How can i get the same result by avoiding if () ?

You could take an object and check if the action exists. If so, take the value as key for assignment.

const actions = {
        accept: 'showAcceptButton',
        reject: 'showRejectButton',
        transfer: 'showTransferButton'
    };

for (const element of actionsReferences) {
    if (element in actions) {
        this[actions[element]] = true;
    }
}


Guides the User with Prompts to go through a procedure and Records User Input I am making a prototype worksheet that follows 9 step problem solving procedure.

Some improvements I'm considering would be to iterate over some of it instead of making a new code line for every single variant.  More or less I need to optimize this code so I can learn how to do this type of thing in the future.

It's not a very straight looping process and I can't think of any good ways to set it up.

Here is a picture of the sheet that the values will go into.

Here is my code

Sub nine_Step()

Dim step1, step2, step3, step4a, step4b, step4c, step4d, step4e, step5a, step5b, step5c, step5d, step5e, step6a, step6b, step6c, step6d, step6e, step7, step8, step9 As Variant

step1 = InputBox("Step 1: Define the Problem.")
    Cells(2, 3).Value = step1

step2 = InputBox("Step 2: What is the Measured Actual Value?")
    Cells(4, 3).Value = step2

step3 = InputBox("Step 3: What is the Target Value Desired?")
    Cells(6, 3).Value = step3

    If step3 <> "" Then
        Cells(7, 3).Value = step3 - step2
    Else:
    End If

step4a = InputBox("Step 4: Why is " & step1 & " happening?")
    Cells(9, 3).Value = step4a
step4b = InputBox("Step 4: Why is " & step4a & " happening?")
    Cells(10, 3).Value = step4b
step4c = InputBox("Step 4: Why is " & step4b & " happening?")
    Cells(11, 3).Value = step4c
step4d = InputBox("Step 4: Why is " & step4c & " happening?")
    Cells(12, 3).Value = step4d
step4e = InputBox("Step 4: Why is " & step4d & " happening?")
    Cells(13, 3).Value = step4e

step5a = InputBox("Step 5: Describe your First Possible Solution")
    Cells(15, 3).Value = step5a
step5b = InputBox("Step 5: Describe your Second Possible Solution")
    Cells(16, 3).Value = step5b
step5c = InputBox("Step 5: Describe your Third Possible Solution")
    Cells(17, 3).Value = step5c
step5d = InputBox("Step 5: Describe your Fourth Possible Solution")
    Cells(18, 3).Value = step5d
step5e = InputBox("Step 5: Describe your Fifth Possible Solution")
    Cells(19, 3).Value = step5e

step6a = InputBox("Step 6: Experiment Measurement Results Solution 1")
    Cells(21, 3).Value = step6a
step6b = InputBox("Step 6: Experiment Measurement Results Solution 2")
    Cells(22, 3).Value = step6b
step6c = InputBox("Step 6: Experiment Measurement Results Solution 3")
    Cells(23, 3).Value = step6c
step6d = InputBox("Step 6: Experiment Measurement Results Solution 4")
    Cells(24, 3).Value = step6d
step6e = InputBox("Step 6: Experiment Measurement Results Solution 5")
    Cells(25, 3).Value = step6e

step7 = InputBox("Step 7: When did you impliment a solution? Enter a date in
XX/XX/XXXX format")

If step7 = "" Then
    MsgBox ("Find out when you implimented a solution and enter it when you get a chance.")
Else:
    Cells(28, 3).Value = step7
End If

step8 = InputBox("Step 8: Now that the solution is implemented perminently, what is the current measurement?")
    Cells(31, 3).Value = step8

    If step8 <> "" Then
        Cells(32, 3).Value = Cells(6, 3).Value - step8
    Else:
    End If

step9 = InputBox("Step 9: Any other Ideas that have sprung up since you have started this nine step related to this area?")
    Cells(34, 3).Value = step9

MsgBox ("You have successfully completed this 9 Step.  Congrats!")

End Sub

The way you are declaring your variables isn't what you expect it to be. Luckily, you're declaring variants anyway. But if you weren't declaring Variants they would all by default be Variants.
Dim step1 as variant
Dim step2 as variant, step3 as variant

You need to give all variables a type.
You also have some superfluous variables -

step6a = InputBox("Step 6: Experiment Measurement Results Solution 1")
    Cells(21, 3).Value = step6a
step6b = InputBox("Step 6: Experiment Measurement Results Solution 2")
    Cells(22, 3).Value = step6b

Since you don't re-reference these variables later on, you can just write to the cell
.Cells(21,3) = InputBox("Step 6: Experiment Measurement Results Solution 1")
.Cells(22,3) = InputBox("Step 6: Experiment Measurement Results Solution 2")

Guiding the user
You mention you want to guide the user here. When you prompt them for input, you should tell them what type of input, and then check it's the correct Type.

 Cells(7, 3).Value = step3 - step2

This will error if there are values that aren't numerical. You can define what type you want.
You asked me

What is the Measured Actual Value?

But I didn't know if you wanted what kind of measurement or an integer. This can cause a mismatch error later on.
 step3 = Application.InputBox("Step 3: What is the Target Value Desired?", Type:=1)

Now you require me to enter a number value. You control what happens if I don't put in the expected type.

Refactoring
You see yourself doing the same thing over and over. Normally you would farm this out to a function, sending arguments and getting data back. But, you're using the InputBox which is already a function. So how to make this cleaner? Arrays!
Make yourself an input array and an output array. We know how many questions there are, so no problem.
Dim questions(1 to 21) as String
Dim answers(1 to 21) as Variant

And then populate your questions and pass them to inputbox with a type, and set each consecutive item in the answers array to the response. Then a simple loop
For i = 9 to 25
  .Cells(3,i) = answers(i-8)
Next

There are some other things you could do with your strings and whatnot, but that's up to you.

For the brave
Something like this -
Const STEP1 As String = "Define the Problem."
Const STEP2 As String = "What is the Measured Actual Value?"
Const STEP3 As String = "What is the Target Value Desired?"
Const STEP4a As String = "Why is " & questions(1) & " happening?"
Const STEP4b As String = "Why is " & questions(4) & " happening?"
Const STEP4c As String = "Why is " & questions(5) & " happening?"
Const STEP4d As String = "Why is " & questions(6) & " happening?"
Const STEP4e As String = "Why is " & questions(7) & " happening?"
Const STEP5a As String = "Step 5: Describe your First Possible Solution"
Const STEP5b As String = "Step 5: Describe your Second Possible Solution"
Const STEP5c As String = "Step 5: Describe your Third Possible Solution"
Const STEP5d As String = "Step 5: Describe your Fourth Possible Solution"
Const STEP5e As String = "Step 5: Describe your Fifth Possible Solution"
Const STEP6a As String = "Step 6: Experiment Measurement Results Solution 1"
Const STEP6b As String = "Step 6: Experiment Measurement Results Solution 2"
Const STEP6c As String = "Step 6: Experiment Measurement Results Solution 3"
Const STEP6d As String = "Step 6: Experiment Measurement Results Solution 4"
Const STEP6e As String = "Step 6: Experiment Measurement Results Solution 5"
Const STEP7 As String = "Step 7: When did you impliment a solution?"
Const STEP8 As String = "Step 8: Now that the solution is implemented perminently, what is the current measurement?"
Const STEP9 As String = "Step 9: Any other Ideas that have sprung up since you have started this nine step related to this area?"

Dim questions(1 To 21) As String
questions(1) = STEP1
questions(2) = STEP2
questions(3) = STEP3
questions(4) = STEP4a
questions(5) = STEP4b
questions(6) = STEP4c
questions(7) = STEP4d
questions(8) = STEP4e
questions(9) = STEP5a
questions(10) = STEP5b
questions(11) = STEP5c
questions(12) = STEP5d
questions(13) = STEP5e
questions(14) = STEP6a
questions(15) = STEP6b
questions(16) = STEP6c
questions(17) = STEP6d
questions(18) = STEP6e
questions(19) = STEP7
questions(20) = STEP8
questions(21) = STEP9

Then you can loop through like
For i = 1 To 21
    answers(i) = InputBox(questions(i))
End Sub

But you might need a custom argument in case you need to restrict what Type the user can enter. This you cannot do with a UserForm unless you get into keypresses and whatnot.


Parsing structured text in Java I am quite new to Java. I have data structured like this:

tree 503(there is a null character here)040000 tree 0bfec80bc7b7aa0a50e0b373044929cb0aae278b build/
100755 blob e30876c61eb7712ad09b50593a30513d9b173805 build.xml
100755 blob 1574df4a2de5d2884f33a5d3e30ae3daa5affee1 manifest.mf
040000 tree fdab6a12a56a709212f105c9c8e31cd062d01e0a nbproject/
040000 tree 3c977009b6104310d7f3a66200f29bff8cda7576 src/
100755 blob de4bcf5c862c352531819e082204bb51ddabaa9c test_file
100755 blob 26af8cac3b104b4b6514621b1c512e353307a867 unicode.txt

I call each line of this an entry. To parse this data into an array of entries, I wrote this;

class TreeEntry {
    public String perms;
    public String type;
    public String hash;
    public String name;

    @Override
    public String toString() {
        return perms + " " + type + " " + hash + " " + name;
    }
}

private static TreeEntry[] parseTree(String hash) {
    String tree = readObject(hash);

    if (!tree.startsWith("tree")) {
        System.err.println("Given hash doesn't belong to a tree object");
        return null;
    }
    int currentArrayCapacity = 4;
    int currentEntries = 0;
    TreeEntry[] entries = new TreeEntry[currentArrayCapacity];

    // We are interested in data after null character
    int nullCharacterPosition = 0;
    while (tree.charAt(nullCharacterPosition) != '\0') {
        nullCharacterPosition++;
    }
    tree = tree.substring(nullCharacterPosition + 1);

    StringTokenizer st = new StringTokenizer(tree, "\n");
    while (st.hasMoreElements()) {
        TreeEntry e = new TreeEntry();
        StringTokenizer st2 = new StringTokenizer((String)st.nextElement(), " ");
        e.perms = (String) st2.nextElement();
        e.type = (String) st2.nextElement();
        e.hash = (String) st2.nextElement();
        e.name = (String) st2.nextElement();

        // There can be space in name
        while (st2.hasMoreElements()) {
            e.name += " " + (String) st2.nextElement();
        }
        currentEntries++;

        // To increase capacity of the array
        if (currentEntries > currentArrayCapacity) {
            currentArrayCapacity = currentArrayCapacity * 2;
            TreeEntry[] tmp = new TreeEntry[currentArrayCapacity];
            System.arraycopy(entries, 0, tmp, 0, entries.length);
            entries = tmp;
        }

        entries[currentEntries - 1] = e;
    }
    return entries;
}

How does it look as a Java code, and how does it look data structures and algorithms wise?

I'll first suggest you that the raw format that you use to store your data, use a different separator than a white-space. This will simplify the code since we are sure that we have unique separator.

Your TreeEntry should encapsulate his fields in getter and setter. Public field in a class is not something that you want to do in a OOP program. You should always limit the scope of your variable. Your TreeEntry class should look like :

class TreeEntry {

    private String perms;
    private String type;
    private String hash;
    private String name;

    @Override
    public String toString() {
        return perms + " " + type + " " + hash + " " + name;
    }

    public String getPerms() {
        return perms;
    }

    public void setPerms(String perms) {
        this.perms = perms;
    }

    public String getType() {
        return type;
    }

    public void setType(String type) {
        this.type = type;
    }

    public String getHash() {
        return hash;
    }

    public void setHash(String hash) {
        this.hash = hash;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
}

For the parsing part, I will use a different format than yours. I use the ; character to separate each "column" of the data. The format would look like :

tree;503(null)040000;tree;0bfec80bc7b7aa0a50e0b373044929cb0aae278b;build/
100755;blob;e30876c61eb7712ad09b50593a30513d9b173805;build.xml
100755;blob;1574df4a2de5d2884f33a5d3e30ae3daa5affee1;manifest.mf
040000;tree;fdab6a12a56a709212f105c9c8e31cd062d01e0a;nbproject/

I've changed a lot of things in the parseTree() method so I'll drop the new method and I'll explain later what are the changes and why.

private static List<TreeEntry> parseTree(String hash) {
    String tree = readObject(hash);

    if (!tree.startsWith("tree")) {
        System.err.println("Given hash doesn't belong to a tree object");
        return null;
    }

    List<TreeEntry> entries = new LinkedList<>();

    // // We are interested in data after null character
    int nullCharacterPosition = tree.indexOf("\0");
    tree = tree.substring(nullCharacterPosition + 1);

    String[] dataTree = tree.split("\n");

    for (String entry : dataTree) {
        TreeEntry e = new TreeEntry();
        String[] args = entry.split(";");
        e.setPerms(args[0]);
        e.setType(args[1]);
        e.setHash(args[2]);
        e.setName(args[3]);
        entries.add(e);
    }

    return entries;
}

If you want to find the null character, you should use indexOf, this will do the loop for you and will return the index of the first character corresponding. Since I don't have an input with a null value, I've not tested my code to find the null character. So be sure to verify this part.

I've also changed the type of entries to a List. I've choose a LinkedList since all the operation I'm doing is adding new entries, and you'll probably iterate over it later. This allow to focus on the task of parsing and I don't have to manage the array (I don't have to check the size before adding something).

I've used the spilt() method instead of the StringTokenizer. I've find it easy to operate in conjunction with a for-each loop, and you don't seems to want to work with anything than String, so split() is perfect for that. I'll point you to this question for further information on why I would not use StringTokenizer, read the second answer.

It seems rather odds to me that the parsing method is static. I don't know where the method is called, but I would have go with something like a factory or utility class to do the parsing job.

PS: This is my first review, so if this is not a good format or there is some inaccuracy let me know.


Permutations of a list in Haskell I have this piece of code which given a list returns another list which combines every element with every other element (without repetitions) using a given function as a combiner. I came up with three different version (2 of them listed below, fastest to slowest), and now I'm asking for help to optimize it/them even further. Currently, my perms3 does not utilize multithreading at all, and since I'm quite new to Haskell I can't seem to figure out why, but I'm thinking that in its current state my algorithm isn't parallelizable, even though it can be.

I've also concluded that the length of the output list will always be \$ \binom{x}{2} + x = x(x-1)/2 + x\$ where \$x\$ is the length of the input list, but I dont know how to use that in my algorithm either to minimize the allocations.

The reason the why I'm asking is because I want to know more about the inner workings of Haskell, how lazy-evaluation affects performance and what is considered good/bad Haskell code.

My two permutation functions:

perms3  c l             = perms3' c l l
perms3' _ []      _     = []
perms3' c (a:b)   [x]   = c a x:perms3' c b b -- the outer loop
perms3' c l@(a:_) (h:t) = c a h:perms3' c l t -- the inner loop

perms2 _ [] = [] --this can probably be 2 folds, the inner fold might be parallelizable?
perms2 c l@(h:t)= foldr (\x acc -> c h x:acc) [] l ++ perms2 c t

Input:  (,) ['a'..'c']
Output: [('a','a'),('a','b'),('a','c'),('b','b'),('b','c'),('c','c')]

Input   (+) [1..5]
Output: [2,3,4,5,6,4,5,6,7,6,7,8,8,9,10]

Unsurprisingly, perms3 is a fair bit faster than perms2.

Testing perms3:

import Control.Monad
import System.Environment

main :: IO ()
main = liftM (length . perms3 (,) . enumFromTo 1 . read . head) getArgs >>= print

perms3 :: (a -> a -> t) -> [a] -> [t]
perms3  c l             = perms3' c l l
perms3' _ []      _     = []
perms3' c (a:b)   [x]   = c a x:perms3' c b b
perms3' c l@(a:_) (h:t) = c a h:perms3' c l t

Running it:

$ ghc -O3 main && time ./main 10000
50005000

real    0m0.615s

$ ghc -O3 main && time ./main 30000
450015000

real    0m5.347s

Question(s):

Is this a good way to write Haskell code?
Can I squeeze any more performance out of my function?
Should I prefer the imperative do-notation instead of arrows in my main function?

3. Monadic notation

Let's start with your last question, as it has a clichéd answer: Do whatever you prefer.  I often find the bind operator >>= nicer to read, but switch to do-notation as soon as the code gets complicated.  Another rule of thumb is to use do-notation if the code is easier to express imperatively.  Both notations are equivalent, there is no performance impact.

1. Idiomatic Haskell

Haskell has a few conventions about variable naming.  Lists are commonly referred to as xs or ys (think plural of x), while function parameters are named f or g.  Type parameters are almost always taken from the first letters of the alphabet.

perms3 :: (a -> a -> b) -> [a] -> [b]
perms3  f xs               = perms3' f xs xs
perms3' _ []         _     = []
perms3' c (x:xs)    [y]    = f y z : perms3' f xs  xs
perms3' c xs'@(x:_) (y:ys) = f x y : perms3' f xs' ys

Since perms3' is only used within perms3, it would make sense to define it as a local function.  This has the additional benefit that we can drop f from its parameter list, since it's already in scope and won't change.

perms3 :: (a -> a -> b) -> [a] -> [b]
perms3  f xs = perms3' xs xs
  where
    perms3' []         _     = []
    perms3' (x:xs)    [y]    = f y z:perms3' f xs  xs
    perms3' xs'@(x:_) (y:ys) = f x y:perms3' f xs' ys

2. Performance improvement

Pattern matching on lists usually takes two cases: empty and non-empty lists.  This can reduce code (DRY etc) and actually improve performance, as tests for empty lists are faster than tests for singleton lists.

perms4 :: (a -> a -> b) -> [a] -> [b]
perms4  f xs = perms4' xs xs
  where
    perms4' []        _      = []
    perms4' (_:ys)    []     = perms4' ys ys
    perms4' ys'@(y:_) (z:zs) = f y z : perms4' ys' zs

A quick test gave me a 20% improvement compared to perms3.

Notes on perms2

The fold in perms2 is just a map.  Using partial application, it could be written as

perms5 _ [] = []
perms5 f xs'@(x:xs) = map (f x) xs' ++ perms5 f xs
-- faster:
-- perms5 f xs'@(x:xs) = foldr ((:) . f x) [] xs' ++ perms5 f xs

However, this is (somewhat surprisingly) slower than the fold.  AFAICT, this is due to the fact that ghc won't inline map, but will do so with foldr.

Finally, here is how I (naïvely) would have written the code.  Looks nice,  performs terribly.

import Data.List (concatMap, tails)
perms6 f = concatMap pairs . tails
  where
    pairs [] = []
    pairs (ys'@(y:_)) = map (f y) ys'

or

perms7 f xs = concat $ zipWith (map . f) xs (tails xs)


Is there a better way to code this text adventure game? This is my first attempt with a basic text adventure game, and I was wondering if there was a way to make it more efficient (or any other things I could add to make the game better).  It isn't totally finished yet, but if there is a way to make it better, please post in the comments.

#lets me use the exit function
import sys
#Lets me use the time function
import time
#Want to play loop

    #asks for name
name = raw_input('What is your name, adventurer? ')
print 'Nice to meet you, '+name+'. Are you ready for your adventure?'
while True:
    ready = raw_input('y/n ')
if ready == 'y':
    print 'Good, let us start our great adventure!'
    break
elif ready == 'n':
    print 'That is a real shame...'
    time.sleep(1)
    print 'Exiting program in 5 seconds:'
    time.sleep(1)
    print '5'
    time.sleep(1)
    print '4'
    time.sleep(1)
    print '3'
    time.sleep(1)
    print '2'
    time.sleep(1)
    print '1'
    time.sleep(1)
    sys.exit('Exiting Game...')
    break
else:
    print 'That is not a valid answer... Try again'
    time.sleep(2)

#First level loop
while True:
print 'You awaken on the ground of a great forest. You can see two possible things to         do, enter the cabin to your left, or wander through the woods.'
print ' '

print 'Where do you want to go?'
FirstDecision = raw_input('cabin/woods ')
    #Cabin option
if FirstDecision == 'cabin':
    print 'You approach the door to the cabin, luckily it is unlocked.'
    Cnt = raw_input('Enter < to continue')
    print 'You open the door and walk through, only to hear a dry and shaky voice say:'
    print '\"Get out.\"'
    Cnt = raw_input('Enter < to continue')
    print 'What do you do?'
    FirstCabin = raw_input('leave/fight ')
    if FirstCabin == 'leave':
        print 'As you run out the door, the voice shoots and kills you.'
        Cnt = raw_input('Enter < to continue')
        print ' '
        FirstRetry = raw_input('Try again? (y/n)')
        if FirstRetry == 'y':
            print 'Restarting back at checkpoint...'
            time.sleep(2)
        elif FirstRetry == 'n':
            print 'That\'s a shame...'
            time.sleep(1)
            print 'Exiting program in 5 seconds:'
            time.sleep(1)
            print '5'
            time.sleep(1)
            print '4'
            time.sleep(1)
            print '3'
            time.sleep(1)
            print '2'
            time.sleep(1)
            print '1'
            time.sleep(1)
            sys.exit('Exiting Game...')
            break
    elif FirstCabin == 'fight':
        print 'You turn to where the voice came from and you bluntly say \"No.\"``

A couple things. First of all, you should abstract the function that exits the game:

def exitGame():
    print 'That\'s a shame...'
    print 'Exiting program in 5 seconds:'
    for i in range(5):
        time.sleep(1)
        print(i+1)
    sys.exit('Exiting Game...')

Second, I would probably push Cabins, retries, etc. into arrays.

DECISIONS = []
CABINS = []
CONTINUES = []

Third, I would define the first level as a function so you can do the gracefully exit:

EXIT_MESSAGE = 'Exiting Game...'
#First level loop
while True:
    try: firstLevel()
    except SystemExit as e:
        print(str(e))
        break

One last thing. Use print as a function and not a statement. And maybe it's personal preference, but I find string formatting strongly preferable:

print('Nice to meet you, %s. Are you ready for your adventure?' % name)


Compare lines in 2 text files with different numbers of fields This is the (hopefully) final version of my script for my file comparison problem mentioned previously in two posts on Stack Overflow  (here and here).

I have come up with the code shown below, which does what I need it to do, but I'm wondering if it can be written in a more pythonic (read elegant) way, especially the clean up of the lists.

#!/usr/bin/python
import sys
import csv

f1 = sys.argv[1]
f2 = sys.argv[2]

with open(f1) as i, open(f2) as j:
    a = csv.reader(i)
    b = csv.reader(j)
    for linea in a:
        lineb = next(b)
        lista = ([x for x in linea if len(x) > 0])
        listastr = map(str.strip, lista)
        listastrne = filter(None, listastr)
        listb = ([x for x in lineb if len(x) > 0])
        listbstr = map(str.strip, listb)
        listbstrne = filter(None, listbstr)
        if len(listastrne) != len(listbstrne):
            print('Line {}: different fields: A: {} B: {}'.format(a.line_num, listastrne, listbstrne))
        elif sorted(map(str.lower, listastrne)) != sorted(map(str.lower, listbstrne)):
            print('Line {}: {} does not match {}'.format(a.line_num, listastrne, listbstrne))

Example input files:

A.csv:

1,2,,
1,2,2,3,4
1,2,3,4
X
AAA,BBB,CCC
DDD,,EEE,
GGG,HHH,III
XXX,YYY   ,ZZZ

 k,

B.csv:

1,2,2,2
1,2,3,4
1,2,3,4
W
AAA,,BBB,CCC
EEE,,DDD,,
,,GGG,III,HHH
XXX,YYY,ZZZ

,

Less repetition

You have a lot of repeated code (and unnecessary code).  For example, you manually increment your second reader when you could zip them together.  You perform the same list comprehension on each of them.  You map multiple things onto them after a list comprehension.  You perform a pointless filter that is essentially a copy.  Removing all of this gives us this:

from itertools import izip
import csv
import sys

file1 = sys.argv[1]
file2 = sys.argv[2]

def get_clean_line(line):
    return [entry.strip().lower() for entry in line if entry]

with open(file1) as first_file, open(file2) as second_file:
    first_reader = csv.reader(first_file)
    second_reader = csv.reader(second_file)

    for first_line, second_line in izip(first_reader, second_reader):
        first_list, second_list = get_clean_line(first_line), get_clean_line(second_line)

        if (len(first_list) != len(second_list) or
                sorted(first_list) != sorted(second_list)):
            print('Line {}: different fields: A: {} B: {}'.format(
                first_reader.line_num, first_list, second_list))

You could easily move the sorted call into get_clean_line if you want to, but if you think the lengths will be different more often it might make sense to leave it there so short circuiting avoids too many expensive sorts.

Names

You could use better names - bytes are cheap

file1 and file2 vs f1 and f2, first/second_reader vs a and b, etc.  There are probably even better names, but that would probably depend on your specific domain.


encrypt/decrypt person data Ahead of the General Data Protection Regulation (GDPR) I begin to crypt more than ever before, in fact, all I can crypt become crypt.

However, I wonder if anybody can tell me if this code is secure enough or can make it even better, optimize or just have some tips about the code. and not sure about the OpenSSL are secure anymore so if any could give me some feedback about it I will love you forever.

The $keyis defined in the config.php and are a base64 encrypt and look like this e.g bRuD5WYw5wd0rdHR9yLlM6wt2vteuiniQBqE70nAuhU

class myCrypt {

    public function my_encrypt($data, $key) {
        // Remove the base64 encoding from our key
        $encryption_key = base64_decode($key);
        // Generate an initialization vector
        $iv = openssl_random_pseudo_bytes(openssl_cipher_iv_length('aes-256-cbc'));
        // Encrypt the data using AES 256 encryption in CBC mode using our encryption key and initialization vector.
        $encrypted = openssl_encrypt($data, 'aes-256-cbc', $encryption_key, 0, $iv);
        // The $iv is just as important as the key for decrypting, so save it with our encrypted data using a unique separator (::)
        return base64_encode($encrypted . '::' . $iv);
    }

    public function my_decrypt($data, $key) {
        // Remove the base64 encoding from our key
        $encryption_key = base64_decode($key);
        // To decrypt, split the encrypted data from our IV - our unique separator used was "::"
        list($encrypted_data, $iv) = explode('::', base64_decode($data), 2);
        return openssl_decrypt($encrypted_data, 'aes-256-cbc', $encryption_key, 0, $iv);
    }
}

I like the fact that the same encrypt is never the same, but if a hacker hacks the code will he/she just use the decrypt method to decrypt the data??

  PS: This is not for passwords

I guess my first suggestion is that, unless you really understand cryptography, you really should not be writing your own encryption libraries. Use an existing one like libsodium (or something based on it) , or, if you can't install the libsodium extension in your environment, perhaps the defuse/php-encryption library which is well-regarded.

Now on to your specific code...

I would start by not having your $key in your codebase at all.  Any sensitive information - passwords, encryption keys, etc. should never be placed in your code base where these items can be visible in your revision control systems.  Ideally, you can derive this from environmental configuration and inject the secure material during environmental deployment.

Stylistically, you should consider starting all PHP class names with uppercase letter (which is pretty much de facto standard) and stay away from mixing camelCase and snake_case in your code libraries as you are doing here with camelCase class name and snake_case method and variable names. I know PHP is not good about this within it's built-in classes and functions, but that doesn't mean you shouldn't be in your own code.

From a use standpoint, is there really reason for a class like this to be instantiated, or should these be static methods?


F# parse data and calculate sum I decided to play with F# and found simple task on Codewars.
The task is sounds like "Calculate Sum/Mean/Avarage for selected City".
I decided to calculate only Sum. My solution is horrific. I would be grateful for your advice's how to do this in a right way. Thank you!

let d0 =
        "Rome:Jan 81.2,Feb 63.2,Mar 70.3,Apr 55.7,May 53.0,Jun 36.4,Jul 17.5,Aug 27.5,Sep 60.9,Oct 117.7,Nov 111.0,Dec 97.9\n\
        London:Jan 48.0,Feb 38.9,Mar 39.9,Apr 42.2,May 47.3,Jun 52.1,Jul 59.5,Aug 57.2,Sep 55.4,Oct 62.0,Nov 59.0,Dec 52.9\n\
        Paris:Jan 182.3,Feb 120.6,Mar 158.1,Apr 204.9,May 323.1,Jun 300.5,Jul 236.8,Aug 192.9,Sep 66.3,Oct 63.3,Nov 83.2,Dec 154.7\n\
        NY:Jan 108.7,Feb 101.8,Mar 131.9,Apr 93.5,May 98.8,Jun 93.6,Jul 102.2,Aug 131.8,Sep 92.0,Oct 82.3,Nov 107.8,Dec 94.2\n\
        Vancouver:Jan 145.7,Feb 121.4,Mar 102.3,Apr 69.2,May 55.8,Jun 47.1,Jul 31.3,Aug 37.0,Sep 59.6,Oct 116.3,Nov 154.6,Dec 171.5\n\
        Sydney:Jan 103.4,Feb 111.0,Mar 131.3,Apr 129.7,May 123.0,Jun 129.2,Jul 102.8,Aug 80.3,Sep 69.3,Oct 82.6,Nov 81.4,Dec 78.2\n\
        Bangkok:Jan 10.6,Feb 28.2,Mar 30.7,Apr 71.8,May 189.4,Jun 151.7,Jul 158.2,Aug 187.0,Sep 319.9,Oct 230.8,Nov 57.3,Dec 9.4\n\
        Tokyo:Jan 49.9,Feb 71.5,Mar 106.4,Apr 129.2,May 144.0,Jun 176.0,Jul 135.6,Aug 148.5,Sep 216.4,Oct 194.1,Nov 95.6,Dec 54.4\n\
        Beijing:Jan 3.9,Feb 4.7,Mar 8.2,Apr 18.4,May 33.0,Jun 78.1,Jul 224.3,Aug 170.0,Sep 58.4,Oct 18.0,Nov 9.3,Dec 2.7\n\
        Lima:Jan 1.2,Feb 0.9,Mar 0.7,Apr 0.4,May 0.6,Jun 1.8,Jul 4.4,Aug 3.1,Sep 3.3,Oct 1.7,Nov 0.5,Dec 0.7"

module CalculatorHelper =
    let SumByTown (town:string) (input:string) =
        let rows = input.Split('\n')
        for i in rows do
            if (i.Contains(town))
            then
                let monthes = i.Replace(town + ":","").Split(",")
                let sequence = seq { for j in monthes do yield j.Remove(0,4) |> double } |> Seq.sum
                printf "%f" sequence

let result = CalculatorHelper.SumByTown "Rome" d0

You should reduce the use of explicit type declaration as much as possible.

A function isn't of much value if it just prints its calculated result. You should return the result to let it be up to the client to handle it.

But you can't break out from a loop via an if-statement in F#. You can overcome that by wrapping the outer for-loop in a seq {...} statement and then yield the one and only result and then take the first element from that sequence. But that is inefficient in that it iterates through all the cities even if the first is the one searched for.

Below I've refactored your algorithm according to the above in order to be able to return the result from the function:

let SumByTown town input =
    let rows = (string input).Split('\n')
    seq { for row in rows do
            if (row.Contains(town)) then
                let months = row.Replace(town + ":" , "").Split(',')
                let sequence = seq { for j in months do yield j.Remove(0,4) |> double } |> Seq.sum
                yield sequence
    } |> Seq.head

But this still doesn't look very functional IMO.

For inspiration is here a version that uses the built in higher order functions in a more functional manner:

let calculateCity city data =
    let sum values = values |> Array.sum
    let avg values = values |> Array.average
    let mean values = (values |> Array.sort).[values.Length / 2]

    let handleCity cityText =
        let values = Regex.Replace(cityText, "[^0-9,.]+", "").Split(',') |> Array.map float
        (sum values, avg values, mean values)

    (string data).Split('\n') |> Seq.find (fun s -> s.StartsWith(city)) |> handleCity

let printCityData (sum, avg, mean) = printfn "Sum: %.2f, Average: %.2f, Mean: %.2f" sum avg mean

Usage:

printCityData (calculateCity town data)

Update

Antoher way to use Regex is to match numbers instead of removing non-numeric chars:

let calculateCity1 city data =
    let sum values = values |> Seq.sum
    let avg values = values |> Seq.average
    let mean values = (values |> Seq.sort).ElementAt((values|> Seq.length)/ 2)
    let extractValues text = Regex.Matches(text, "\d+.\d+").Cast<Match>() |> Seq.map (fun m -> m.Value)

    let handleCity cityText =
        let values = cityText |> extractValues |> Seq.map float
        (sum values, avg values, mean values)

    (string data).Split('\n') |> Seq.find (fun s -> s.StartsWith(city)) |> handleCity

let printCityData (sum, avg, mean) = printfn "Sum: %.2f, Average: %.2f, Mean: %.2f" sum avg mean

This requires an open System.Linq at the top of the module


Cleaning up the code of a C++-based Personal Web Page Generator I have tried to implement the following challenge:

Personal Web Page Generator
Write a program that asks the user for his or her name, then asks the user to enter a sentence that describes himself or herself. Here is an example of the program's screen:
Enter your name: Julie TaylorEnter
Describe yourself: I am a computer science major, a member of the jazz club, and I hope to work as a mobile app developer after I graduate.Enter
Once the user has entered the requested input, the program should create an HTML file, containing the input, for a simple webpage. Here is an example of the HTML content, using the sample input previously shown:

Julie Taylor

  I am a computer science major, a member of the Jazz club,
  and I hope to work as a mobile app developer after I graduate.

I implemented this in C++. Here is my code:
ofstream outfile_webpage;
string name, description;

outfile_webpage.open("Webpage.html");

cout << "Enter your name: ";
getline(cin,name);

cout << "Describe yourself: ";
getline(cin,description);

outfile_webpage << "" << endl;
outfile_webpage << "" << endl;
outfile_webpage << "" << endl;
outfile_webpage << "" << endl;
outfile_webpage << "\t" << endl;
outfile_webpage << "\t\t
" << name << "
" << endl;
outfile_webpage << "\t" << endl;
outfile_webpage << "\t
" << endl;
outfile_webpage << "\t" << description << endl;
outfile_webpage << "\t
" << endl;
outfile_webpage << "" << endl;
outfile_webpage << "" << endl;

outfile_webpage.close();

return 0;

This works just fine, but I see that there are a lot of repetitive lines. How can I reduce these repetitive lines?
Also, is there a way to make this code cleaner in another way, like by using a custom HTML template?

There is a lot that can be improved here.  Here are some ideas.
Don't abuse using namespace std
Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.
Use all required #includes
This code fragment uses a number of features that require included library headers.  In this case they appear to be:
#include <iostream>
#include <fstream>
#include <string>

It was not difficult to infer, but it helps reviewers if the code is complete.
Provide complete code to reviewers
This is not so much a change to the code as a change in how you present it to other people.  Without the full context of the code and an example of how to use it, it takes more effort for other people to understand your code.  This affects not only code reviews, but also maintenance of the code in the future, by you or by others.  One good way to address that is by the use of comments.  Another good technique is to include test code showing how your code is intended to be used.
Decompose your program into functions
All of the logic here is apparently in main in one long and repetitive chunk of code.  It would be better to decompose this into separate functions.
Don't use std::endl if you don't really need it
The difference betweeen std::endl and '\n' is that '\n' just emits a newline character, while std::endl actually flushes the stream.  This can be time-consuming in a program with a lot of I/O and is rarely actually needed.  It's best to only use std::endl when you have some good reason to flush the stream and it's not very often needed for simple programs such as this one.  Avoiding the habit of using std::endl when '\n' will do will pay dividends in the future as you write more complex programs with more I/O and where performance needs to be maximized.
Simplify and consolidate steps
Rather than having two separate lines to declare and then open the webpage file, do it in a single step:
std::ofstream webpage{"Webpage.html"};

Use string concatenation
The program has many repeated lines where the ostream operator<< is used multiple times with a fixed string.  Those multiple calls don't need to happen.  You could simply rely on the fact that C++ merges separate constant strings automatically.  For example, here is a recoded portion:
std::ofstream webpage{"Webpage.html"};
webpage << "\n"
    "\n"
    "\n"
    "\n"
    "\t\n"
    "\t\t
" << name << "
\n"
    "\t\n"
    "\t
"
    "\t" << description << "\n"
    "\t
"
    ""
    "";

First, HTML parsers don't care about newlines, so it really doesn't matter much if they are present or not.  Second, this is now a single explicit invocation of ostream operator<<.
Understand relevant standards
The HTML  tag is obsolete and should not be used.
Adding features
Using an HTML template instead is a good idea and would keep the code generic while enabling customization of use.  One way to do that would be to read in a file and then substitute special tags such as @name@ and @description@ but simply copying the rest verbatim.


Recover a secret string from random triplets Got this problem from Codewars.

There is a secret string which is unknown to you. Given a collection of random triplets from the string, recover the original string.
A triplet here is defined as a sequence of three letters such that each letter occurs somewhere before the next in the given string. "whi" is a triplet for the string "whatisup".
As a simplification, you may assume that no letter occurs more than once in the secret string.
You can assume nothing about the triplets given to you other than that they are valid triplets and that they contain sufficient information to deduce the original string. In particular, this means that the secret string will never contain letters that do not occur in one of the triplets given to you.

For example, given triplets [['t','u','p'], ['w','h','i'], ['t','s','u'], ['a','t','s'], ['h','a','p'], ['t','i','s'], ['w','h','s']], the code should return the string whatisup. All the context that I had to solve the problem came from the description. This is a programming challenge, not meant to solve a real life problem. My code passed the tests. There where no random tests though. My main interest is about performance.
function recoverSecret(triplets) {
    const after = {};

    const precedence = (a, b) => after[a].has(b)
        || [...after[a]].some(c => precedence(c, b) == -1) ? -1 : 1;

    for (let triplet of triplets) {
        after[triplet[0]] = new Set([...(after[triplet[0]] || []), ...triplet.slice(1)]);
        after[triplet[1]] = new Set([...(after[triplet[1]] || []), triplet[2]]);
        after[triplet[2]] = new Set([...(after[triplet[2]] || [])]);
    }

    return Object.keys(after).sort(precedence).join``;
}

Tried destructuring triplet into [a, b, c] to make it more readable, but got execution timeout. Codewars platform has a max time limit of 12000ms. Wanted to do this:
for (let [a, b, c] of triplets) {
    after[a] = new Set([...(after[a] || []), b, c]);
    after[b] = new Set([...(after[b] || []), c]);
    after[c] = new Set([...(after[c] || [])]);
}

I have mainly two things I'd like to look at:

I'm not a big of fan of the constant deconstructing and recreating of Sets in the loop, especially since this most likely has negative impact on the performance. Unfortunately because there is not direct way to add multiple values to a Set there is no functional "one line" solution. My function to do this would look like this:

const addToAfter = (key, values) => {
    const set = after[key] || (after[key] = new Set());
    values.forEach(a => set.add(a));
};

which changes the loop to

for (let triplet of triplets) {
    addToAfter(triplet[0], triplet.slice(1));
    addToAfter(triplet[1], [triplet[2]]);
    addToAfter(triplet[2], []);
}

This leads to my second point: the obvious code repetition. I don't think there is a reason not to use another loop here, which also has the "advantage", that you don't need to limit yourself to "triplets", but support arrays of any length:

for (let triplet of triplets) {
    for (let i = 0, len = triplet.length; i < len; i++) {
        addToAfter(triplet[i], triplet.slice(i + 1));
    }
}

If performance is important, I'd even consider getting rid of the .slice call and replace it with a loop, that adds the characters directly to the Set.

In general: I'm avoiding the "modern" functional way, because it (at least in this case) leads to slightly cryptic syntax (and thus unread code) and a lot of (slow) memory reallocation and data copying.


Little script to Caesar shift strings I wrote a little Perl script to Caesar shift strings. Usage:

$ # usage: caesar <string> [shifts,...]

$ caesar abcde # The default shift is 13.
nopqr

$ caesar abcde 1
bcdef

$ caesar abcde 1,13 # Multiple shifts are supported.
bcdef
nopqr

$ caesar abcde 1-3,5-6 # Shift ranges are supported.
bcdef
cdefg
defgh
fghij
ghijk

Here is the code:

#!/usr/bin/env perl

use strict;

sub usage {
    die 'usage: ceaser <string> [shifts,...]'
}

usage if $#ARGV < 0;

my @str = split //, $ARGV[0];
my @shifts = split /,/, $ARGV[1];
@shifts = (13) unless @shifts;

# Resolve ranges.
my @shifts2 = ();
SHIFT: for my $shift (@shifts) {
    my @ranges = split /-/, $shift;
    for my $range (@ranges) {
        if ($range < 0 || $range > 25) {
            warn "invalid shift size: $range";
            next SHIFT;
        }
    }
    push @shifts2, ($ranges[0]..$ranges[$#ranges]);
}
@shifts = @shifts2;

# Generate ciphered strings.
for my $shift (@shifts) {
    my @str2 = @str;
    for my $c (@str2) {
        my $d = ord($c);
        if ($d >= ord('a') && $d <= ord('z') - $shift ||
            $d >= ord('A') && $d <= ord('Z') - $shift) {
            $d += $shift;
        } elsif ($d >= ord('z') - $shift && $d <= ord('z') ||
                 $d >= ord('Z') - $shift && $d <= ord('Z')) {
            $d -= ord('z') - ord('a') - $shift + 1;
        }
        $c = chr($d);
    }
    printf "%s\n", join '', @str2;
}

This is my first Perl script. I am looking for advice on making the code simpler, cleaner, and more idiomatic.

This is good Perl code.  You've followed good practices such as use strict and declaring variables using my.  It's also quite readable.

However, it feels a bit like C, trudging along with a lot of iteration and low-level operations.  Also, I would suggest defining subroutines such as caesar and shifts.

A typical way to implement a Caesar cipher using Perl is with the tr operator.  Unfortunately, since the amount of shifting is to be determined at runtime, you would have to use a nasty eval.

my $uc_in = join '', ('A'..'Z');
my $lc_in = join '', ('a'..'z');
sub caesar {
    my ($shift, $text) = @_;
    my $uc_out = substr((join '', ('A'..'Z', 'A'..'Z')), $shift, 26);
    my $lc_out = substr((join '', ('a'..'z', 'a'..'z')), $shift, 26);
    eval "\$text =~ tr/$uc_in$lc_in/$uc_out$lc_out/r";
}

An alternative implementation, closer to your original idea but avoiding splitting and iterating, is to perform a substitution using s///eg.

sub caesar {
    my ($shift, $text) = @_;
    $text =~ s{([A-Z])|([a-z])}
              { chr($1 && (ord($1) <= ord('Z') - $shift) ? ord($1) + $shift :
                    $2 && (ord($2) <= ord('z') - $shift) ? ord($2) + $shift :
                    $1 ? ord($1) + $shift - (ord('Z') - ord('A') + 1) :
                         ord($2) + $shift - (ord('z') - ord('a') + 1)
                   )
              }eg;
    return $text;
}

To parse the shifts, I would prefer a more functional approach, building the list using map instead of appending results within a for loop.

sub shifts {
    my ($arg) = @_;
    return map {
        my ($low, $high) = split /-/, $_, 2;
        if ($low < 0 || $high > 25 || defined $high && $low >= $high) {
            warn "Invalid shift size: $_";
        }
        defined $high ? ($low .. $high) : $low;
    } (split /,/, $arg);
}

With the difficult stuff out of the way, the main program would be something like

# print "…\n" is annoying.  Use say() if you have a newer Perl.
use v5.10;
use feature qw(say);

die 'usage: caesar <string> [shifts,...]' unless @ARGV;

my @shifts = defined $ARGV[1] ? shifts($ARGV[1]) : (13);
for my $shift (@shifts) {
    say caesar($shift, $ARGV[0]);
}


Node function to walk through file system and make sure it has a matching db record I have an app that uses the file system a lot to store generated files (zip files and scss/css fil).    When a user generates a file, the metadata and other information is stored in the db for later retrieval and action in another part of the app.   Through testing or error, I realized the DB and file system could become out of sync, so I created a function to walk through the db and match each record to a directory, and then the other way around (walk through the file system and match it to the db.

I use both fs-plus (fs) and fs-extra (fse) to achieve the recursive search and removal of directories.

The code seems fragile and confusing, any feedback would be appreciated.

walkStyles: function() {
  // First let's lookup styles and make sure there is a directory for them.  If not, delete it from the db.
  Style.find({system: false}, function(err, result) {
    if (err) {
      console.log(err);
      return null;
    }
    if (!result) {
      console.log('No styles found in db');
      return null;
    }
    _.forEach(result, function(style)  {
    // make sure I can access the style fil.
      fs.access('./content/userstyles/' + style._id + '/style.css', function(err) {
        if (err && err.code === 'ENOENT') {
          //Can't find the style file, i need to delete it
          Style.remove({_id: style._id}, function(err, result) {
            if (err) {
              console.log('Error removing style from the db');
            }
              if (result) {
                console.log('Out of sync style pruned');
              }
            });
          }
        });
        console.log('Matched style from DB to disk.   No action.');
    });
  });
  // let's walk the other way and make sure all styles found on disk have a match in the db at the folder level.
  fsp.traverseTree('./content/userstyles',  // walk through the content directory
     function(file) {
    console.log('File: ' + file);
    },
  function(dir) {
    let _id = dir.replace('content\\userstyles\\', '');  // on dir, trim to just the id.  this can be matched to the DB _id field.
    console.log(_id);
    Style.find({_id: _id}, function(err, result) {
      if (err) {
        console.log(err);
      }
      if (result) {  // found the style
        console.log('Style synced from disk to db.  No action.');
      }
      if (!result) {  // didn't find the style.
        console.log('Style found on disk, but not in the db.   Removing from disk.');
        fs.remove('./content/userstyles/' + _id, function(err) {
          if (err) {
            console.log(err);
          }
        });
      }
    });
  },
    function() {
  console.log('done checking styles.');
});
}

./content/userstyles/' + style._id + '/style.css

This appears to be a very brittle convention. It assumes the styles live in a specific directory. Not following the convention, it might lead to more unsynced files. You need a more robust way to handle this. For instance, ID'ing the file using its path.

fs.access('./content/userstyles/' + style._id + '/style.css', function(err) {
    if (err && err.code === 'ENOENT') {
      //Can't find the style file, i need to delete it
      Style.remove({_id: style._id}, function(err, result) {

Instead of a back-to-back filesystem and DB operation, why not operate in batches. Like first get a list of files from DB. Next, check if you can access them all. Only after then do you wipe the DB with missing ones. This way, you have a much more linear set of operations instead of one step doing too many at a time.

It can even be simplified into something like similar to this pseudo-code implementation:

db.getStyles()                                      // Get all styles
  .map(path => { path, hasAccess: hasAccess(path)}) // Get access
  .filter(file => !file.hasAccess)                  // Get unaccessible
  .forEach(path => db.delete(path));                // Delete

Now if I rememer correctly, fs-plus has a method to list down all the possible paths under a given path. I think it was listTreeSync. This way, instead of having a callback run on every file and directory, you can simply analyze the array of paths returned by the function. Equates to lesser filesystem operations.

fsp.listTreeSync(dir)
   .map(path => extractId(path))
   .forEach(id=> storeOrDeleteId(id));

Also, I recommend writing it in a synchronous manner. Unless you're multiplexing other operations in the same app together with this, you'll see no benefit from writing this in an asynchronous manner if this is the only operation that runs at a given time.


Given a sorted array in increasing order, create a BST of minimum height I know there are a few recursive ways to solve this problem but the code below is what came to mind for me.

void create_bst(node *root, vector<int> arr)
{
    int len = arr.size();
    add_nodes(root,len-1); //Make the tree structure
    inorder_traversal(root,arr);
}

void add_nodes(node *n, int len)
{
queue<node *> bfs;
bfs.push(n);
int i = 1;

    while(!bfs.empty())
    {
    node *temp = bfs.front();
    bfs.pop();

    temp->left = new node(i++);
    len--;
    if(len == 0)
    {
        break;
    }
    bfs.push(temp->left);

    temp->right = new node(i++);
    len--;
    if(len == 0)
    {
        break;
    }
    bfs.push(temp->right);
    }
}

void inorder_traversal(node *n, vector<int> arr)
{
static int i=0;
if(n->left != NULL)
{
    inorder_traversal(n->left,arr);
}
if(n != NULL)
{
    n->data = arr[i];
    i++;
}
if(n->right != NULL)
{
    inorder_traversal(n->right,arr);
}
}

The add nodes function creates the BST and the inorder traversal function inserts the appropriate values at each node. Is the algorithm above an efficient way to solve this problem?

Your code will only work one time, because of the static int i in your inorder_traversal function. You have no way to reset i to 0
Splitting creation and filling into two parts is not what I'd call efficient. This way you have to traverse the tree twice: once while creating it and once while filling it. And if you do not depend on a complete binary tree, managing a queue is a wast of time and unnecessary complexity to whom ever is trying to read your code.

My advice would be, work recursive on recursive data. The recursive way is much shorter and much easier to read, isn't it?

node* create_bst(vector<int> const& arr)
{
  return create_bst(arr, 0, arr.size());
}

node* create_bst(vector<int> const& arr, int start, int end)
{
  if (start >= end)
    return null;

  int m = (start + end) / 2;

  node* root = new node(arr[m]);

  root->left = create_bst(arr, start, m);
  root->right = create_bst(arr, m + 1, end);

  return root;
}


Need a feedback on my Javascript code and app implementation idea I'm new into Javascript, and I'm not sure that is a good approach. The code works, and does what I need it to do but I'm sure I didn't do the things the right way.

Can you give me feedback about the implementation idea and code?

So, let's say that in index.html I have the following code into body section.

<script type="text/javascript" src="js/main.js" language="javascript"></script>

The content of the main.js file is the following:

document.write('<script language="javascript" type="text/javascript" >');

function loaded() {
}

loaded();
document.write('</script>');

I know that using document.write it is not a smart thing at all. I'm sure that other things are faulty, but I'm newbie and I'm looking for your feedback.

From a quick glance there are a couple of points where your making simple mistakes.

// Document.write is bad
document.write('<script language="javascript" type="text/javascript" >');

// use css definitions instead
counter_div.setAttribute('style', 'width: 310px; height: 50px; font-family:lucida,tahoma,helvetica,arial,sans-serif; display: block; overflow:hide;');

// dont set inner html. This is bad. use DOM manipulation instead
title_span.innerHTML = meter_title;

// uses eval here. pass a function rather then a string
setInterval("increment()", interval);

// forgetting to declare addCommas with `var`. This is implecetly global.
addCommas = function(nStr) {

I shall have a look at how to redesign this.

Why do you need to append the div after the script tag. It's bad by design to use the script tag in the DOM for positioning.

it would be better to create a <div> to fill with the timer/counter.

Refactored partly here. This includes fixing the issues above plus making the dom manipulation a bit nicer.

I'm not too certain how to do refactoring on your timer code.


Populating a list of all .wtv files across 4 hard drives I am using Vba excel to populate a list of all my .wtv files using the .GetDetailsOf method. There are some 3000 files across 4 hard drives filed in a b c... subdirectories.

This sends my memory usage from just over 4 (normal) to nearly the full 8 GB available and halts my computer for a short time.

Is there a better method or something else i can set that doesn't force windows to open every file?

(This is a mash up of two codes on the net found here and here

Option Explicit
Sub GetDetails()

Cells.Select
Selection.ClearContents
Range("A1").Select

Dim fso, oFolder, oSubfolder, queue As Collection 'removed oFile,

Dim dirs As Variant
Dim oShell As Object
Dim oFile As Object
Dim oFldr As Object
Dim lRow As Long
Dim iCol As Integer

Dim fArray As Variant
fArray = Array("K:\Video_Store\Movies_a_k", "I:\Video_Store\Movies_j_r", "J:\Video_Store\Movies_The", "H:\Video_Store\Movies_s_z")
Dim vArray As Variant
vArray = Array(0, 21, 27, 1, 260, 15, 253, 261, 16, 259, 177, 19) 'removed
'0=Name, 21=Title, 27=length, 01=size, 260=recordingtime, 15=year, 253=channelno,
'261=stationcallsign, 16=genre, 259=progdesc, 177=path, 19=rating

Set fso = CreateObject("Scripting.FileSystemObject")
Set queue = New Collection

lRow = 1

For dirs = LBound(fArray) To UBound(fArray)
    queue.Add fso.GetFolder(fArray(dirs)) 'obviously replace

    Do While queue.Count > 0
        Set oFolder = queue(1)
        queue.Remove 1 'dequeue
        '...insert any folder processing code here...
        For Each oSubfolder In oFolder.SubFolders
            queue.Add oSubfolder 'enqueue

            Set oShell = CreateObject("Shell.Application")
            Set oFldr = oShell.Namespace(CStr(oSubfolder))
                With oFldr
'                    For iCol = LBound(vArray) To UBound(vArray)
'                        Cells(lRow, iCol + 1) = .GetDetailsOf(.Items, vArray(iCol))
'                    Next iCol
                    For Each oFile In .Items
                        lRow = lRow + 1
                        For iCol = LBound(vArray) To UBound(vArray)
                            Cells(lRow, iCol + 1) = .GetDetailsOf(oFile, vArray(iCol))
                        Next iCol
                    Next oFile
                End With

        Next oSubfolder
'        For Each oFile In oFolder.Files
'            '...insert any file processing code here...
'        Next oFile
    Loop

Next dirs

End Sub

Your macro/procedure is implicitly working off the ActiveSheet: everywhere you use an unqualified Cells or Range, what VBA is seeing is ActiveSheet.Cells or ActiveSheet.Range: if you're supposed to work off Sheet1 or, if you gave it a code name, some DestinationSheet worksheet, use that instead, so that VBA doesn't need to resolve what the active sheet is every time you refer to it.

You should also avoid using .Select and .Activate and working off the Selection whenever possible.

The procedure is doing way too many things; break it down! You need a procedure whose role is to clear the contents of the DestinationSheet. Then you need a function that takes some rootPath and recursively returns all .wtv files under it and its subdirectories: you want to flatten that tree and work off a straight list of filenames to process (simply tweak that code to make it a Function and fill up and return some Collection instead of Debug.Printing as it goes). Seems your code will process any file, not just .wtv ones.

This is a missed opportunity:

Dim vArray As Variant
vArray = Array(0, 21, 27, 1, 260, 15, 253, 261, 16, 259, 177, 19) 'removed
'0=Name, 21=Title, 27=length, 01=size, 260=recordingtime, 15=year, 253=channelno,
'261=stationcallsign, 16=genre, 259=progdesc, 177=path, 19=rating

Consider declaring an Enum type here:

Private Enum FileInfoField
    Name = 0
    Size = 1
    Year = 15
    Genre = 16
    Rating = 19
    Title = 21
    Length = 27
    Path = 177
    Channel = 253
    ProgDesc = 259
    RecordingTime = 260
    StationCallsign = 261
End Enum

And now you can do away with these comments and mental mappings:

Dim infoFields As Variant
infoFields = Array(Name, Size, Year, Genre, Rating, Title, Length, Path, Channel, ProgDesc, RecordingTime, StationCallsign)

Note the identifier infoFields is much more descriptive than vArray which says essentially nothing other than "well it's a variant array", assuming the v is a Hungarian Notation prefix for "variant".

Because we've recursively found all files under a given root folder already, I don't think we need that queue overhead anymore.

Remember we're collecting the data at this point, so there won't be any Cells(lRow, iCol + 1) = assignment here. Instead we'll .GetDetailsOf each file and perhaps populate a data type - I'd add a class module here, to represent some "file info" data; the class doesn't need to be very complicated, it's just a "dumb data container", so a bunch of public fields can do:

Option Explicit

Public Name As String
Public Genre As String
Public Title As String
Public Size As String
'...

Or we can make it a little more than that, and embed some of the functionality in that class - in that case I'd opt for get-only properties instead, move the FileInfoField definition to that class module, and rename its members to avoid  clashing identifiers - I'd call that class MovieFileInfo:

Option Explicit
Private Enum FileInfoField
    NameField = 0
    SizeField = 1
    YearField = 15
    GenreField = 16
    RatingField = 19
    TitleField = 21
    LengthField = 27
    PathField = 177
    ChannelField = 253
    ProgDescField = 259
    RecordingTimeField = 260
    StationCallsignField = 261
End Enum

Private Type TMovieFileInfo
    Name As String
    Genre As String
    Title As String
    Size As String
    '...
End Type

Private this As TMovieFileInfo

Public Property Get Name() As String
    Name = this.Name
End Property

Public Property Get Genre() As String
     Genre = this.Genre
End Property

Public Property Get Title() As String
    Title = this.Title
End Property

Public Property Get Size() As String
    Size = this.Size
End Property

'...

Public Sub LoadFrom(ByVal parentFolder As Shell32.Folder, ByVal info As Shell32.FolderItem)
    With parentFolder
        this.Name = .GetDetailsOf(info, NameField)
        this.Title = .GetDetailsOf(info, TitleField)
        this.Size = .GetDetailsOf(info, SizeField)
        this.Genre = .GetDetailsOf(info, GenreField)
        '...
    End With
End Sub

Back to your loops. oShell doesn't need to be re-created at every single iteration of the inner-inner loop; it can very well be declared and assigned before any looping is done, and destroyed after all loops complete. The oFldr reference could be held by the With block itself - instead of this:

Set oFldr = oShell.Namespace(CStr(oSubfolder))
With oFldr

You could have this:

With oShell.Namespace(subfolder.Path)

And then the object reference is gone at End With.

Notice Path is teh default property of a Scripting.Folder, so by doing CStr(oSubfolder) you're really just fetching its Path property.... quite indirectly.

I'd suggest you add an actual reference to the Microsoft Scripting Runtime and Microsoft Shell Controls and Automation libraries, and early-bind all those, and work with strongly-typed objects instead of Variant and Object, which are costing you lots of overhead here.

So, something like this (I think I'm getting confused with the actual involved types here though):

Dim item As MovieFileInfo
Dim info As Shell32.FolderItem
Dim allFiles As Collection

With oShell.Namespace(subfolder.Path)
    For Each info In subfolder.Items
        Set item = New MovieFileInfo
        item.LoadFrom subfolder, info
        allFiles.Add item
    Next
End With

Anyway, so you traverse your flat list of files and populate a collection of MovieFileInfo items.

Once you have that, you dimension an array large enough to fit all rows and all columns, and then you populate it by iterating your collection.

Once you have that 2D array, you just dump it onto the target worksheet in one, single, instant worksheet write.

Alternatively you could skip the MovieFileInfo part and write directly to the array, but the constant resizing to accomodate each new record might not be worth it.

So, to recap:

Reduce nesting by extracting loop body into specialized procedures/functions
Avoid writing to cells as much as possible
Early-bind references as much as possible, avoid Variant and Object types
Avoid implicitly working off the active sheet; use explicit worksheet references whenever possible


Project Euler #1 - JavaScript I'm new to JavaScript, and decided to learn the language by running through as many of the Project Euler problems as I have time to do. I just finished the first one, and am looking for some feedback, specifically on language specific features I could use to do this faster, and faster performance overall (it seems to run a little slow) after some edits to fit the problem, it runs much faster, the document.write seemed to be really slowing the code down (probably from directly editing the page once every division of 3 or 5), and writing once value makes a huge difference.

Working fiddle here.

document.write = function (val) {
    document.body.insertAdjacentHTML("beforeend", val);
}

//If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3, 5, 6 and 9. The sum of these multiples is 23.

//Find the sum of all the multiples of 3 or 5 below 1000.

var ans = [];
var k = 0;

for (var i = 1; i < 1000; i++) {
    if (i % 3 === 0 || i % 5 === 0) {
        ans[k] = i;
        k++;
    }
}

var sum = 0;

for (var i = 0; i < ans.length; i++) {
    sum += ans[i];
}

document.write(sum);

//If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3, 5, 6 and 9. The sum of these multiples is 23.

//Find the sum of all the multiples of 3 or 5 below 1000.

var ans = [];
var k = 0;

for (var i = 1; i < 1000; i++) {
    if (i % 3 === 0 || i % 5 === 0) {
        ans[k] = i;
        k++;
    }
}

var sum = 0;

for (var i = 0; i < ans.length; i++) {
    sum += ans[i];
}

document.write(sum);

You missed the specification.

  Find the sum of all the multiples of 3 or 5 below 1000.

You print all of the multiples of 3 or 5 below 1000. To meet the spec, you need to actually sum up all of the elements of ans.


Detecting the attachment of a DataContext In our application the views and the viewmodels are created in parallel to increase the performance and when the viewmodel is created it is attached to the datacontext.

While there is no datacontext the view shows an indefinite progressbar. To hide it we use the following code:

    <!-- Loading background. It gets removed when the datacontext is attached -->
    <Border Visibility="{Binding IsDataContextLoaded, Converter={StaticResource NegatedBooleanToVisibilityConverter}, Mode=OneWay, FallbackValue=Visible}" >
        <views1:CircularProgressBar Visibility="Visible" />
    </Border>

And the viewmodel has the following code:

    /// <summary>
    /// Gets a value indicating whether the datacontex has been loaded or not
    /// </summary>
    /// <remarks>If the view can reach this property it means that the datacontext
    /// has been loaded.</remarks>
    public bool IsDataContextLoaded
    {
        get
        {
            return true;
        }
    }

The solution looks a bit weird to me, any ideas that are more elegant?

If by:

  While there is no datacontext the view shows an indefinite progressbar.

You mean that the view's DataContext is null until the ViewModel is loaded, then you can simply use this:

<Border Visibility="{Binding, Converter={StaticResource IsNullToVisibilityConverter}, Mode=OneWay, FallbackValue=Visible}" >
    <views1:CircularProgressBar Visibility="Visible" />
</Border>

Which is to say, bind directly to the view model property path (which will be null until the view model is loaded) and convert a null value to mean 'Collapsed' and a non-null value to mean 'Visible'.

This removes the need for an IsLoaded boolean property per view model.

Of course, you will need to write the converter.


Using foldl to add numbers, while validating that they are nonnegative I have a function that will calculate the sum of a list of integer, however if any of the number is negative, it will should an error instead (not throwing exception).

The following is a working Haskell code (Glasgow Haskell Compiler, Version 7.10.3):

addAll :: [Int] -> Either String Int
addAll xs = foldl safeAdd (Right 0) xs
   where
     safeAdd :: Either String Int -> Int -> Either String Int
     safeAdd = (\acc next -> case acc of
                     Left s  -> Left s
                     Right n -> if next < 0
                                 then Left (show next ++ " is not positive")
                                 else Right (n + next))

Examples output of the function:

addAll [1,2,3]  -- Right 6
addAll [-1,2,3] -- Left "-1 is not positive"

Although the code works, but I'm not entirely satisfied with it, because the recursion keeps going until the last element even though errors are encountered early.  Not only that, it feels a little more complicated than it should be.

After reading some SO posts such as this one, I think that the code above can be further simplified using functions like foldM or even applicative <$>.

But, I'm just not sure how to use those functions, so please do show me a way to simplify the code above.

import Data.Foldable

addAll :: [Int] -> Either String Int
addAll = foldlM safeAdd 0 where
  safeAdd :: Int -> Int -> Either String Int
  safeAdd n next = if next < 0
    then Left (show next ++ " is not positive")
    else Right (n + next)

Refer http://hackage.haskell.org/package/base-4.12.0.0/docs/Data-Foldable.html#v:foldlM


True story about two drag racers I wrote this as an academic exercise over on CA.  It is supposed to tell a true story about two drag racers, one in his 80's who beat a young world champion in cars that traverse a 1/4 mile in 3 seconds at 300 mph.  It is structured to let the old man win every time because it is based on a true story.  I did it mainly to explore objects.  The story has been left out for brevity.

var comedy;
comedy = function () {

    confirm("In the world of motorsports, 'snip'");
    function dragRacer(name, age, drink, cocktail) {
        this.name = name;
        this.age = age;
        this.drink = drink;
        this.cocktail = cocktail;
    }

    var chris = new dragRacer("Chris Karamesines", 82, "Nitro Methane");
    var tony = new dragRacer("Tony Shumacher", 43, null, "Blueberry Vodka in lime soda");

    dragRacer.prototype.rt = function () {
        var reactionTime = Math.random();
        return reactionTime;
    };

    dragRacer.prototype.et = function () {
        var elapsedTime = Math.random() + 3.70;
       return elapsedTime;
    };

    var chrisET = chris.et();
    var tonyET = tony.et();
    var chrisRT = chris.rt();
    var tonyRT = tony.rt();

    if (chrisRT <= tonyRT) {
        chrisRT = tonyRT + 0.09;
    }
    console.log(chrisRT);//test
    console.log(tonyRT);//test

    if (chrisRT <= tonyRT) {
        chrisRT = 0.19 + tonyRT;
    }
    if (tonyET <= chrisET) {
      tonyET = chrisET + 1.2;
    }

    chrisRT = chrisRT.toFixed(2);
    tonyRT = tonyRT.toFixed(2);
    chrisET = chrisET.toFixed(2);
    tonyET = tonyET.toFixed(2);
    console.log(chrisRT);
    console.log(tonyRT);

    console.log(chrisET);//test
    console.log(tonyET);//test

    confirm("Chris' favorite drink is " + chris.drink + " and " + chris.age +//
        " and " + chrisET + " and " + chrisRT + " . ");

    confirm("Tony' favorite cocktail is " + tony.cocktail + " and " + tony.age +//
        " and " + tonyET + " and " + tonyRT + " . ");
};
comedy();

Code-wise:

var comedy;
comedy = function(){...};
comedy();

The first two lines can be simplified into var comedy = function(){...}. If you don't need the function to be anonymous, you can even use function comedy(){...};. If this is the only place you are using this function, you can even go for an immediately invoked function expression: (function comedy(){...})() (at which point the name only serves as documentation). Separating the definition from the call may serve code structuring purposes, but I don't see the need to separate the definition and the declaration here.

OOP classes should use UpperCase names, so function DragRacer(...){...}. If I see a function named dragRacer, I don't expect it to be a constructor.

Prototype definitions should be written just after the constructor. Logically, they both are parts of a class definition. Technically, instantiating a class before it is fully defined could be problematic - if the constructor calls some prototype functions, the prototype needs to be defined before the constructor is called.

I am not well-versed in the motor sport industry, but it seems to me the function names rt and et are not very descriptive. You can get away with single-lettered variables used in function bodies (especially if they are math-heavy), but not in public facing method names. The function body can be simplified as well. Clarity will be maintained by the function name:

DragRacer.prototype.reactionTime = function () {
    return Math.random();
};

chrisRT = chrisRT.toFixed(2); -- at this point, you are changing the type of a variable. While the compiler can handle that, you could reduce the developer performance. First, the semantics of a variable are dependent on where it is used in code. While it is not unusual to have variables change their values, here the meaning of the variable changes dynamically. Second, you might need the float value after you need the string value. In this case, you need two separate variables anyways. If you need help naming variables that only differ in types, you could borrow the hungarian notation: strChrisRT = chrisRT.toFixed(2);. The variable name is still somewhat cryptic, but at least you don't have to worry when to convert from float to string.

I am wondering why a drag racer's cocktail is not considered a drink. Blueberry Vodka in lime soda is surely a drink as far as I can tell.

if (chrisRT <= tonyRT) {
    chrisRT = tonyRT + 0.09;
}
console.log(chrisRT);//test
console.log(tonyRT);//test

if (chrisRT <= tonyRT) {
    chrisRT = 0.19 + tonyRT;
}

Here you have some redundant code. After the first condition body executes, the second condition body will not. If the first condition body does not execute, the second condition body will not execute either, because the condition is the same. In effect, the second condition body is a piece of dead code, and can be removed altogether. As a side note, I'm wondering why the constant is the left argument once, and the right argument the second time.

I know the purpose is to cheat, but the business logic here is weird anyways. Let me demonstrate with the following table; especially note the discontinuity around 0.00:

chrisRT - tonyRT | -0.02 | -0.01 | 0.00 | 0.01 | 0.02 || 0.08 | 0.09 | 0.10 |
-----------------------------------------------------------------------------
chrisRT - tonyRT |  0.09 |  0.09 | 0.09 | 0.01 | 0.02 || 0.08 | 0.09 | 0.10 |

from the user interface standpoint, confirms are not very nice. Since you are not using the return value, you could use an alert instead, but, if you want a nice interface, you could turn the code into being event driven and communicating via HTML. confirm("hello") => $messages.append($("<p>").text("hello")) (I am using jQuery, but you don't have to). Especially the initial storyline could be present in the initial document as a plain paragraph with a button that runs the javascript part.

  Chris' favorite drink is Nitro Metane and 83 and 0.19 and 0.09 . "

I know these are only debugging messages, but still... what?


Generic tree data structure for flat data from a database Could someone please review my code for performance(any other suggestions are welcome) which converts flat data list coming from database to a tree?

Interface for db entity class

public interface IDbEntityNode
{
     int Id { get; set; }
     int ParentId { get; set; }
     string Data { get; set; }
}

Example of db Entity class

public class ExceptionCategory :IDbEntityNode
{
    public  int Id { get; set; }
    public  int ParentId { get; set; }
    public string Data { get; set; }
    public ExceptionCategory(string data, int id, int parentId)
    {
        Id = id;
        ParentId = parentId;
        Data = data;
    }
}

Generic class which holds the structure of tree node

public class GenericNode<T>
{
    public T NodeInformation { get; set; }
    public GenericNode<T> Parent { get; set; }
    public List<GenericNode<T>> Children { get; set; } = new List<GenericNode<T>>();
}

Method which coverts flat list to tree

public static List<GenericNode<T>> CreateGenericTree<T>(List<T> flatDataObject,Func<T,bool> IsRootNode) where T : IDbEntityNode
{
    var lookup = new Dictionary<int, GenericNode<T>>();
    var rootNodes = new List<GenericNode<T>>();
    var noOfElements = flatDataObject.Count;
    for (int element = 0; element < noOfElements; element++)
    {
        GenericNode<T> currentNode;
        if (lookup.TryGetValue(flatDataObject[element].Id, out currentNode))
        {
            currentNode.NodeInformation = flatDataObject[element];
        }
        else
        {
            currentNode = new GenericNode<T>() { NodeInformation = flatDataObject[element] };
            lookup.Add(flatDataObject[element].Id, currentNode);
        }

        if (IsRootNode(flatDataObject[element]))
        {
            rootNodes.Add(currentNode);
        }
        else
        {
            GenericNode<T> parentNode;
            if (!lookup.TryGetValue(flatDataObject[element].ParentId, out parentNode))
            {
                parentNode = new GenericNode<T>();
                lookup.Add(flatDataObject[element].ParentId, parentNode);
            }
            parentNode.Children.Add(currentNode);
            currentNode.Parent = parentNode;
        }
    }

    return rootNodes;
}

Execution:

private static void Main(string[] args)
{
    List<IDbEntityNode> flatDataStructure = new List<IDbEntityNode>
    {
        new ExceptionCategory("System Exception",1,0),
        new ExceptionCategory("Index out of range",2,1),
        new ExceptionCategory("Null Reference",3,1),
        new ExceptionCategory("Invalid Cast",4,1),
        new ExceptionCategory("OOM",5,1),
        new ExceptionCategory("Argument Exception",6,1),
        new ExceptionCategory("Argument Out Of Range",7,6),
        new ExceptionCategory("Argument Null",8,6),
        new ExceptionCategory("External Exception",9,1),
        new ExceptionCategory("Com",10,9),
        new ExceptionCategory("SEH",11,9),
        new ExceptionCategory("Arithmatic Exception",12,1),
        new ExceptionCategory("DivideBy0",13,12),
        new ExceptionCategory("Overflow",14,12),
    };

    var tree = CreateGenericTree(flatDataStructure, IsRootNode);
}

Root node has ParentId set to 0

private static bool IsRootNode(IDbEntityNode dbEntity)
{
    bool isRootNode = false;
    if (dbEntity.ParentId == 0 )
        isRootNode = true;
    return isRootNode;
}

GenericNode<T>

I'll adopt the name Node<T>, as Henrik Hansen has done.

This class is completely publically mutable, which is probably not ideal. Does it make any sense to change the NodeInformation once you've created the node? Maybe it does, but if not, then you should work to enforce the idea by making it immutable. I'd also wager it makes no sense to have a node without this information, so I'd add a constructor to this effect:

    public T NodeInformation { get; }

    public Node(T nodeInformation)
    {
        NodeInformation = nodeInformation;
    }

The children and parent properties are more tricky, as (without some effort), you can't know who who the children are upfront, and it's even more effort to make both the Children list and Parent immutable. Instead, let's make them privatelly mutable, and add an AddChild method, so that it's more difficult for an external entity to produce an invalid tree:

public Node<T> Parent { get; private set; }

private readonly List<Node<T>> _children = new List<Node<T>>();
public IReadOnlyList<Node<T>> Children => _children;

public void AddChild(Node<T> childNode)
{
    if (childNode.Parent != null)
        throw new ArgumentException(nameof(childNode), "Child node already has a parent");

    _children.Add(childNode);
    childNode.Parent = this;
}

Note that we have a List<Node<T>> so we can add children, but we only expose an IReadOnlyList<Node<T>>, so they can only be added (without dodgy casts) by means of the AddChild method.

IDbEntityNode

Again, everything here is publically mutable: I do not think it makes much sense to change the Id of something! As a general rule, immutable is good if you can have it. It's very easy to add a public setter back in, but it's potentially nightmarish to take it out.
You mentioned wanted more generic IDs: I'd be inclined to make this type generic, with both generic Ids and generic Data. You can always provide a 'convenience' concrete version which has int and string baked in. If you have generic Ids, you either need to make the type comparable, or you'll want to pass CreateGenericTree an IEqualityComparer<TId> to handle it (or you could assume the default comparer, but that's much less fun, and you can again provide a 'convenience' overload which uses it by default).

CreateGenericTree

Conceptually, this appears to create multiple trees, as oppose to one tree (as there can be many roots); should it be called CreateTrees?
You are repeatedly indexing into flatDataObject: much better to loop over something like i, which everyone knows is an index, and set element = flatDataObject[i] (or use a foreach as Henrik Hansen has said).
I think I'm in a minority, but I'd be strongly inclined to use a dedicated delegate type rather than Func<T, bool>. What question does Func<T, bool> answer? It is a horse? Is it lazily loaded? I don't know. You might consider then something like, but a lot of people dislike nominal delegates, for various reasons.

public delegate bool RootDetector<in T>(T node) where T : IDbEntityNode

You are returning a List<T>, and there is a good chance that you could just as well return a more abstract type, such as IList<T> or IReadOnlyList<T>. This give you freedom to change the actual returned type later (e.g. if you change the implementation) without having to change the return type, and if it turns out later that you need some members of a less abstract type, then you can more easily change an abstract return type to less abstract return type than you can the other way round.
You are also taking List<T> as a parameter. As someone consuming your API, this would concern me, as List<T> is openly mutable, and I would unsure whether your code is going to modify what I pass it. Much better to use an abstract type like IReadOnlyList<T>, which gives the caller more freedom, and communicates immediately that you are not going to modify the parameter.
Your code will currently heave oddly if the flatDataObject contains nodes with the same Id. Unfortunately, checking for this would be ugly.

Below is my take (you'll note it's pretty similar to Henrik Hansen's code).

public static IReadOnlyList<Node<T>> CreateTrees<T>(IEnumerable<T> flatDataObjects, RootDetector<T> isRoot) where T : IDbEntityNode
{
    if (flatDataObjects == null)
        throw new ArgumentNullException(nameof(flatDataObjects));
    if (isRoot == null)
        throw new ArgumentNullException(nameof(isRoot));

    var nodes = flatDataObjects.ToDictionary(fdo => fdo.Id, fdo => new Node<T>(fdo));

    List<Node<T>> roots = new List<Node<T>>();

    foreach (var node in nodes.Values)
    {
        if (isRoot(node.NodeInformation))
        {
            roots.Add(node);
        }
        else
        {
            if (nodes.TryGetValue(node.NodeInformation.ParentId, out var parentNode))
            {
                parentNode.AddChild(node);
            }
            else
            {
                throw new InvalidOperationException($"Non-root node {node.NodeInformation.Id} has ParentId {node.NodeInformation.ParentId}, but no node exists with that id");
            }
        }
    }

    return roots;
}

Note that I've added dedicated checks for null parameters, and that upon detecting an invalid input (a node with no parent) I throw a helpful error message. Note also that ToDictionary will throw if two elements have the same key (Which is good), though the message will be less helpful: you could actively trap this exception and throw your own explanation, or otherwise you could manually detected duplicates and throw first/write your own version of ToDictionary to achieve the same.

I would argue that these LINQy versions are much more readable, because each bit of logic is mostly self contained. Even clearer would be to select the roots only at the very end, but there is a good case to be made that calling isRoot as few times as possible is desirable, as we don't know what it does, and partitioning the dictionary would introduce complexity.

Boring things

IsRootNode is a method parameter, and usually these are in lowerCamelCase (e.g. isRootNode). Note that it has the same name currently as the method you are passing to it, which means if you change it to isRootNode you might forget to change the usage if the method is in scope, and then you have a problem (C# warns about unused variables, but not unused parameters).
Pay attention to your white-space: inconsistent white-space can make perfectly good code terribly untidy-looking, and a few more line-breaks here-and-there really help to break up the logic and make the code easier to scan.


Responsive mixin in LESS I wrote a responsive mixin for Less when I couldn't find a full featured one. Curious what other developers have done to solve this problem and hoping to improve/reduce my code since I'm very new to Less:

It would be invoked like:

element {
    overflow: visible;

    .responsive(768px, 0, min, {
        overflow: hidden;
    });
}

Args are: width, height, min|max, rules
This would set overflow: hidden for > 768px. You could also set max in the declaration to set overflow: hidden for < 768px.

.responsive(@width: 0, @height: 0, @type: min, @rules) {
.responsive-max() when(@type = max) {
    .responsive-max-full() when(@width > 0) and (@height > 0) {
        @media only screen and (max-width: @width) and (max-height: @height) {
            @rules();
        }
    }
    .responsive-max-width() when(@width > 0) and (@height < 1) {
        @media only screen and (max-width: @width) {
            @rules();
        }
    }
    .responsive-max-height() when(@width < 1) and (@height > 0) {
        @media only screen and (max-height: @height) {
            @rules();
        }
    }
    .responsive-max-full();
    .responsive-max-width();
    .responsive-max-height();
}
.responsive-min() when(@type = min) {
    .responsive-min-full() when(@width > 0) and (@height > 0) {
        @media only screen and (min-width: @width) and (min-height: @height) {
            @rules();
        }
    }
    .responsive-min-width() when(@width > 0) and (@height < 1) {
        @media only screen and (min-width: @width) {
            @rules();
        }
    }
    .responsive-min-height() when(@width < 1) and (@height > 0) {
        @media only screen and (min-height: @height) {
            @rules();
        }
    }
    .responsive-min-full();
    .responsive-min-width();
    .responsive-min-height();
}
.responsive-max();
.responsive-min();
}

I think you're overcomplicating it. Typically, I think Less users just write the media queries inline with a variable. As in:

@tablet-min: ~"only screen and (min-width: 768)";

element {
  overflow: visible;

  @media @tablet-min {
      overflow: hidden;
  }
}


Smart Pointers with Reference Counting - C++ I am trying to improve my general skills in C++ so I plan to write versions of shared_ptr and unique_ptr from scratch. I wrote a version of shared_ptr with reference counting and I was wondering for reviews.

#ifndef __SHIVA_XSMARTPTRS__
#define __SHIVA_XSMARTPTRS__

/*
This tries to define a reference counted smart pointer class instead of
regular pointers
*/

namespace Shiva
{
template <class T>
class SmartPtr
{
  public:
    //Default Constructor
    explicit SmartPtr(T* p = 0)
    {
      if(p)
        ptrWrapper_ = new PointerWrapper(p,1);
      else
        ptrWrapper_ = new PointerWrapper();
    }

    //The copy constructor
    SmartPtr(const SmartPtr& copyObj)
    {
      ptrWrapper_ = copyObj->ptrWrapper_;
      ptrWrapper_->incrementRefCount();
    }

    //Assignment operator
    SmartPtr& operator=(const SmartPtr& copyObj)
    {
      if(this != copyObj)
      {
        //Decrement and decimate as needed
        unsigned int preReleaseRefCount = ptrWrapper_->getRefCount();
        ptrWrapper_->decrementRefCount();
        if(preReleaseRefCount==1)
        {
          delete ptrWrapper_;
          ptrWrapper_ = 0;
        }
        ptrWrapper_ = copyObj->ptrWrapper_;
        ptrWrapper_->incrementRefCount();
      }
      return *this;
    }

    //The main object destructor
    //Essentially decrement the refcount by 1. If the refcount is equal to 0, then you should delete the pointer
    ~SmartPtr()
    {
      unsigned int preReleaseRefCount = ptrWrapper_->getRefCount();
      ptrWrapper_->decrementRefCount();
      if(preReleaseRefCount==1)
      {
        delete ptrWrapper_;
        ptrWrapper_ = 0;
      }
    }

    //Referncing operator
    T& operator*()
    {
      return ptrWrapper_->getReference();
    }

    //Address operator
    T* operator->()
    {
      return ptrWrapper_->getPointer();
    }

    //Get reference counter
    int getRefCount() const
    {
      return ptrWrapper_->getRefCount();
    }

    //Is this the only copy of the object present
    bool unique() const
    {
      return ptrWrapper_->isUnique();
    }

private:
    class PointerWrapper
    {
    public:
      PointerWrapper(T* ptr=0, int refCount = 0)
      :ptr_(ptr),refCount_(refCount)
      {}

      T* getPointer()
      {
        return ptr_;
      }

      T& getReference()
      {
        return *ptr_;
      }

      unsigned int getRefCount()
      {
        return refCount_;
      }

      void decrementRefCount()
      {
        refCount_--;
        if(!refCount_)
        {
          delete ptr_;
          ptr_ = 0;
        }
      }

      void incrementRefCount()
      {
        refCount_++;
      }

      bool isUnique()
      {
        return (refCount_==1);
      }

    private:
      T* ptr_;
      int refCount_;
    };

    PointerWrapper* ptrWrapper_;
};
}

#endif

Review

Issue

My first issues is that using your shared pointer is not safe. Once I give you the pointer. You are responsible for deleting it all situations (especially when exceptions happen).

But if the call to new PointerWrapper(p, 1) fails. ie it throws BadAlloc then you will leak my object. So you need to watch out for that allocation and delete the pointer immediately if you throw.

Missing Feature

You have not implemented move semantics. This is absolutely essential.

Your object is missing a swap function.

Design

I don't think the class PointerWrapper buys you ay functionality or makes the code any easier to write.

nullptr

Prefer to use nullptr over 0 as the non existing pointer. 0 is a number and can easily be converted to other things when you least expect it. nullptr of std::nullptr_t and can only be assigned to pointers and is not auto converted into other things.

General grumbles

I think the assignment operator is overly complex. It would be much simpler to write this as using the standard copy and swap idiom.

    SmartPtr& operator=(const SmartPtr& copyObj)
    {
         SmartPtr  copy(copyObj);
         copy.swap(*this);
         return *this;
    }

This logic seems a bit overly complex.

        unsigned int preReleaseRefCount = ptrWrapper_->getRefCount();
        ptrWrapper_->decrementRefCount();
        if(preReleaseRefCount==1)
        {
          delete ptrWrapper_;
          ptrWrapper_ = 0;
        }

Personally I would have got decrementRefCount() to return true if it deleted the object.

        if (ptrWrapper_->decrementRefCount())
        {
          delete ptrWrapper_;
        }

No point in setting the ptrWrapper_ to nullptr

           ptrWrapper_ = 0;

When the very next statement sets it to another value.

        ptrWrapper_ = copyObj->ptrWrapper_;

Self Plug

These articles are a good read (I wrote them):

Unique Ptr
Shared Ptr
Constructor