Checking for viruses in uploaded files using ClamAV I have to check an uploaded file for anti-virus and if there is an exception, I have to give a message according to the exception to the users. e.g. if its a timeout, I would ask the user to try againg in a while.

My first class, fileUpload is:

public class FileUpload {

    //Message when no virus found.
    private static final String NO_VIRUS_FOUND =  "No virus found";

    //More attributes and functions

    public void checkFile(FacesContext ctx, UIComponent comp, Object value) throws IOException {
          file = (Part) value;
          if (file != null) {
                  String antiVirusResult;
                  antiVirusResult = FileCheckClass.checkVirus(getConfig(), file.getInputStream(), NO_VIRUS_FOUND);
                 if (!antiVirusResult.contains(NO_VIRUS_FOUND)) {

                     generateAntiVirusMessages(antiVirusResult);
                     throw new ValidatorException(msgs);
                 }
          }
     }

}

My seond class, FileCheck class has a checkVirus() function:

public static String checkVirus(FileConfig fileconfig, InputStream inputStream, String noVirusFound) {
        try {

            ClamAVClient cl = new ClamAVClient(fileConfig.getClamavHost(), fileConfig.getClamavPort(), DEFAULT_TIMEOUT);

            if(ClamAVClient.isCleanReply(cl.scan(inputStream))){
                return noVirusFound;
            }
        } catch (Exception e) {
            logger.error("Error while scanning file: ", e);
            return e.toString();
        }
        return noVirusFound;
    }

From the check virus function, i have to return a string so I pass NO_VIRUS_FOUND into it from the first class.

Is it an acceptable practice to pass a string from one class to another just for the purpose of string comparison at the end. My fear is that if I define the strings separately in two class, change in one class would severely affect the output of the function in the other.
Can there could be some improvements that some one can suggest to the above situation?

there seems to be logical falacy: what if ClamAVClient.isCleanReply() returns false but no exception is thrown? there is no else clause there, and checkVirus would return no virus found.
instead of returning String, why doesn't checkVirus() re-throw the exception? that way, it can also return a String of a virus name (or whatever is produced from ClamAVClient)
instead of passing the String, make it public. then any class can refer to it as FileUpload.NO_VIRUS_FOUND
better still, make an enum out of the constant. that way, you can add values to the enum (like VIRUS_FOUND ...)

though it is arguable, Coding by exception is considered an anti pattern.  Exceptions should be reserved for error conditions, not as indicator that a virus  was found.

EDIT:

with regards to the question how to implement pt 2:
First, I would create a custum exception that signifies a failed virus check

public class VirusCheckException extends Exception {
    public VirusCheckException(Throwable cause) {
        super(cause);
    }
    public VirusCheckException(String message, Throwable cause) {
        super(message, cause);
    }
    // also rest of constructors...
}

then, instead of returning the exception message, throw the custom exception. the return value is freed to return more rich data from the scan (if possible):

public static String checkVirus(FileConfig fileconfig, InputStream inputStream, String noVirusFound)
throws VirusCheckException {
    try {
        ClamAVClient cl = new ClamAVClient(fileConfig.getClamavHost(), fileConfig.getClamavPort(), DEFAULT_TIMEOUT);

        Object response = cl.scan(inputStream);
        if (ClamAVClient.isCleanReply(response)) {
            return noVirusFound;
        } else {
            return ClamAVClient.getVirusName(response);  // assuming there is such...
        }
    } catch (Exception e) {
        return new VirusCheckException("Virus Found!", e);
    }
}

now you will have to catch that exception in the callng method and handle it:

public void checkFile(...) {
    try {
        antiVirusResult = FileCheckClass.checkVirus(getConfig(), file.getInputStream(), NO_VIRUS_FOUND);
    } catch (VirusCheckException e) {
        generateAntiVirusMessages(e.getMessage());
        throw new ValidatorException(msgs);
    }
}


Function to add two binary numbers in C I tried to write a function bin_add() in C to add two positive binary numbers represented as zero terminated strings:

#include <errno.h>
#include <stdbool.h>
#include <stddef.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>

#define MAX(x, y) ((x) > (y) ? (x) : (y))

size_t gpp(char const *s)
{
    char *n = strchr(s, '.');
    return n  ? (size_t)(n - s + 1) : 0;
}

char* bin_add(char const *a, char const *b)
{
    char const *inp[] = { a, b };
    size_t ll[] = { strlen(a), strlen(b) };
    size_t pp[] = { gpp(a), gpp(b) };
    size_t OO[2], off[2];

    for (size_t i = 0; i < 2; ++i) {
        OO[i] = pp[i] ? pp[i] - 1 : ll[i];
        pp[i] = pp[i] ? ll[i] - pp[i] : 0;}

    for (size_t i = 0; i < 2; ++i)
        off[i] = OO[i] < OO[!i] ? OO[!i] - OO[i] : 0;

    size_t ML = MAX(OO[0], OO[1]) + MAX(pp[0], pp[1]) + (!!pp[0] || !!pp[1]);
    char *Ol = calloc(ML + 2, 1);
    if(!Ol) return Ol;

    char ops[2];
    int xc = 0;
    size_t lO = ML;
    unsigned cc[2] = { 0 };
    for (size_t i = ML; i; --i) {
        bool pt = false;
        for (size_t l = 0; l < 2; ++l) {
            ops[l] = i <= ll[l] + off[l] && i - off[l] - 1
                       <  ll[l] ? inp[l][i - off[l] - 1] : '0';
            if (ops[l] == '.') {
                if (cc[l]) {
                    free(Ol);
                    return NULL;
                }
                pt = true;
                ++cc[l];
            }
            ops[l] -= '0';
        }
        if (pt) {
            Ol[i] = '.';
            continue;
        }
        if ((Ol[i] = ops[0] + ops[1] + xc) > 1) {
            Ol[i] = 0;
            xc = 1;
        }
        else xc = 0;
        lO = (Ol[i] += '0') == '1' ? i : lO;
    }
    if((Ol[0] = '0' + xc) == '1') return Ol;
        for (size_t i = 0; i <= ML - lO + 1; ++i)
            Ol[i] = Ol[lO + i];

    return Ol;
}

int main(void)
{
    char a[81], b[81];

    while (scanf(" %80[0.1] %80[0.1]", a, b) & 1 << 1) {
        char *c = bin_add(a, b);
        if (!c && errno == ENOMEM) {
            fputs("Not enough memory :(\n\n", stderr);
            return EXIT_FAILURE;
        }
        else if (!c) {
            fputs("Input error :(\n\n", stderr);
            goto clear;
        }

        char* O[] = { a, b, c };
        size_t lO[3], Ol = 0;
        for (size_t i = 0; i < 3; ++i) {
            lO[i] = gpp(O[i]);
            lO[i] = lO[i] ? lO[i] : strlen(i[O]) + 1;
            Ol = lO[i] > Ol ? lO[i] : Ol;
        }

        putchar('\n');
        for (size_t i = 0; i < 3; ++i) {
            for (size_t l = 0; l < Ol - lO[i]; ++l, putchar(' '));
        puts(O[i]);
        }
        putchar('\n');

        free(c);
        clear :{ int c; while ((c = getchar()) != '\n' && c != EOF); }
    }
}

The main() is just included to provide input to the function and pretty-print its results. Every C99 compliant compiler should be able to compile above code.

Do you see any flaws in my code and anything that could be improved?

Do you see any flaws in my code and anything that could be improved?

Segregate code

Splitting into bin_add.c, bin_add.h, main.c would help delineate what is the code, its user interface and test code.

No compilation errors

As posted now, I did not notice any warnings either- good.

Some comments would help

gpp() would benefit with at least a line comment about its goal, expected input, output, etc.  Same for bin_add() - which should alert that the return pointer needs to be free'd.  This becomes even more important when the user only has access to the declaration in a .h file.

Commenting some of the block of code would help too.

When to shift

When there is not a final carry, code shifts Ol[].  As a final carry with this FP-like code is more rare, I'd shift when there is a carry.

Collapsing

With floating point strings, I expect code to drop trailing zero digits to the right of the '.'.

Leading zero digits are possible based on input.  Perhaps eat those too with an early while (*a == '0') a++; and with b. - depends on coding goals though.

Inconsistent bracket style

//                                v ??
pp[i] = pp[i] ? ll[i] - pp[i] : 0;}

Hopefully code is auto-formatted.

Inconsistent indentation

if((Ol[0] = '0' + xc) == '1') return Ol;
//  v   Why indented here?
    for (size_t i = 0; i <= ML - lO + 1; ++i)
        Ol[i] = Ol[lO + i];

This implies code is not auto formatted.  Save time, and use auto-formatting.

Terse digit like object names lose clarity

The short object names OO, lO, O, Ol, ll look too much like 00, 10, 0, 01, 11.  Consider more clear alternatives.

Other examples:
int xc as the carry bit looks more clear as int carry.
size_t ML more meaningful as MaxLength.

Input error detection

I'd suggest a separate bool bin_valid(const char *s) and let bin_add() assume  valid strings a,b.  This would help simplify - a NULL return would only indicate out-of-memory.

Good use of cast to ward off warnings

return n  ? (size_t)(n - s + 1) : 0;
//          ^------^

Misc.

ops[2], cc[2] could be local to for (size_t i = ML; i; --i) {

Good use of const.

Good use of size_t.

Personal preference: Consider char *Ol = calloc(ML + 2, sizeof *Ol);

Main

Do not assume EOF is -1

Simply test if the scanf() result is 2.

// while (scanf(" %80[0.1] %80[0.1]", a, b) & 1 << 1) {
while (scanf(" %80[0.1] %80[0.1]", a, b) == 2) {

ENOMEM

ENOMEM is not part of the standard C.

Test cases

Some specific example test cases would be useful.

  maybe even C90/C89

Not quite.

Lots of error: 'for' loop initial declarations are only allowed in C99 or C11 mode problems

warning: control reaches end of non-void function [-Wreturn-type]

error: redefinition of 'i'


Can someone help me optimize my DFS implementation in Python? def dfs(graph, node):
"""Run DFS through the graph from the starting
node and return the nodes in order of finishing time.
"""
stack = [[node, True]]
while True in [x[1] for x in stack]:
    i = 0
    for x in xrange(len(stack)):
        if stack[x][1] == True:
            i = x
            break
    stack[i][1] = False
    stack = [[x, True] for x in graph[stack[i][0]] \
                 if (([x, True] not in stack) and ([x, False] not in stack))] + stack
return [x[0] for x in stack]

Input: Graph as adjacency list Eg: {1:[2, 3], 2:[1], 3:[]}
       Node is an integer value from where depth first search begins Eg: 1
Output: The list of nodes in order of their finishing times.
        Eg: [3, 2, 1]

For large graphs, the above code is taking too long to execute. Is there a better way to do this?
Edit: I could do better time by implementing the following code. Is it possible to do even better?

def dfs(graph, node):
"""Run DFS through the graph from the starting
node and return the nodes in order of finishing time.
"""
seen = defaultdict(bool)
stack = [node]

while True:
    flag = True
    for x in stack:
        if not seen[x]:
            stack = [a for a in graph[x] if not seen[a]] + stack
            seen[x] = True
            flag = False
            break
     if flag:
        break
return stack

Edit: I've changed seen and stack variables to sets. But I'm not getting the required performance boost. What am I doing wrong?

def dfs(graph, node):
    """Run DFS through the graph from the starting
    node and return the nodes in order of finishing time.
    """
    seen = set()
    stack = {node}
    while True:
       flag = True
       for x in stack:
           if x not in seen:
                stack = stack.union(set(graph[x]))
                seen = seen.union({x})
                flag = False
                break
        if flag:
            break
    return list(stack)

def dfs(graph, node):

This isn't a depth first search

    """Run DFS through the graph from the starting
    node and return the nodes in order of finishing time.
    """
    seen = set()
    stack = {node}

You aren't using this as a stack, don't call it that

    while True:
       flag = True

avoid flag variables, they confound code. Also that's a very undescriptive name.

       for x in stack:
           if x not in seen:

Seen these are sets, you should be probably use: for x in stack - seen: which should be more efficient

                stack = stack.union(set(graph[x]))

Using this approach will make things rather slow, you just keep making stack bigger and bigger.

                seen = seen.union({x})

Use seen.add to add things to the set. Using union like this will be much slower

                flag = False
                break
        if flag:
            break
    return list(stack)

The set gives you no guarantees as to the order of the elements. I'm guessing that's now what you wanted. As it stands,this will end up giving you all the elements in your graph in a random order.

My approach

def breadth_first_search(graph, start):
    seen = set()
    current = {start}
    history = []

    while current:
         history.extend(current) # record when we found all these elements
         seen.update(current) # add them to the list of elements we've already seen
         neighbours = set() # generate a set of all elements that we can reach from these ones
         for node in current:
             neighbours.update(graph[node])
         current = neighbours - seen # the next round of elements is this one, except those we've already seen

    return history


Creating table from code behind I have to create a table that will allow users to upload different documents for each of the entry in the table. To be more exact, each row will contain a unique ID on first column, followed by 3 inputs (upload) for video, text and image, and a save button (each on next column of the row).

The problem is that I have a pretty big number of rows in the table, so creating the table in HTML code is pretty much out of the question.

The code I made generates HTML code in the code behind, and prints it in a literal, so basically it looks like this:

foreach (row tempR in allRows)
        {
            dataToShow += "
";
            dataToShow += "" + tempR.IDrow + "	";
            dataToShow += "<input type=\"file\" id=\"uploadVideo" + tempR.IDrow + "\" runat=\"server\" />	";
            dataToShow += "<input type=\"file\" id=\"uploadPDF" + tempR.IDrow + "\" runat=\"server\" />	";
            dataToShow += "<input type=\"file\" id=\"uploadImg" + tempR.IDrow + "\" runat=\"server\" />	";
            dataToShow += "<button runat=\"server\" class=\"btn\" id=\"btnSave_" + tempR.IDrow + "\">Save</button>	";
            dataToShow += "
";
        }
litDoc.Text = dataToShow;

My problem is that I am pretty sure this is the worst possible way in achieving the desired result, but sadly I can't find another way I can achieve it. Also now if I want to handle the click events of the save buttons I need to get the id of the button with jquery and call with ajax a server side method that will handle that event, which I believe is not pretty great either.

Any advices you could give me would be greately appreciated! (the project is in ASP.NET WebForms)

Instead of doing this all in code behind, a different alternative is to use MVC (Model, View and Controller pattern), and let the table be generated by providing a partial view, and then propagate the allRows directly to the partial view. But it could be that using MVC is not available to (or not wanted by) you.

Related to the click event, I'm assuming that all of the buttons are executing the same action, and as such you could use one general save method, which picks up the id from the row where you clicked the button. See following html code example:

  ...

      <input type="file" id="uploadVideo" runat="server" />
      <input type="file" id="uploadPDF" runat="server" />
      <input type="file" id="uploadImg" runat="server" />
      <button runat="server" class="btn">Save</button>

  ...

It is rather easy to make a general save handler, which would pick up the id from the
's data element, and the action url from the
's data element. In addition the html code looks cleaner and is easier to read and handle.

I once built an entire system using data-attributes for different actions, and made general javascript for doing inline editing, deleting, addition. In the backend there were general methods which returned the modified partial view of the edited row. This system allowed me to maintain larger tables, with a minimum of traffic between client and server.


A little Python roguelike with mostly using standard libraries So, I made this in order to practice my Python OOP and algorithms skills, and refactored it a bit after a few months of s tale. May anybody review it in a PR please? Want to see the flaws that could be fixed, considering I didn't improve much since the initial development of this.
https://github.com/fen0s/TABSlike/
One that especially makes me worries is the implementation of getting a 3x3 area around the unit to check for enemy units... Is it fine, and can you understand it overall?

def check_surroundings(self, y_coord, x_coord):
        y_coord -= 1 #to check y level above unit
        enemies = []
        for _ in range(3):
                if self.engine.check_inbounds(y_coord, x_coord-1) and self.engine.check_inbounds(y_coord, x_coord+2):
                    for entity in self.engine.techmap[y_coord][x_coord-1:x_coord+1]:
                        if entity and entity.team != self.team:
                            enemies.append(entity)
                y_coord+=1
        return enemies

Your use of y_coord and range is suboptimal here. You're iterating over a range(3) and ignoring the produced number, while also using y_coord as an iteration variable.

Just iterate over the range of numbers that you want from the start:

def check_surroundings(self, y_coord, x_coord):
    enemies = []
    for cur_y in range(y_coord - 1, y_coord + 2):  # Exclusive end
        if self.engine.check_inbounds(cur_y, x_coord - 1) and self.engine.check_inbounds(cur_y, x_coord + 2):
            for entity in self.engine.techmap[cur_y][x_coord - 1:x_coord + 1]:
                if entity and entity.team != self.team:
                    enemies.append(entity)
    return enemies

The main benefit here is the boost to readability. Ideally, it should be easy to tell what a for loop is iterating for just by looking at it.

for entity in self.engine.techmap[cur_y][x_coord - 1:x_coord + 1]:

It's obvious that you're getting all the matching entities from techmap, then doing something with each.

for _ in range(3):

Here though, you're ignoring the iteration variable, so it isn't clear what is actually changing each iteration. Superficially, it looks like you're just repeating the same body three times, which is odd. It takes a second step of thinking to realize that you're using y_coord to iterate.

This is also arguably a good place to use a list comprehension (although 2D+ list-comprehensions usually tend a little on the ugly side). The point of your nested loops is to produce a filtered list from existing collections (the iterable returned by techmap, and range in this case). That is exactly the use-case for list comprehensions.

def check_surroundings(self, y_coord, x_coord):
    return [entity
            for cur_y in range(y_coord - 1, y_coord + 2)
            if self.engine.check_inbounds(y_coord, x_coord - 1) and self.engine.check_inbounds(y_coord, x_coord + 2)
            for entity in self.engine.techmap[cur_y][x_coord - 1:x_coord + 1]
            if entity and entity.team != self.team]

You may want to save that into a variable then return the variable. It depends on your style. I can't say that I necessarily recommend using the comprehension here, but I thought I'd show that such an option is available. I'll note too that I didn't test this; I eyeballed it. It looks like it should be equivalent though.


bash wrapper around 'git commit' to automatically bump (Python) package CalVer and create matching CalVer tag on new commit I'm doing a lot of Python development lately for various (small-)data analysis pipelines at work. I've been wrestling with how to robustly and ~automatically version the code at a fine-grained level, so as to provide strong guarantees of reproducibility of a result generated from any particular version of the code, at any particular point in my development process.

I've settled on a CalVer approach. Given that I often want to have multiple versions of the code tagged within a single day, I'm using a ~nonstandard $TIMESTAMP format of YYYY.MM.DD.hhmm. (hhmmss seemed like it would be overkill.)

In any event, I want two things to happen every time I commit code to one of these data analysis repos:

Wherever relevant in the package (usually just in the main __init__.py), __version__ should be updated to $TIMESTAMP.
Once the code is committed, a tag named $TIMESTAMP should be applied to the new commit

Ancillary goals are the usual: easy to configure, minimal likelihood of breaking all the things, and minimal additional cleanup effort in common non-happy-path scenarios.

The following is a bash script I've put together for the purpose:

#! /bin/bash

export TIMESTAMP="$( date '+%Y.%m.%d.%H%M' )"
export VERPATH='.verpath'

if [ -z $VERPATH ]
then
  # Complain and exit
  echo "ERROR: Path to files with versions to update must be provided in {repo root}/.verpath"
  echo " "
  exit 1
fi

# $VERPATH must contain the paths to the files to be updated with
# the timestamped version, one per line
while read VERFILE
do
  # Cosmetic
  echo ""

  if [ -e "$VERFILE" ]
  then
    # File to be updated with version exists; update and add to commit.
    # Tempfile with old file stored in case of commit cancellation.
    echo "Updating $VERFILE"
    cp "$VERFILE" "$VERFILE.tmp"
    sed -i "s/^__version__ = .*$/__version__ = '$TIMESTAMP'/" $VERFILE
    git add "$VERFILE"

  else
    echo "$VERFILE not found!"

  fi

done < $VERPATH

# Cosmetic
echo ""

# So user can see what was updated
sleep 2s

# Actually do the commit, passing through any parameters
git commit $@

# If the commit succeeded, tag HEAD with $TIMESTAMP and delete temp file(s).
# If the commit failed, restore the prior state of the $VERFILEs.
if [ "$?" -eq "0" ]
then
  git tag -f "$TIMESTAMP"

  while read VERFILE
  do
    rm -f "$VERFILE.tmp"
  done < $VERPATH

else
  while read VERFILE
  do
    if [ -e "$VERFILE.tmp" ]
    then
      git reset HEAD "$VERFILE" > /dev/null 2>&1
      rm "$VERFILE"
      mv "$VERFILE.tmp" "$VERFILE"
    fi
  done < $VERPATH

fi

The contents of .verpath in my test repo are:

pkg/__init__.py
pkg/__dupe__.py
pkg/nofile.py

Both pkg/__init__.py and pkg/__dupe__.py exist; pkg/nofile.py does not.

I have *.tmp in my .gitignore so that the $VERFILE.tmp don't show up as untracked files when drafting the commit message.

It works like I want it to... the happy path works great, and it handles aborted commits and nonexistent .verpath files gracefully.

I'm no bash expert, though, so I'm partly concerned about subtle misbehaviors I haven't thought of. Also, I'm not super thrilled about the use of in-folder temporary files, and per here and here the while read VERFILE ... done < $VERPATH has the potential to be fragile if I don't set it up correctly.

First of all I'd like to recommend shellcheck (online, GitHub) as a very useful tool to detect errors (even small misspellings of variables) and possible misbehaviours, etc.. With this you would have been warned about double-quoting your variables to prevent accidental splitting.

#! /bin/bash

There is no need to export these variables, since they won't be used after the termination of the script.
I'd personally stay clear from all caps variable names, if they are not environment variables. (By this convention, shellcheck will not complain for these variables if they are unset, as it assumes these to be provided by the shell.)
In principle, there is no need to double-quote a subshell, but it does not hurt.

timestamp="$( date '+%Y.%m.%d.%H%M' )"
verpath='.verpath'

Since you have just set the variable, this if clause will never trigger, as it is never empty. You probably would like to check whether the file exists (and is readable).

if [ -z $VERPATH ]
then
  # Complain and exit
  echo "ERROR: Path to files with versions to update must be provided in {repo root}/.verpath"
  echo " "
  exit 1
fi

I second switching to the bash built-in test [[ expression ]].
For debugging purposes I try to stick with positive evaluations, usually inserting a statement that it works as intended.
While in a simple script like this, echo is perfectly fine, I still recommend looking at printf for more complex things. I'd probably use something like the following:

if [[ -f "$verpath" && -r "$verpath" ]] ; then
  :  # do nothing (or give a debug statement)
else
  # complain and exit
  printf 'ERROR: Paths to files with versions to update should be provided in %s.\n\n' "$verpath"
  exit 1
fi

Be aware that read without the option -r will mangle backslashes (see SC2162). If for whatever reason the carriage return is missing from the last line, it will be ignored, better use the following while loop, where -n tests for a non-zero length string:

while read -r verfile || [[ -n "$verfile" ]] ; do
  : # do something
done < "$verpath"

Like in the other answer suggested, I also prefer to keep track of temporary files, instead of assuming they have been created as intended. This will also spare you of reading in the file again.
Since you are making backups, I would also switch to a different ending like .bak. Initialise the array first:

declare -a backups

I'd again check whether the file exists and is readable.
Since these are backups, I'd also use the -a option to cp to archive them. I'd advise on exiting if the command fails. You might also want to consider checking whether the target backup file does already exist and exit if it does.
The inline substitution with sed might have a catch, if there are fewer of more spaces in the search pattern. I think you could be a bit more greedy.

if [[ -f "$varfile" && -r "$varfile" ]] ; then
  printf 'INFO: Updating %s.\n' "$varfile"
  # Add backup file to array
  backups+=( "$varfile.bak" )
  [[ -e "${backups[-1]} ]] && { printf 'WARNING: backup %s exists.\n' "${backups[-1]}" ; exit 1 ; }
  cp -a -- "$varfile" "${[backups[-1]}" || { printf 'ERROR: backup of %s failed.\n' "$varfile" ; exit 1 ; }
  sed -i "s/^__version__.*$/__version__ = '$timestamp'/" "$verfile"
  git add "$verfile" || { printf 'ERROR: adding %s to repository failed\n' "$verfile" ; exit 1 ; }
  printf '\n'
else
  printf 'INFO: file %s not found.\n\n' "$verfile"
fi

# So user can see what was updated
sleep 2s

The bash built-in $@ is an array, it needs to be double-quoted to prevent resplitting. On the other hand, $* is a string, i.e. it looses the elements of an array. Unquoted, it will also be resplit, which is hardly ever anything you would want.
Always check the exit state of a command directly to prevent it being overwritten.

# Actually do the commit, passing through any parameters
if git commit "$@"
then
# Commit succeeded: tag HEAD with $timestamp and delete backup file(s).
  git tag -f "$TIMESTAMP" || { printf 'WARNING: Adding tag failed.\n' ; }
  rm -f -- "${backups[@]}"
else
# Commit failed: restore the backups to its original location.
  for file in "${backups[@]}" ; do
    mv -- "$file" "${file%.bak}" && git reset HEAD "$file" &> /dev/null
  done
fi

Generally I do all file handlings much more verbose, wrapping everything into a function and switch off the output of that function as necessary, i.e.

#! /bin/bash
mybackup ()
{
  while [[ -n $1 ]] ; do
    : # Things to do
    source="$1"
    shift
    target="$source.bak"
    cp -vp "$source" "$target" >&3 2>&3
  done
  : # more things to do ...
}

if [[ "$1" == "-s" ]] ; then
  exec 3> /dev/null
  shift
else
  exec 3>&1
fi

mybackup "$@"

This would also make things simpler if you were to create logfiles of your script, but that is beyond this review.

Right now your versioning depends on an auxiliary file, that you probably create manually. I don't know how large your repositories are and how often you are using the versioning statements. It might be better to either check every file, or hardcode the ones that need to be checked into the script, instead of an external file.


Using delegates to communicate between forms and networkcommunicator class I have multiple forms, each of which would like to send message through the network, using my NetworkCommunicator class's SendMessage method.

The solution I described here works, however it is ugly, and bludgeons some of the main concepts of OOP paradigm. What I'm looking for (and I'm open to any suggestions) is basically to achieve the same goal but in a more elegant, and OOP conforming solution. (maybe by events?)

I'm using more than these forms, but for the example, these seemed to be enough.

//using statements
namespace examplespace
{
 public delegate void ImprovedSenderDelegate(string message);

 public class LoginForm :Form
  {
   private  NetworkCommunicator ncom = new NetworkCommunicator();
   public Ncom
   {
    get { return ncom; }
   }

   private void Login()
   {
    MainForm mf = new MainForm();
    mf.Owner = this;
    //other things i do etc.
    this.Hide();
    mf.Show();
   }
  }//endofLoginForm

 public class MainForm :Form
 {
  public ImprovedSenderDelegate ISD;

  private void Load ()
  {
   (this.Owner as LoginForm).NCom.SubscribeToSendMessage(this);
  }

  private void I_Want_To_send_A_Message(string message)
  {
   ISD(string);
  }
 }//endofMainForm

 public class NetworkCommunicator
  {
   public void SubscribeToSendMessage(object sender)
    {
     if (sender is MainForm)
     {

         (sender as MainForm).ISD += new ImprovedSenderDelegate(SendMessage);
     }
    }

   private void SendMessage(string message)
    {/*magic*/}
  }/endofNetworkCommunicator
}

My suggestions:

If the NetworkCommunicator exists only once per application, you should create a singelton. Code sample:

    /// <summary>
    /// Provides a base class for singeltons.
    /// </summary>
    /// <typeparam name="T">The class that act as a singelton.</typeparam>
    public abstract class Singleton<T>
        where T : class
    {
        protected Singleton()
        {
        }

        /// <summary>
        /// Gets the singelton instance.
        /// </summary>
        public static T Instance
        {
            get { return Nested._instance; }
        }

        private sealed class Nested
        {
            /// <summary>
            /// Creates the nested instance class.
            /// </summary>
            ///
            /// <remarks>
            /// Explicit static constructor to tell the compiler
            //  not to mark type as beforefieldinit.
            /// </remarks>
            static Nested()
            {
                ConstructorInfo constructor = typeof (T).GetConstructor(Type.EmptyTypes) ??
                                              typeof(T).GetConstructor(BindingFlags.NonPublic | BindingFlags.Instance, null, Type.EmptyTypes, null);

                if (constructor == null)
                    throw new InvalidOperationException("Class has no private nor public constructor");

                _instance = (T)constructor.Invoke(null);
            }

            internal static readonly T _instance;
        }
    }

If someone has any suggestion about my singelton implementation, let me know it..

The delegate makes the code more complex and violates the KISS OOP principle: Keep It Simple and Stupid. Make it as easy as possible first.


JavaScript HTML Title Scrolling I made this little library that makes the title of a webpage scroll so you can see the entire string. I'm not sure if this is the best way to accomplish this so if anyone has any input that'd be great.

title-scroll.js

var start = 3000;
var speed = 250;

function getParams(startAttr, speedAttr) {
    var params = window.document.getElementsByTagName('script');

    for (var i = 0; i < params.length; i++) {
        if (params[i].src.indexOf('title-scroll.js') !== -1) {
            if (params[i].getAttribute(startAttr) !== null && params[i].getAttribute(startAttr) !== "") {
                start = params[i].getAttribute(startAttr);
            }

            if (params[i].getAttribute(speedAttr) !== null && params[i].getAttribute(speedAttr) !== "") {
                speed = params[i].getAttribute(speedAttr);
            }

            return;
        }
    }
}

window.onload = function(e) {
    getParams('data-start', 'data-speed');
    var title_ref = window.document.getElementsByTagName('title')[0];
    var title = title_ref.text;
    var i = 0;

    setTimeout(function() {
        setInterval(function () {
            title_ref.text = title.substr(i, title.length) + "  ---  " + title.substr(0, i);
            i++;

            if (i === title.length) {
                i = 0;
            }
        }, speed);
    }, start);
}

HTML to properly load the file

<script src="title-scroll.js" data-start="3000" data-speed="250"></script>

Edit:

data-start: Time between page load and when the title starts to scroll.

data-speed: Time between each character movement.

A history lesson

I feel obligated to note that similar functionality (in the page body) has already been implemented in native html using the <marquee> tag (mdn). This tag has been deprecated for forever though, because it is deemed a usability nightmare and visually distracting without a cause.

The same issue applies to changing the title of the page. It distracts the user, and may annoy the user. Users can already view the entire page title by hovering over the tab, but most people do not care about what the title is at all.

Avoid onload (and other on... definitions)

Don't use the on... family of attributes to create listeners for events. Your code does not operate in a void. There are other scripts, some of which you do not have control over. When you assign to an on... attribute, you override the previous assigned function. This, in turn, creates hard to debug problems.

Instead, use .addEventListener(..) (mdn). You can add and remove as many listeners as you want without having to worry about other code.

var clicker = document.getElementById('clicker');

clicker.onclick = function () {
  console.log('First handler');
}
clicker.onclick = function () {
  console.log('Second function');
}
The problem with onclick (and onload, and other on... attributes):

Click me!

Custom parameters

You are correctly using attributes with the data- prefix for your custom attributes. If we want to use such attributes though, we usually put them on the element we are manipulating, not a random script tag. In this case, it's probably better to put it on the title tag. This will also simplify your code a lot, since you only need to grab the title tag and do not have to worry how you load in your code, or if any other code is using your custom attributes.

Optimalisation of housekeeping in interval callback

You are currently using the following code for housekeeping in your interval callback

i++;

if (i === title.length) {
    i = 0;
}

You could instead leverage the modulo operator to simplify the code.

i = (i + 1) % title.length;

Separator

You are calculating your title length without the separator. This means that the title will not complete a full round and instead will jump back to the beginning when -- is at the start.

Various other things

It is always a good idea to use IIFE (Immediate Invoked Function Expressions) to encapsulate your code. This prevents other scripts from interacting with your variables and functions. An IIFE looks something like the following, and is invoked with the variables it depends on (e.g. a javascript library such as jQuery):

(function () {
  // Your code
})();

You have on inconsistent space in your function declaration at setInterval(function () {. Your variable title_ref is not written in camelcase, while the rest of the variables and functions in your code are written that way.

Other than that your code looks clean. Your variable names are clear and make it easy to understand what they contain.


Is WebClient.DownloadData acceptable for getting data from a website? Currently on my ASP.NET website when I define a new user I write their data to a .txt file like this:

public class User
{
public string UserName;
public string PassWord;
public string Email;
public string FirstName;
public string LastName;
public string Twitter;
public string FaceBook;
public int Age;
public DateTime DateCreated;

public User(string username, string password, string email, string facebook, string twitter, string firstname, string lastname, int age, DateTime datecreated)
{
    UserName = username;
    PassWord = password;
    Email = email;
    FirstName = firstname;
    LastName = lastname;
    Age = age;
    DateCreated = datecreated;
    Twitter = twitter;
    FaceBook = facebook;

    AddToTextFile();
}

public bool CheckUsername(string username, List<User> users)
{
    for (int i = 0; i < users.Capacity; i++)
    {
        if (users[i].UserName == username) return true;

        else return false;
    }

    return false;
}

public bool CheckPassword(string username, string password, List<User> users)
{
    for (int i = 0; i < users.Capacity; i++)
    {
        if (users[i].UserName == username)
        {
            if (users[i].PassWord == password) return true;

            else return false;
        }

        else return false;
    }

    return false;
}

private void AddToTextFile()
{
    string file = "../users-information.txt";
    StreamWriter sw = new StreamWriter(@file);
    sw.WriteLine("{0} {1} {2} {3} {4} {5} {6} {7} {8}", UserName, PassWord, FirstName, LastName, Email, FaceBook, Twitter, Age, DateCreated);
}

Then on my C# app I use WebClient.DownloadData to get the data and write it to a file.

    private void GetUsers()
    {
        //JavaScriptSerializer serializer = new JavaScriptSerializer();
        //users = serializer.Deserialize<User[]>(Request.Form["users"]);

        WebClient client = new WebClient();
        byte[] file = client.DownloadData("http://kinected-security.com/users-information.txt");

        File.WriteAllBytes(@"../user-information/user-information.txt", file);

        string[] lines = File.ReadAllLines("../user-information/user-information.txt");

        if (lines.Length != 0)
        {
            foreach (string line in lines)
            {
                string[] splited = new string[]{ };
                splited = line.Split(new Char[] {' ' });

                users.Add(new User(splited[0], splited[1], splited[2], splited[3], splited[4], splited[5], splited[6], int.Parse(splited[7]), DateTime.Parse(splited[8])));
            }
        }
    }

Should I be using WebClient.DownloadString? Is there a more efficient way to do this?

Here is my cleanup of the User class. Note that there is a world of a difference between the .Count and the .Capacity properties of a list. Look them up! Your CheckUsername and CheckPassword methods were broken in more than one way.

namespace Foo
{
    using System;
    using System.Collections.Generic;
    using System.Diagnostics.Contracts;
    using System.IO;
    using System.Linq;

    public class User
    {
        private static readonly string filePath = "../users-information.txt"; // Or not readonly if you wish to set it at run time.

        public User(IList<string> tokens)
        {
            Contract.Requires(tokens.Count == 9, "Expected exactly 8 tokens but got " + tokens.Count + " of them.");
            int age = Convert.ToInt32(tokens[7]);
            DateTime dateCreated = Convert.ToDateTime(tokens[8]);

            this.Init(
                userName : tokens[0],
                password: tokens[1],
                email: tokens[2],
                facebook: tokens[3],
                twitter: tokens[4],
                firstName: tokens[5],
                lastName: tokens[6],
                age: age,
                dateCreated: dateCreated);
        }

        public User(
            string username,
            string password,
            string email,
            string facebook,
            string twitter,
            string firstname,
            string lastname,
            int age,
            DateTime dateCreated)
        {
            this.Init(username, password, email, facebook, twitter, firstname, lastname, age, dateCreated);
        }

        public string UserName
        {
            get;
            private set;
        }

        public string PassWord
        {
            get;
            private set;
        }

        public string Email
        {
            get;
            private set;
        }

        public string FirstName
        {
            get;
            private set;
        }

        public string LastName
        {
            get;
            private set;
        }

        public string Twitter
        {
            get;
            private set;
        }

        public string FaceBook
        {
            get;
            private set;
        }

        public int Age
        {
            get;
            private set;
        }

        public DateTime DateCreated
        {
            get;
            private set;
        }

        private void Init(
            string userName,
            string password,
            string email,
            string facebook,
            string twitter,
            string firstName,
            string lastName,
            int age,
            DateTime dateCreated)
        {
            this.UserName = userName;
            this.PassWord = password;
            this.Email = email;
            this.FirstName = firstName;
            this.LastName = lastName;
            this.Age = age;
            this.DateCreated = dateCreated;
            this.Twitter = twitter;
            this.FaceBook = facebook;

            this.AddToTextFile();
        }

        /***** These methods are not great:
        A) It is not safe to send plain-text passwords over the wire as well as well as store them as such. You would want to compare a very well crafted and tested "hash code"/checksum.
        B) This has O(n) run time. Dictionary or a set would be a better data structure. You probably want a dictionary that maps a user name to a pair `[userName, hash]` and a dictionary that maps `hash` to the same pair. Maybe you will need a database, but a linear list is not as great.

        // Unused?
        public static bool CheckUsername(string username, IList<User> users)
        {
            return users.Any(user => user.UserName.Equals(username, StringComparison.Ordinal)); // Case-sensitive?
        }

        // Unused? Not the best method name - CheckCredentials perhaps?
        public static bool CheckPassword(string username, string password, List<User> users)
        {
            return users.Any(user => user.UserName.Equals(username, StringComparison.Ordinal)  // Case-sensitive?
                && user.PassWord.Equals(password, StringComparison.Ordinal));
        }
        */

        private void AddToTextFile()
        {
            using (var sw = new StreamWriter(filePath))
            {
                sw.WriteLine(
                    "{0} {1} {2} {3} {4} {5} {6} {7} {8}",
                    this.UserName,
                    this.PassWord,
                    this.FirstName,
                    this.LastName,
                    this.Email,
                    this.FaceBook,
                    this.Twitter,
                    this.Age,
                    this.DateCreated);
            }
        }
    }
}


Python list comparison: Can this code run faster? The idea is to apply some function on each element in each list, then compare two lists by the value returned by the function.

My current solution works but is not fast enough. running "python -m cProfile" gives sth. like:

  ncalls    tottime  percall  cumtime  percall filename:lineno(function)
  2412505   13.335   0.000    23.633   0.000   common.py:38(<genexpr>)
  285000    5.434    0.000    29.067   0.000   common.py:37(to_dict)
  142500    3.392    0.000    35.948   0.000   common.py:3(compare_lists)

Here is my code, I would like to know how to optimize it to run faster.

import itertools

def compare_lists(li1, li2, value_func1=None, value_func2=None):
    """ Compare *li1* and *li2*, return the results as a list in the following form:

    [[data seen in both lists], [data only seen in li1], [data only seen in li2]]

    and [data seen in both lists] contains 2 tuple: [(actual items in li1), (actual items in li2)]

    * *value_func1* callback function to li1, applied to each item in the list, returning the **logical** value for comparison
    * *value_func2* callback function to li2, similarly

    If not supplied, lists will be compared as it is.

    Usage::

        >>> compare_lists([1, 2, 3], [1, 3, 5])
        >>> ([(1, 3), (1, 3)], [2], [5])

    Or with callback functions specified::

        >>> f = lambda x: x['v']
        >>>
        >>> li1 = [{'v': 1}, {'v': 2}, {'v': 3}]
        >>> li2 = [1, 3, 5]
        >>>
        >>> compare_lists(li1, li2, value_func1=f)
        >>> ([({'v': 1}, {'v': 3}), (1, 3)], [{'v': 2}], [5])

    """

    if not value_func1:
        value_func1 = (lambda x:x)
    if not value_func2:
        value_func2 = (lambda x:x)

    def to_dict(li, vfunc):
        return dict((k, list(g)) for k, g in itertools.groupby(li, vfunc))

    def flatten(li):
        return reduce(list.__add__, li) if li else []

    d1 = to_dict(li1, value_func1)
    d2 = to_dict(li2, value_func2)

    if d1 == d2:
        return (li1, li2), [], []

    k1 = set(d1.keys())
    k2 = set(d2.keys())

    elems_left  = flatten([d1[k] for k in k1 - k2])
    elems_right = flatten([d2[k] for k in k2 - k1])

    common_keys = k1 & k2
    elems_both  = flatten([d1[k] for k in common_keys]), flatten([d2[k] for k in common_keys])

    return elems_both, elems_left, elems_right

Edit:

zeekay suggests using set, which is also what I was doing, except that I make a dict for each list first, then compare the keys using set, finally return the original elements using the dict. I realized that the speed actually depends on which one will take more time -- the callback function, or the groupby. In my case, the possible callback functions are mostly dot access on objects, and the length of lists can be large causing groupby on lists takes more time.

In the improved version each callback function is executed more than once on every single element, which I considered is a waste and has been trying to avoid in the first place, but it's still much faster than my original approach, and much simpler.

def compare_lists(li1, li2, vf1=None, vf2=None):
    l1 = map(vf1, li1) if vf1 else li1
    l2 = map(vf2, li2) if vf2 else li2

    s1, s2 = set(l1), set(l2)
    both, left, right = s1 & s2, s1 - s2, s2 - s1

    orig_both = list((x for x in li1 if vf1(x) in both) if vf1 else both), list((x for x in li2 if vf2(x) in both) if vf2 else both)
    orig_left = list((x for x in li1 if vf1(x) in left) if vf1 else left)
    orig_right = list((x for x in li2 if vf2(x) in right) if vf2 else right)

    return orig_both, orig_left, orig_right

So it seems you primarily want to use your callback functions to pull the value to compare out of an object, if you don't mind simplifying how compared items are returned, a faster and simpler approach would be:

def simple_compare(li1, li2, value_func1=None, value_func2=None):
        s1, s2 = set(map(value_func1, li1)), set(map(value_func2, li2))
        common = s1.intersection(s2)
        s1_diff, s2_diff = s1.difference(s2), s2.difference(s1)
        return common, s1_diff, s2_diff

>>> simple_compare(li1, li2, value_func1=f)
<<< (set([1, 3]), set([2]), set([5]))

>>> compare_lists(li1, li2, value_func1=f)
<<< (([{'v': 1}, {'v': 3}], [1, 3]), [{'v': 2}], [5])

Which depending on actual use case, might be something you could live with. It's definitely a lot faster:

>>> timeit x = simple_compare(xrange(10000), xrange(10000))
100 loops, best of 3: 2.3 ms per loop

>>> timeit x = compare_lists(xrange(10000), xrange(10000))
10 loops, best of 3: 53.1 ms per loop


Reimplementation of Laravel's config system I am trying to create a config class identical to Laravel's but wanted to do it myself, to test my knowledge on the PHP language, can anyone help me improve it?

Each config file is located in the /app path and can be named whatever you want .php.

You can then get your config values by doing
Config::get('file.array_key')

So lets say I had a file called app.php and an array with a config key I wanted to get called 'author' or something I could simply do Config::get('app.author') I could even dive deeper and go for a key of a key by doing Config::get('app.author.email')

If I wanted to go even deeper, what would be a better way other than just doing an if statement for each index? checking the length etc?

Here is my code:

<?php

namespace PhpTestApp\App\Library\Config;

class Config
{
    static private $configFiles = [];

    public static function loadConfig() {
        $files = scandir(AP . '/app/config/');

        foreach(glob(AP . '/app/config/*.php') as $file) {
            self::$configFiles[explode('.', basename($file))[0]] = include(AP . '/app/config/' . basename($file));
        }
    }

    public function get($getString) {
        $parts = explode('.', $getString);
        $countDot = count($parts);

        if ($countDot == 2) {
            return array_key_exists($parts[0], self::$configFiles) ?
            (array_key_exists($parts[1], self::$configFiles[$parts[0]]) ?
            self::$configFiles[$parts[0]][$parts[1]] : '...')
            : '';
        }
        else if ($countDot == 1) {
            return ''; // just a file?
        }
        else {
            return '';
        }
    }
}

Where is AP defined?  Should loadConfig just get passed a directory path as parameter rather than relying on some global constant and hard-coded path?

I would consider using SplDirectoryIterator and SplFileInfo functionality to load your config files, as this should allow to do this in a much more robust way in comparison to glob().  You could (and probably should), verify that the config directory and all files are readable to the application and throw an exception to bail out of the process if you can't load config properly.

Right now you are kind of "soft" loading the config.  include(), for example, will not halt execution if the file could not be included.  I would think execution should halt with exception thrown so that your application cannot even run if the dependencies aren't met.

Shouldn't get() be static, since that is the only context in which it makes sense (there is no instantiation of this config).

$configFiles as property name seems odd.  This is you actual configuration right?

I guess I am not a big fan of deriving some configuration "keys" from file name and some from the actual data structures in the file.  This seems a bit obfuscated to me.  Is it really that hard to put top level key in the file?

i.e. file like:

return [
    'toplevelkey' => [
        'secondLevel' => [
            'thirdLevel' => [
                ...
            ]
        ]
];

and in your loader:

self::config = array_merge(self::config, require $filepath);

This also could conceivably allow for configuration overrides from config in one file with config from other (this may or may not be desirable for you).  You would likely need to manage file load order in some manner for this, something you may not be able to do effectively with just using glob().

Consider adding helper method with get() to decouple key retrieval logic into it's own (testable) method with appropriate exception triggering to make your class less fragile to bad input. After all, if some portion of your application is expecting some config to be available and it is not what is expected, you should be failing loudly, because your dependency is not being met.

That might look like:

public static function get($keyString) {
    $keys = explode('.', $keyString);
    return self::findConfig($keys, self::config);
}

protected static function findConfig(array $keys, array $haystack, $keyString = '') {
    $key = array_shift($keys);
    $keyString = ltrim($keyString . '.' . $key, '.');
    if (!isset($haystack[$key]) {
        throw new OutOfRangeException("Config key '{$keyString}' is missing");
    }
    if(count($keys) === 0) {
        return $haystack[$key];
    }
    if(!is_array($haystack[$key]) {
        throw new UnexpectedValueException("Expected array value at key '{$keyString}'");
    }
    return self::findConfig($keys, $haystack[$key], $keyString);
}

This approach to use recursion to find a key in your config also unbounds you from your current hard-coded limit of only having 3 key segments.

Consider providing better validation for parameters passed to public methods.  For example, validate get() input is non-zero length string.  Particularly with something as critical to application operation as configuration, you want to take reasonable measures to make sure the class and its data are set up in an appropriate state and that other areas in application are interacting with it in a proper manner.


Permutation of n lists I have code which gives permutations for 10 lists. The code works fine for small number of lists with small number of values.

Result : It should return all possible permutations. I have to store the permutations instead of directly working on it and discarding.

As soon as I try for 10 lists, it errors out with OOM : heap space.

Can one of you suggest optimizations for this piece of code?

public static <T> Collection<List<T>> permutations(List<Collection<T>> collections) {
    if (collections == null || collections.isEmpty()) {
        return Collections.emptyList();
    } else {
        Collection<List<T>> result = Lists.newLinkedList();
        findPermutations(collections, result, 0, new LinkedList<T>());
        return result;
    }
}

private static <T> void findPermutations(List<Collection<T>> inputList, Collection<List<T>> outputList, int d, List<T> tempList) {
    if (d == inputList.size()) {
        outputList.add(tempList);
        return;
    }

    Collection<T> currentCollection = inputList.get(d);
    for (T currElement : currentCollection) {
        List<T> copy = Lists.newLinkedList(tempList);
        copy.add(currElement);
        findPermutations(inputList, outputList, d + 1, copy);
    }
}

@SuppressWarnings("unchecked")
public static <T> void main(String[] args) {

    List<Integer> list1 = Lists.newArrayList(1, 2, 3, 4, 5, 6, 7, 8, 9, 0);
    List<String> list2 = Lists.newArrayList("a", "s", "d", "f", "g", "h", "j", "k", "l", "m", "n", "b");
    List<Boolean> list3 = Lists.newArrayList(true, false, true, true, true);
    List<Integer> list4 = Lists.newArrayList(1, 2, 3, 4, 5, 6, 7, 8, 9, 0);
    List<String> list5 = Lists.newArrayList("a", "s", "d", "f", "g", "h", "j", "k", "l", "m", "n", "b");
    List<Boolean> list6 = Lists.newArrayList(true, false, true, true, true);
    List<Integer> list7 = Lists.newArrayList(1, 2, 3, 4, 5, 6, 7, 8, 9, 0);
    List<String> list8 = Lists.newArrayList("a", "s", "d", "f", "g", "h", "j", "k", "l", "m", "n", "b");
    List<Boolean> list9 = Lists.newArrayList(true, false, true, true, true);
    List<Boolean> list10 = Lists.newArrayList(true, false, true, true, true);

    long currentTime1 = System.currentTimeMillis();

    List<Collection<T>> finalList = Lists.newArrayList();
    finalList.add((Collection<T>) list2);
    finalList.add((Collection<T>) list1);
    finalList.add((Collection<T>) list3);
    finalList.add((Collection<T>) list4);
    finalList.add((Collection<T>) list5);
    finalList.add((Collection<T>) list6);
    finalList.add((Collection<T>) list7);
    finalList.add((Collection<T>) list8);
    finalList.add((Collection<T>) list9);
    finalList.add((Collection<T>) list10);

    Collection<List<T>> resultList = permutations(finalList);

    long currentTime2 = System.currentTimeMillis();

    long timeTaken = (currentTime2 - currentTime1);

    System.out.println("result is : " + resultList + "\n" + "size is : " + resultList.size() + "\n" + "time taken is : " + timeTaken);

}

When writing programs it is often important to do some mental math before you start. In Math class at school they would often suggest that you estimate what the answer will be to a problem before you calculate it "for real".

Here, what you have, is a recursive function. Each time the function is called, it loops over the current-list's members creates a new LinkedList, adds the member and then recurses.

The algorithm is actually quite clean, and readable.... but, do the math:

For a list of 10 members, you create 10 "sub" lists. For each of those sub lists, you recurse, and create 10 more..... so, taking your example of 10 lists of 5 members, how big will your results be (how many lists....)?

5 * 5 * 5 * 5 * 5 * 5 * 5 * 5 * 5 * 5

9765625, but let's call that 10 million

Now, each list is a LinkedList which creates a Node instance for each member, so, each LinkedList has 6 instances created to store references to the members (and the list itself), so, let's just say that each instance is 32 bytes (which is about right - by the way).... that's

10Meg * 6 * 32

That's 1.92 Gig

Now, you take in to account various other things, like the fact that you are storing each list in another list, which will need 10 million members of 32-bytes.... or 320 meg, you are past the 2Gig mark.

So, your solution will use "more than" 2 Gig for your 10-lists of 5-members.

How can you support 10-by-10 solutions? (10 billion permutations - 1000 times more than 10-by-5)..... I don't believe you can....

What you can do though, is be more memory efficient with your data structures....

Using an ArrayList instead of a Linked List will reduce your per-node memory from 32bytes-per-node to just 8 or so.....

Using an iterator/push mechanism will remove the need for the list entirely, and will not need to store anything at all.....

Otherwise, frankly, you're out of luck.... there's just no way that you can create a scalable system that has a size-complexity of \$O(m^n)\$ where m is the number of elements in each list, and n is the number of lists....

For example, in your code, you can remove the results list entirely, and then simply replace the lines:

if (d == inputList.size()) {
    outputList.add(tempList);
    return;
}

with:

if (d == inputList.size()) {
    System.out.println(tempList);
    return;
}

And suddenly you will no longer be constrained by memory (though it will be pretty damn slow still).


Lock implementation using atomics I was recently given an interview and was asked to write my own lock implementation using atomics, but not necessary reentrant.

I didn't answer the question, but at home, I wrote this code. Please asses my implementation and suggest better solutions.

public class MyLock {

    AtomicBoolean locked = new AtomicBoolean(false);

    ThreadLocal<Boolean> threadLocal = new ThreadLocal();

    public void lock() {
        while (true) {
            if (locked.compareAndSet(false, true)) {
                threadLocal.set(true);
                return;
            }
        }
    }

    public void unLock() {
        if (threadLocal.get()) {
            locked.compareAndSet(true, false);
            threadLocal.set(false);
        }
    }

}

class Main {

    public static void main(String[] args) {
        MyLock myLock = new MyLock();
        for (int i=0;i<10;i++)
            new MyThread(myLock).start();
    }
}

class MyThread extends Thread {

    final MyLock myLock;

    MyThread(MyLock myLock) {
        this.myLock = myLock;
    }

    @Override public void run() {
        myLock.lock();
        System.out.println("start - " + Thread.currentThread().getName());
        try {
            Thread.sleep(5000);
        } catch (InterruptedException e) {
            e.printStackTrace();  //To change body of catch statement use File | Settings | File Templates.
        }
        System.out.println("finish - " + Thread.currentThread().getName());
        myLock.unLock();
    }
}

I think the main problem of your implementation is that lock() performs Busy waiting. This will cause a thread that is waiting to get the lock to repeatedly waste CPU-cycles by polling (multiple times in one time slot) whether the lock has been released.

Easiest fix for that is to perform a call to Thread.yield() after trying to acquire the lock. This will cause the thread to exit its time slot and therefore let the other threads do their work.

public void lock() {
    while (true) {
        if (locked.compareAndSet(false, true)) {
            threadLocal.set(true);
            return;
        }
        Thread.yield();
    }
}

Oh and I would rename the method unLock() to unlock() since it is an existing english word.


Simple library for an undocumented archive format I'm writing a little library for some undocumented file formats. The original program uses a path string to find files in an archive and this function tries to do the same.
The archive contains a magic string, an offset into the file list, the data in the files and the file list itself. A entry in the filelist consists of a path (256 bytes long, 0-padded) and three numbers, the second one being the offset from the archive start to the entry file. The entries are sorted alphabetically respecting case; here are some mockup paths to demonstrate the ordering:
Entry/Graphics/Sprite.cgf
Entry/Sound/Ding.wav
Quit/Graphics/exit.cgf
_RAW8123

Here are the relevant utility functions and defines. These are defined in Utils.h and Utils.c, I've put them here together for context.
#define BYTESZE sizeof(char)

typedef unsigned int uint;
typedef unsigned char byte;

uint readU4L(FILE* file) {
    byte buf[4];
    fread(buf, BYTESZE, 4, file);
    return buf[3] << 24 | buf[2] << 16 | buf[1] << 8 | buf[0];
}

Finally, here's the "library":
// ====== BIGfile.h
#define BIG_MAGIC_LEN 7

#define BIG_SEEK_SUCC 1
#define BIG_SEEK_FAIL 0

typedef struct BIGFile {
    char magic[BIG_MAGIC_LEN];
    uint offset;
    FILE* read;

} BIGFile;

BIGFile* load_BIGFile(char* fpath);

int seekToEntry(BIGFile* bigfile, char* path);

// ====== BIGFile.c
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include "Utils.h"
#include "BIGFile.h"

BIGFile* load_BIGFile(char* fpath) {
    BIGFile* bigfile = malloc(sizeof(BIGFile));

    fopen_s(&(bigfile->read), fpath, "rb");

    if (!bigfile->read) {
        printf("Can't open file.\n");
        return 0;
    }

    fread(bigfile->magic, BYTESZE, BIG_MAGIC_LEN, bigfile->read);
    bigfile->offset = readU4L(bigfile->read);
    return bigfile;
}

int seekToEntry(BIGFile* bigfile, char* path) {

    fseek(bigfile->read, bigfile->offset, SEEK_SET);

    byte buf[256];

    int pos = 0;

    // while not EOF, read string part
    while (fread(buf, BYTESZE, 256, bigfile->read)) {

        // seek over entry data
        fseek(bigfile->read, 12, SEEK_CUR);

        // loop until given path != read path
        while (path[pos]==buf[pos]) {
            pos++;

            // path has ended, so buf == path
            if (!path[pos]) {

                // seek back into entry data
                fseek(bigfile->read, -8, SEEK_CUR);

                // read unsigned little endian 4-byte int
                uint res = readU4L(bigfile->read);

                // seek there and return success
                fseek(bigfile->read, res, SEEK_SET);
                //printf("found at %d\n", res);
                return BIG_SEEK_SUCC;
            }
        }
        // path char is smaller than buf char
        // we seeked past where the entry could bem return failure
        if (path[pos]<buf[pos]) {
            return BIG_SEEK_FAIL;
        }
    }
    // EOF, return failure
    return BIG_SEEK_FAIL;
}

I'd like some feedback on general style, as well as on the seekToEntry function, perhaps this can be improved somehow? Thanks in advance!

Overall, your coding style is pretty clean, but here are some improvement suggestions to make the code more robust and portable, as well as safer:
Use fixed sized integers
In C, sizeof(char) is guaranteed to be 1 by the standard, but sizeof(unsigned int) is not guaranteed to be 4.
Fortunately, C99 introduces fixed size integers such as uint32_t that is guaranteed to be 32 bit (4 bytes) defined in stdint.h.
Since you are using non standard fopen_s I assume you are not required to stick to ANSI C.
Add error checking
In the code you posted, you never check the return value of your file read operations or memory allocation operations.
You also never check the sanity of the data you read.
For a library, robust error handling is important, so you should always check if read operations success, and if data such as entry sizes or offsets are reasonable - match expected size / do not go past end of file.
On error, you should return one of several error codes you define for you library and list in documentation.
Reduce disk operations
You will probably not feel this in your testing, both due to modern hardware, small file size, and multiple caches on different levels, but your seekToEntry function is inefficient due to many back and forward disk operations (both seek and read).
There are two approaches here, which you can choose depending on how you want to trade memory for speed, and how would your library be used:

Preload the entire "catalog" of file entries in to array of structures in memory and search that when you need to find entry position in file.
Since your entries are guaranteed to be sorted, you may want to implement a more efficient search, maybe arrange them in a tree by prefix.

Since every entry has fixed size, read at least the current entry completely in to a structure.
If you don't want to load them all to memory in advance, at least load all data (name, size, offset) for each entry as you iterate, and save your self a bunch of seek operations.

Either approach will also give you the opportunity to create a documented structure for "entry" of this archive file, which will be good for you and your library users.
Create a name space
Since this is a library, it may be used in a big project with other libraries or many functions defined in the project it self.
You want to avoid name collision for your functions, structures, and defines.
Though C does not support special syntax for namespces, one convention for libraries is to use a fixed and consistent prefix to names followed by an underscore.
If your library is for handling "Big File Archive" you could prefix all your names with BFA_ like so: BFA_loadFile, BFA_seekToEntry.
Avoid non-standard functions
While MS may claim fopen_s is more secure, it is also non-standard, so if you or one of your library users ever switches compiler or OS, your library will no longer work.
You should use C standard fopen instead and just add proper error checking as specified above.
Also, there is no reason to reimplement strncmp as you do with your loops.
Unlike strcmp which is not safe if the string is not guaranteed to be null terminated, strncmp is perfectly safe for your use case where maximum length is known in advance.
Use consistent interface
It is hard to tell with only two functions, but you seem to have some difference in how your library functions work:
One returns an "object" (the BIGFile structure), another returns error / success codes instead.
While this is technically valid approach, it would be more consistent and comfortable for the user to have a unified interface where all functions would return the same thing - error codes.
If a function must also return a value for the user, such as a pointer to a structure, it should return it using one of the parameters.
Your open function could look like this:
/**
 * Opens an archive file and loads its header
 *
 * @param fpath   path to the archive file
 * @param header  place to return loaded header data if the file is successfully loaded (structure will be allocated by the function)
 * @return success / error status code
 */
int BFA_loadFile(char *fpath, BFA_FileHeader **header) {
    ...
}

Here, I also included one common method of documenting functions in libraries which can be used with automated tools such as doxygen to produce documentation files in HTML and other formats.


Enclosing Circle Problem implementation in Haskell I implemented the algorithm by Pr. Chrystal described here in Haskell so could someone please tell me: Do i have implemented this algorithm correctly?
Initial calling of findPoints takes the first and second point of convex hull and the list of points on convex hull.

My second question is: I am trying to solve the problem on sphere online judge [link of problem in code below, because I can not post more than 2 links] using this algorithm but I'm getting the wrong answer.
The code for the problems is on ideone. Why am I getting the wrong answer.

--http://www.spoj.pl/problems/QCJ4/
findAngle :: ( Num a , Ord a , Floating a ) => Point a -> Point a -> Point  a  -> ( Point a , Point a , Point  a , a )
findAngle u@(P x0 y0 ) v@(P x1 y1 ) t@(P x2 y2)
| u == t || v == t = ( u , v , t , 10 * pi )  -- two points are same so set the angle more than pi
| otherwise =  ( u , v, t , ang ) where
        ang = acos ( ( b + c - a ) / ( 2 * sb * sc ) ) where
        b = ( x0 - x2 ) ^ 2 + ( y0 - y2 ) ^ 2
        c = ( x1 - x2 ) ^ 2 + ( y1 - y2 ) ^ 2
        a = ( x0 - x1 ) ^ 2 + ( y0 - y1 ) ^ 2
        sb = sqrt b
        sc = sqrt c

findPoints :: ( Num a , Ord a , Floating a ) => Point a -> Point a -> [ Point  a ] -> ( Point a , Point a , Point a , a )
findPoints u v xs
  | 2 * theta >= pi   =     ( a , b , t , theta )
  | and [ 2 * alpha <= pi , 2 * beta <= pi ]   = ( a , b , t , theta )
  | otherwise = if 2 * alpha > pi then findPoints v t xs else findPoints u t xs
     where
       ( a , b , t , theta ) = minimumBy ( \(_,_,_, t1 ) ( _ , _ , _ ,t2 ) -> compare  t1 t2 ) . map ( findAngle u v )  $ xs
       ( _ , _ , _ , alpha ) = findAngle v t u  --angle between v u t angle subtended at u by v t
       ( _ , _ , _ , beta ) = findAngle u t v   -- angle between u v t angle subtended at v by  u t

As per request by Mihai , here is the accepted code of spoj problem . The only error in previous code was " The minimal enclosing circle is determined by the diametric circle of S ".  Instead of finding a circle passing through two points , i was calculating circle from three points.

import Data.List
import qualified Data.Sequence as DS
import Text.Printf
import Data.Function ( on )

data Point a = P a a deriving ( Show , Ord , Eq )
data Turn = S | L | R deriving ( Show , Eq , Ord , Enum  ) -- straight left right

--start of convex hull  http://en.wikipedia.org/wiki/Graham_scan
{--
compPoint :: ( Num  a , Ord a ) => Point a -> Point a -> Ordering
compPoint ( P x1 y1 ) ( P x2 y2 )
  | compare x1 x2 == EQ = compare y1 y2
  | otherwise = compare x1 x2

findMinx :: ( Num a , Ord a ) => [ Point a ] -> [ Point a ]
findMinx xs = sortBy ( \x  y  -> compPoint  x y  ) xs

compAngle ::(Num a , Ord a ) => Point a -> Point a -> Point a -> Ordering
compAngle ( P x1 y1 ) ( P x2 y2 ) ( P x0 y0 ) = compare ( (  y1 - y0 ) * ( x2 - x0 )  ) ( ( y2 - y0) * ( x1 - x0 ) )

sortByangle :: ( Num a , Ord a ) => [ Point a ] -> [ Point a ]
sortByangle (z:xs) = z : sortBy ( \x y -> compAngle x y z ) xs

convexHull ::( Num a , Ord a )  => [ Point a ] -> [ Point a ]
convexHull xs = reverse . findHull [y,x]  $ ys where
    (x:y:ys) = sortByangle.findMinx $ xs

findTurn :: ( Num a , Ord a , Eq a ) => Point a -> Point a -> Point a -> Turn
findTurn ( P x0 y0 ) ( P x1 y1 ) ( P x2 y2 )
  | ( y1 - y0 ) * ( x2- x0 ) < ( y2 - y0 ) * ( x1 - x0 ) = L
  | ( y1 - y0 ) * ( x2- x0 ) == ( y2 - y0 ) * ( x1 - x0 ) = S
  | otherwise = R

findHull :: ( Num a , Ord a  )  => [ Point a ] ->   [ Point a ] -> [ Point a ]
findHull [x]  ( z : ys )  = findHull [ z , x ]  ys  --incase of second point  on line from x to z
findHull xs  [] = xs
findHull ( y : x : xs )  ( z:ys )
   | findTurn x y z == R = findHull (  x : xs )   ( z:ys )
   | findTurn x y z == S = findHull (  x : xs )   ( z:ys )
   | otherwise = findHull ( z : y : x : xs  )   ys
--}

--start of monotone hull give .04 sec lead over graham scan

compPoint :: ( Num  a , Ord a ) => Point a -> Point a -> Ordering
compPoint ( P x1 y1 ) ( P x2 y2 )
  | compare x1 x2 == EQ = compare y1 y2
  | otherwise = compare x1 x2

sortPoint :: ( Num a , Ord a ) => [ Point a ] -> [ Point a ]
sortPoint xs = sortBy ( \ x y -> compPoint x y ) xs

findTurn :: ( Num a , Ord a , Eq a ) => Point a -> Point a -> Point a -> Turn
findTurn ( P x0 y0 ) ( P x1 y1 ) ( P x2 y2 )
 | ( y1 - y0 ) * ( x2- x0 ) < ( y2 - y0 ) * ( x1 - x0 ) = L
 | ( y1 - y0 ) * ( x2- x0 ) == ( y2 - y0 ) * ( x1 - x0 ) = S
 | otherwise = R

hullComputation :: ( Num a , Ord a ) => [ Point a ] -> [ Point a ] -> [ Point a ]
hullComputation [x] ( z:ys ) = hullComputation [z,x] ys
hullComputation xs [] = xs
hullComputation  ( y : x : xs ) ( z : ys )
  |  findTurn x y z == R = hullComputation ( x:xs ) ( z : ys )
  |  findTurn x y z == S = hullComputation ( x:xs ) ( z : ys )
  |  otherwise = hullComputation ( z : y : x : xs ) ys

convexHull :: ( Num a , Ord a ) => [ Point a ] -> [ Point a ]
convexHull xs = final where
    txs = sortPoint xs
    ( x : y : ys  ) = txs
        lhull = hullComputation [y,x] ys
    ( x': y' : xs' ) = reverse txs
    uhull = hullComputation [ y' , x' ] xs'
    final = ( init lhull ) ++ ( init uhull )

--end of convex hull
--start of finding point algorithm http://www.personal.kent.edu/~rmuhamma/Compgeometry/MyCG/CG-Applets/Center/centercli.htm  Applet’s Algorithm

findAngle :: ( Num a , Ord a , Floating a ) => Point a -> Point a -> Point  a  -> ( Point a , Point a , Point  a , a )
findAngle u@(P x0 y0 ) v@(P x1 y1 ) t@(P x2 y2)
    | u == t || v == t = ( u , v , t , 10 * pi )  -- two points are same so set the angle more than pi
    | otherwise =  ( u , v, t , ang ) where
        ang = acos $  ( b + c - a ) / ( 2.0 * ( sqrt $ b * c ) )  where
        sqrDist ( P x y ) ( P x' y' ) = ( x - x' ) ^ 2 + ( y - y' ) ^ 2
        [ b , c , a ] = map ( uncurry sqrDist ) [ ( u , t ) , ( v , t ) , ( u , v ) ]

 findPoints :: ( Num a , Ord a , Floating a ) => Point a -> Point a -> [ Point  a ] -> ( Point a  , a )
 findPoints u v xs
  | 2 * theta >= pi   =  circle2Points a b  --( a , b , t , theta )
  | and [ 2 * alpha <= pi , 2 * beta <= pi ]   = circle3Points a b t --( a , b , t , theta )
  | otherwise = if 2 * alpha > pi then findPoints v t xs else findPoints u t xs
     where
    ( a , b , t , theta ) = minimumBy ( on compare fn  ) . map ( findAngle u v )  $ xs
    fn ( _ , _ , _ , t ) = t
        ( _ , _ , _ , alpha ) = findAngle v t u  --angle between v u t angle subtended at u by v t
    ( _ , _ , _ , beta ) = findAngle u t v   -- angle between u v t angle subtended at v by  u t

--end of finding three points algorithm
--find the circle through three points http://paulbourke.net/geometry/circlefrom3/

circle2Points :: ( Num a , Ord a , Floating a ) => Point a -> Point a -> ( Point a , a )
circle2Points ( P x0 y0 ) ( P x1 y1 ) = ( P x y , r ) where
    x = ( x0 + x1 ) / 2.0
    y = ( y0 + y1 ) / 2.0
    r = sqrt $ ( x0 - x1 ) ^ 2 + ( y0 - y1 ) ^ 2

circle3Points :: ( Num a , Ord a , Floating a ) => Point a -> Point a -> Point a -> (  Point a , a ) --( center , radius )
circle3Points u@(P x1 y1 ) v@(P x2 y2 ) t@(P x3 y3 )
    | x2 == x1 = circle3Points u t v
    | x3 == x2 = circle3Points v u t
    | otherwise =  ( P x y , 2 *  r )
      where
        m1 = ( y2 - y1 ) / ( x2 - x1 )
        m2 = ( y3 - y2 ) / ( x3 - x2 )
        x = (  m1 * m2 * ( y1 - y3 )  +   m2 * ( x1 + x2 )  -  m1 * ( x2 + x3 )  ) / ( 2 * ( m2 - m1 ) )
        y = if y2 /= y1
                     then ( ( x1 + x2 -  2 * x  ) / ( 2 * m1 ) ) + ( ( y1 + y2 ) / 2.0 )
              else  ( ( x2 + x3 -  2 * x  ) / ( 2 * m2 ) ) + ( ( y2 + y3 ) / 2.0 )
        r = sqrt $ ( x - x1 ) ^2 + ( y - y1 ) ^ 2

--start of SPOJ code

format::(Num a , Ord a ) => [[a]] -> [Point a]
format xs = map (\[x0 , y0] -> P x0 y0 ) xs

readInt  ::( Num a , Read a ) =>   String -> a
readInt  = read

solve :: ( Num a , Ord a , Floating a ) => [ Point a ] -> (  Point a , a )
solve [ P x0 y0 ] = ( P x0 y0 , 0 ) --in case of one point
solve [ P x0 y0 , P x1 y1 ] = circle2Points ( P x0 y0 ) ( P x1 y1 )   -- in case of two points
solve  xs = findPoints x y  t where
    t@( x : y : ys )  = convexHull xs

final :: ( Num a , Ord a , Floating a ) => ( Point a , a ) -> a
final ( t , w )
    | w == 0 = 0
    | otherwise =  w

main = interact $   ( printf "%.2f\n" :: Double -> String ) . final . solve . convexHull . format . map  ( map readInt . words ) . tail . lines


A simple encryption scheme 2.0 This is a follow up to my previous post.
I've used a BCrypt library to create a key from the passed in password.  I then separated out the salt from the hash.  The encryption is done using the salt concatenated to the hash, but only the hash is returned to the user.  The salt is prepended to the encrypted data.
The key(salt + hash) is used to create a 512 bit hash using SHA3512. Each byte of data is XOR'ed with a byte of the key and a byte of the new hash.  This byte of the hash is replaced by being XOR'ed with the byte of the key.  When the end of the hash is reached, this byte array with all new values is then hashed and used for the next 64 bytes of data.
The Decryption separates out the salt and combines it with the passed in key to create the original hash and continue with the original encryption steps.
using System.Text;
using System.Linq;
using System.Collections.Generic;
using SHA3.Net;
using BCrypt.Net;

class Cipher
{
    public static IEnumerable<byte> Encrypt(string password, IEnumerable<byte> data,out string key)
    {
        if (password == null)
        {
            password = "";
        }
        if (data == null)
        {
            data = new byte[] { 0 };
        }
        var hash = MakeKey(password);
        key = new string(hash, 22, 31);
        string salt = new string(hash, 0, 22);
        return GetBytes(data, key,salt);
    }
    public static IEnumerable<byte> Decrypt(IEnumerable<byte> data, string key)
    {
        if (data == null)
        {
            data = new byte[] { 0 };
        }
        return GetBytes(data, key);
    }
    static IEnumerable<byte> GetBytes(IEnumerable<byte> data, string key, string salt = "")
    {
        int saltLength = 0;
        if(salt == "")
        {
            salt = new string(data.Take(22).Select(x => (char)x).ToArray());
            saltLength = 22;
        }
        else
        {
            foreach(char c in salt)
            {
                yield return (byte)c;
            }
        }
        key = $"{salt}{key}";
        var hash = Sha3.Sha3512().ComputeHash(Encoding.UTF8.GetBytes(key));
        int i = 0;
        foreach (var b in data.Skip(saltLength))
        {
            //modulo 64
            int hashIndex = (int)(((uint)i << 26) >> 26);
            if (i > 0 && hashIndex == 0)
            {
                hash = Sha3.Sha3512().ComputeHash(hash);
            }
            byte offset = (byte)key[i % key.Length];
            var retVal = (byte)(b ^ offset ^ hash[hashIndex]);
            hash[hashIndex] ^= offset;
            ++i;
            yield return retVal;
        }
    }

    static char[] MakeKey(string password) => BCrypt.Net.BCrypt.HashPassword(password).Skip(7).ToArray();
}

Here's what the encrypted data can look like:
{86, 110, 68, 77, 68, 75, 46, 116, 73, 99, 50, 53, 114, 57, 89, 47, 109, 70, 88, 105, 114, 117, 233, 83, 221, 231, 216, 45, 118, 223, 227, 185, 170, 177, 131, 154, 240, 170, 72, 192, 182, 112, 149, 208, 102, 235, 252, 156, 127, 189, 255, 240, 203, 152, 142, 189, 140, 64, 166, 196, 13, 254, 138, 159, 229, 199, 80, 80, 113, 162, 137, 87, 216, 59, 81, 140, 123, 199, 211, 75, 43, 62, 11, 3}

Too fancy, too hard to read. The whole code can be compressed into something like
 BCrypt.Net.BCrypt.HashPassword(salt+password).ToArray()
Take a look at how to use this function here: https://jasonwatmore.com/post/2020/07/16/aspnet-core-3-hash-and-verify-passwords-with-bcrypt
Most importantly: there's no point in using mighty hashing like SHA3 and BCrypt only to use some made-up XOR encryption scheme. Use a standard encryption algorithm from a reputable library. If you don't trust any library or algorithm, then encrypt 2 times with two different algorithms, different keys and maybe using libraries from different vendors.
Beyond that, here are other issues I have with this code:

Is .Skip(7) supposed to somehow remove the salt? You can't "remove" the salt, the string you get from that function is not your input anymore, it's a completely different string (aka hash).

All these IEnumerable aren't efficient. Just use byte[].

there's an attempt to be efficient with bitshifts. Just use % and leave it to the compiler. Given the rest of the code doesn't look like the most efficient implementation (IEnumerable, yield...) it makes no sense to sacrifice readability

to that end - add other comments. It's not clear what GetBytes is supposed to achieve, given its generic name and lack of comments.

if the password is null, empty, or string below a certain length - throw an exception. There's no point in encrypting something with an empty password

name things properly. byte offset is not an offset (that would have been i % key.Length), it's a byte from the key

use constants instead of magic values sprinkled across the code.

get familiarized with common encryption libraries and try to follow their patterns, don't reinvent wheels that need no reinventing


Recursive Transversal (Python) For a programming challenge I was doing, I needed to traverse a list of lists, and while writing the standard 2D traversal function, I thought why not generalise it, so I did. Here's my best effort:

My Code

def rTraverse(lst, f = lambda x: x):        #Traverse a list and any of its (non self referential) sublists recursively.
    for item in lst:
        if isinstance(item, list):
            if "..." in str(item):  #Skip self-referential lists.
                continue
            traverse(item, f)   #Traverse the sublist.
        else:
            f(item)

The way you keep track of lists which you are currently exploring is a bit inefficient. You basically have a stack which contains the first argument of the function for each recursive call. This exact information is also stored on the execution stack of the Python interpreter, so it's duplicated.

A more efficient way would be to abandon recursion and use a cycle instead, since you are already using a stack. That way, you will also save some more space by not duplicating the constant (non-changing) arguments f and seen on the execution stack.

Another, even more significant way to improve efficiency is by observing that the check if lst[idx] not in seen takes linear time, although this can be done in constant time.

1. Removing recursion

We can start with the existing stack of lists and extend it as necessary. Since we are basically doing a dept-first search, upon descent to the deeper level we need to remember the list which we are currently iterating over, as well as the position within that list (so we can later return and continue where we left off). This can be done for example by having two lists, one containing the lists and the other one containing the positions. Or we can have just one list containing 2-tuples of the form (list, position).

2. Faster cyclic reference detection

Instead of searching through the stack, item by item, we can use a set as you suggested in a comment. This will allow us to detect cyclic references in constant time and the whole issue will be settled.

An even better approach would be to combine the position-tracking with fast membership checking: we could have a list of lists (the stack), and a dictionary mapping each list to our position within that list. Then a list will be used as a key in the dictionary if and only if it is in the stack. Note that adding, updating and deleting items in a hashed dictionary are all essentially constant time operations, and so is checking whether a key is present. So we achieve constant time cycle detection, much like we would with a set, but in a more elegant way.

One "problem" here may be that if we have the same list in the stack twice, with two different positions, which position do we put in the dictionary? Or can we somehow put both of them there? The answer is, we don't care, because this will never happen. Since we don't want to visit the same list twice on our path down the tree, the stack will never contain the same list twice. Now the only problem is that Python doesn't allow us to use list objects as keys in dict.

3. Using lists as keys in a dictionary

Since list is not a hashable type, it cannot be used as a key in dict (which is a hashed dictionary). However, the solution is quite simple: we use id(lst) instead of the lst itself as the key. We can do this, since we only care about identity, not equality. As a side note, this is another case in which your program behaves incorrectly. It compares lists with the items in seen based on equality, not identity: if lst[idx] not in seen: …. Consider following code:

>>> a = []
>>> a.append(a)
>>> b = [a]
>>> a == b
True
>>> id(a) == id(b)
False
>>> a in [b]
True

What should happen if you call rTraverse(b)? I suppose you would want to traverse both a and b.

Refactored code

The code contains a (very basic) implementation of a custom data structure which facilitates the above-proposed approach. This data structure is then employed in the modified traverse function.

class CustomStack(list):
  """
  Our custom data structure to facilitate position-tracking
  and provide constant time membership checking for cycle detection.

  This data structure serves as the stack, hence inherits from `list`.
  It has an `item_pos` attribute which serves as the dictionary.
  """

  def __init__(self, *args, **kwargs):
    super().__init__((item for item, pos in args), **kwargs)
    self.item_pos = {id(item): pos for item, pos in args}

  def append(self, item, pos):
    if item not in self:
      super().append(item)
      self.item_pos[id(item)] = pos

  def pop(self):
    del self.item_pos[id(self.top)]
    return super().pop()

  def __contains__(self, item):
    return id(item) in self.item_pos

  def __getitem__(self, idx):
    item = super().__getitem__(idx)
    pos = self.item_pos[id(item)]
    return item, pos

  @property
  def top(self):
    return super().__getitem__(-1)

  def setpos(self, pos):
    """
    This functions allows us to update the saved position within the list
    which is being currently explored (= is last in the stack).
    """
    self.item_pos[id(self.top)] = pos

def traverse(lst, f = lambda x: x):

  stk = CustomStack((lst, 0))  # initial position in the list is 0

  while stk:
    curr_list, curr_pos = stk[-1]  # don't pop the top of the stack yet

    for idx in range(curr_pos, len(curr_list)):  # continue from the saved position

      item = curr_list[idx]

      if isinstance(item, list) and item not in stk:
        stk.setpos(idx+1)    # update the current position to restore it later
        stk.append(item, 0)  # push the new list onto the stack
        break  # we are going depth-first into the new, deeper list
      else:
        curr_list[idx] = f(item)

    else:
      # we did not break out of the `for` loop. that means we're done
      # with this list and we are returning to the previous level.
      stk.pop()

Recursive version

Since the code got a bit too long for such a simple task, we can try going back to using recursion, and use a set as you proposed. Similarly to the case with a dictionary, we have to use id(lst) instead of lst, because items in a set are hashed. Considering AJNeufeld's remarks on thread and exception safety, we can use a dummy default value for the seen parameter, and if we see that seen has this dummy value (as opposed to a "real" value, eg. a set instance), then we create a set instance as a local variable on the function's stack frame. Upon recursively calling the function, we pass a reference to this local variable, therefore no thread safety or exception safety issues ensue.

def traverse_simple(lst, f=lambda x: x, seen=None):

  if seen is None:
    seen = set()

  seen.add(id(lst))

  for i, item in enumerate(lst):

    if isinstance(item, list) and id(item) not in seen:
      traverse_simple(item, f, seen)
    else:
      lst[i] = f(item)

  seen.remove(id(lst))

By using id(lst), we are able to not only use a set and therefore detect cyclic references in constant time; we are also able to compare lists by identity instead of equality. This means the function does not fail on inputs on which your original function does fail, such as this:

>>> a = []
>>> b = [a]
>>> a.append(b)


Converting between different units of temperature The task is to convert a temperature in a list of Fahrenheit, Celsius, or Kelvin to the two other temperatures in the list.

Here is my code:

/*
 * #define LANG C
 */

#include <stdio.h> /** standard io **/
#include <stdlib.h> /** exit(), atof and the lot **/
#include <unistd.h> /** needed for getopt() **/

/*** SYMBOIC CONSTANTS ***/
#define VERSION "1.0" /** version string array **/
#define HELP help(argv[0]) /** will insert this instead of help(argv[0]) every time **/
#define FLOATARG atof(optarg) /** atof is kind of vague, so a #define is better **/
#define TOVERH "FAHR\tCELSIUS\tKELVIN\n" /** just so I don't have to type it out ever time **/
#define OPTS "vk:f:c:h" /** I prefer a #define to this -> b:ull:s::hi:t: **/
/** For reference, an option followed by a double colon **/
/**        signifies that it takes no arguments         **/
/**  while an option followed by one colon signifies    **/
/**           that it takes one argument.               **/

static const char *ver = VERSION;

void help(char *s);

int check(float N) { /** check if kelvin temperature is celsius 0 **/
    if(N > 273.1499 && N < 273.15001) { /** I had to set this limit. There is no other way. **/
        return 1;
    }
    else {
        return 0;
    }
}

/*** Fahrenheit ***/
float FK(float F) {
    return(((5.0 / 9.0) * (F - 32.0)) + 273.15);
}
float FC(float F) {
    return((5.0 / 9.0) * (F - 32.0));
}
void printfahr(float F) {
    printf(TOVERH);
    printf("%4.3f\t%5.3f\t%5.3f\n", F, FC(F), FK(F));
}

/** Celsius **/
float CK(float C) {
    return((C + 273.15));
}
float CF(float C) {
    return((C * (9.0 / 5.0) - 32.0));
}
void printc(float C) {
    printf(TOVERH);
    printf("%4.3f\t%5.3f\t%5.3f\n", CF(C), C, CK(C));
}

/*** Kelvin ***/
float KC(float K) {
    return(K - 273.15);
}
float KF(float K) {
    return((K - 273.15) * (9.0 / 5.0) - 32.0);
}
void printk(float K) {
    printf(TOVERH);
    printf("%4.3f\t%5.3f\t%5.3f\n", KF(K), KC(K), K);
}

int main(int argc, char *argv[]) {
    if(argc == 1) {
        HELP;
        exit(argc);
    }
    else {
        char arg = '\0';
        while((arg = getopt(argc, argv, OPTS)) != -1)
        {
            switch(arg) {
                case 'c':
                    if(optarg == NULL)
                    {
                        HELP;
                        exit(1);
                    }
                    else {
                        printc(FLOATARG);
                    }
                break;

                case 'f':
                    if(optarg == NULL)
                    {
                        HELP;
                        exit(1);
                    }
                    else {
                        printfahr(FLOATARG);
                    }
                break;

                case 'h':
                    HELP;
                break;

                case 'k':
                    if(optarg == NULL)
                    {
                        HELP;
                        exit(1);
                    }
                    else {
                        if(check(FLOATARG) != 0)
                        {
                            printf(TOVERH);
                            printf("32.0\t0\t273.15\n");
                        }
                        else {
                            printk(FLOATARG);
                        }
                    }
                break;

                case 'v':
                    printf("%s\n", ver);
                break;

                default:
                    HELP;
                break;
            }
        }
    }
    return 0;
}

void help(char *s) {
    printf("Usage:\t%s [-c Celsius] [-f Fahrenheit] [-h] [-k Kelvin] [-v]\n", s);
}

Numerical consts are double, either add them eg as 5.0f or use double.
Define a constants for 0°C = 273.15K and also for Fahrenheit to Celsius conversation (avoid repeated magic constants 273.15, 9.0 / 5.0, 32.0).
In check the distance from 273.15 is not symmetric, probably a typo.
The name check doesn't imply what is checked, meaningful names not limited to this function would be a recommended.

A modified version could include:

#include <math.h>

const double Celsius2KelvinShift = 273.15;
const double Fahrenheit2KCelsiusShift = 32.0;
const double Fahrenheit2CelsiusFactor = 5.0 / 9.0;

/** check if kelvin temperature is celsius 0 **/
int checkIf0Celsius(double N)
{
    const double tol = 0.001; /** I had to set this limit. There is no other way. **/
    return fabs(N - Celsius2KelvinShift) < tol;
}

/*** Fahrenheit ***/
double Fahrenheit2Kelvin(double F)
{
    return Fahrenheit2CelsiusFactor * (F - Fahrenheit2KCelsiusShift) + Celsius2KelvinShift;
}

double Fahrenheit2Celsius(double F)
{
    return Fahrenheit2CelsiusFactor * (F - Fahrenheit2KCelsiusShift);
}

/** Celsius **/
double Celsius2Kelvin(double C)
{
return C + Celsius2KelvinShift;
}

double Celsius2Fahrenheit(double C)
{
    return C / Fahrenheit2CelsiusFactor - Fahrenheit2KCelsiusShift;
}

/*** Kelvin ***/
double Kelvin2Celsius(double K)
{
    return K - Celsius2KelvinShift;
}

double Kelvin2Fahrenheit(double K) {
    return (K - Celsius2KelvinShift) / Fahrenheit2CelsiusFactor - Fahrenheit2KCelsiusShift;
}


Normalize video filenames and move files to destination directory I have written a powershell script that will scan a directory for any *mkv, *.mp4, *.avi files, and will parse the filename to get Name, Season Number, Episode number. Then will check against destination directory to determine

If Name exists (if not create it)
If season number exists (if not create it)
Once both these checks have been performed move the file to the destination dir

This syntax works for my purpose, but it is slow... I am lower than a beginner when it comes to powershell (is there such a thing?) - And would like any helpful hints or code mods that the more experienced programmers can provide.

$fileDirectory = "C:\Test\"

foreach ($file in Get-ChildItem $fileDirectory | where {($_.extension -eq '.mkv') -or ($_.extension -eq '.avi') -or ($_.extension -eq '.mp4')}){
    #Setting parent dir to check
    $ParentDir = "\\C:\TV Shows\"

    #setting param to split
    $parts =$file.Name -split '\.'

    #capturing variables
    $ShowName = $parts[0].Trim()
    $SeasonNumber = $parts[1].substring(0,3)
    $EpisodeNumber = $parts[1].substring(3,3)

    #Converting numeric season value to text
    switch ($SeasonNumber){
     S01
     {
        $SeasonNumber = "Season 01";
        break
     }
     S02
     {
        $SeasonNumber = "Season 02"
        break
     }
     S03
     {
        $SeasonNumber = "Season 03"
        break
     }
     S04
     {
        $SeasonNumber = "Season 04"
        break
     }
     S05
     {
        $SeasonNumber = "Season 05"
        break
     }
     S06
     {
        $SeasonNumber = "Season 06"
        break
     }
     S07
     {
        $SeasonNumber = "Season 07"
        break
     }
     S08
     {
        $SeasonNumber = "Season 08"
        break
     }
     S09
     {
        $SeasonNumber = "Season 09"
        break
     }
     S10
     {
        $SeasonNumber = "Season 10"
         break
     }
     default {"Something else happened"; break}
     }
    #Setting path variables
    $ShowDir = Join-Path -Path $ParentDir -ChildPath $ShowName
    $SeasonDir = Join-Path -Path $ShowDir -ChildPath $SeasonNumber

    #Write-Host "$ShowDir\"
    #Write-Host $SeasonDir

    #Checking if Show already has a folder
    if (!(Test-Path "$ShowDir\" -PathType Container)) {
        New-Item -ItemType Directory -Force -Path "$ShowDir\"
    }

    #Checking if directory already has folder
    If(!(Test-Path "$SeasonDir\" -PathType Container))
    {
        #if it does not create it
        New-Item -ItemType Directory -Force -Path $SeasonDir
    }

    Move-Item -Path $file.FullName -Destination $SeasonDir
}

Get-ChildItem -include *.ext1, *.ext2 instead of Where
Use pipelining via | (in direct fashion, that is without storing the results in intermediate variables) instead of foreach statement so that the processing starts immediately without waiting for the entire directory to be enumerated first (or use the advanced method of non-blocking IO.DirectoryInfo enumeration in a foreach statement).
Join-Path 'dir' * to construct the path for Get-ChildItem instead of relying on the trailing backslash
-LiteralPath instead of -Path, otherwise PowerShell will fail on paths that contain square brackets as it treats [] as a wildcard specifier.
string concatenation instead of switch

And my personal preferences:

Use regex to split the file name - it'll make the structure more obvious (if you know regex)
Use mkdir instead of New-Item -ItemType Directory (I think it's overly verbose to the point of being obfuscatingly so)
Simply create the directory without checking its existence: the operating system does it anyway, and the code is less "noisy" this way.

$fileDirectory = "C:\Test"
$ParentDir = "c:\TV Shows"

Join-Path $fileDirectory * | Get-ChildItem -include *.mkv, *.avi, *.mp4 | ForEach {
    if ($_.BaseName -notmatch '^\s*(.*?)\s*\.S(\d\d)E(\d\d)') {
        return
        # return is used because we're in a function-like ScriptBlock context
    }
    $ShowDir = Join-Path $ParentDir ($matches[1])
    $SeasonDir = Join-Path $ShowDir ('Season ' + $matches[2])
    # $matches is an internal object set automatically by -match or -notmatch operator

    mkdir $ShowDir -Force >$null
    mkdir $SeasonDir -Force >$null
    # mkdir outputs the directory object so we ignore it

    Move -LiteralPath $_.FullName -Destination $SeasonDir
}