python search replace and join with delimiter I'm brand new and been going through some beginner code problems on checkIO. I finished this one today and am posting because I'm sure there is a much more elegant way to accomplish this, and I think this particular problem has a lot of useful applications. I would like to know what are the 'clean and efficient' ways to do it, or maybe what built-ins I haven't learned yet that I should explore. My version seems pretty ugly. Thanks!

Problem: take a tuple of phrases, anytime the word 'right' appears, replace it with 'left'. Create a new string combining all the phrases separated with a comma.

def left_join(phrases):
    """
        Join strings and replace "right" to "left"
    """
    phrases = list(phrases)
    newstr = ''
    for j, phrase in enumerate(phrases):
        if 'right' in phrase:
            mod = dict(enumerate(phrase))
            for key in mod:
                if mod[key] == 'r' and mod[key+4] == 't':
                    i = key
                    mod[i] = 'l'
                    mod[i+1] = 'e'
                    mod[i+2] = 'f'
                    mod[i+3] = 't'
                    mod[i+4] = ''
            switch = (list(mod.values()))
            phrase = ''.join(switch)
            if j == (len(phrases) - 1) or len(phrases) == 1:
                newstr += phrase
            else:
                newstr += phrase + ','
        else:
            if j == (len(phrases) - 1) or len(phrases) == 1:
                newstr += phrase
            else:
                newstr += phrase + ','
    return newstr

test1 = ("bright aright", "ok", "brightness wright",)
test2 = ("lorem","ipsum","dolor", "fright", "nec","pellentesque","eu",
         "pretium","quis","sem","nulla","consequat","massa","quis",)

Welcome to code review, good job using a docstring and trying to make your code readable.

Space after a comma:

test2 = ("lorem","ipsum","dolor", "fright", "nec","pellentesque","eu",
"pretium","quis","sem","nulla","consequat","massa","quis",)

According to PEP0008 which I recommend checking for being the official Python style guide: a space should be left after a comma for readability.

Which should look like:

test2 = ('lorem', 'ipsum', ...)
Use descriptive names: names like left_join() newstr j are not describing the objects they represent ex: your function name can be def replace_word(words, word, new_word):
Type hints: You may use type hints to indicate what are the inputs and outputs in the following way:

def your_function_name(phrases: tuple) -> str:
Docstrings: Good job using a docstring for your function, however the description is not very clear here's a better description and btw docstrings should indicate what the input and the output is (what the function parameters are and what it returns):

def your_function_name(phrases: tuple, old_word: str, new_word: str) -> str:
    """
    Replace the old_word by the new_word if found in phrases.
    Args:
        phrases: Tuple of words.
        old_word: The word to replace.
        new_word: The replacement.
 Return:
     Joined string after replacements.
    """

str.replace() and str.join() You can use the replace method to achieve the same results in a shorter code and since you're familiar with join():

Instead of doing this for every letter in every word:

else:
    newstr += phrase + ','

You can do:

', '.join(words)

Code might look like:

And Note: You usually wouldn't be creating a function for such a simple task but use replace() and join() directly, I'm creating a function just for the sake of the example.

def replace_word(phrases: tuple, old_word: str, new_word: str) -> str:
    """
    Replace the old_word by the new_word if found in phrases.
    Args:
        phrases: Tuple of words.
        old_word: The word to replace.
        new_word: The replacement.
 Return:
     Joined string after replacements.
    """
    return ', '.join(word.replace(old_word, new_word) for word in phrases)

if __name__ == '__main__':
    test1 = ("bright aright", "ok", "brightness wright",)
    test2 = ("lorem","ipsum","dolor", "fright", "nec","pellentesque","eu",
 "pretium","quis","sem","nulla","consequat","massa","quis",)
    print(replace_word(test1, 'right', 'left'))
    print(replace_word(test2, 'right', 'left'))

Output:

  bleft aleft, ok, bleftness wleft

  lorem, ipsum, dolor, fleft, nec, pellentesque, eu, pretium, quis, sem, nulla,
  consequat, massa, quis


Unique everlasting session with low collision rate and is set across sub-domains The goal is to try and make an everlasting SESSION without too much or unnecessary scripting for updating of sessions. And at the same time to avoid session collision as session collisions could lead to 2 users trying to make use of the same session. That would not look pretty! And it does happen. Also for websites that use different language sub-domains, it would be useful if session did not change so that you can easily go from one sub-domain to another without requiring new sessions.

STEP 0:

function getUserIP(){
    $client  = @$_SERVER['HTTP_CLIENT_IP'];
    $forward = @$_SERVER['HTTP_X_FORWARDED_FOR'];
    $remote  = $_SERVER['REMOTE_ADDR'];

    if(filter_var($client, FILTER_VALIDATE_IP)){
        $ip = $client;
    }else
    if(filter_var($forward, FILTER_VALIDATE_IP)){
        $ip = $forward;
    }else{
        $ip = $remote;
    }
    return $ip;
}
$ip = getUserIP();

STEP 1:

ini_set('session.gc_probability', 0);// 0% probability of purging session log files, in this case on startup we preventing sessions from ever being deleted!
session_set_cookie_params(0, '/', '.website.com');// allow same session to be set across sub-domains!
session_name('session');// now make sure that the same session will not be regenerated across sub-domains and stay the same!
session_save_path($_SERVER['DOCUMENT_ROOT'].'/SESSION');//finally sessions are stored in a folder of choice in say public_html folder..
session_start();// we allow sessions to be read or changed..

STEP 2:

if(isset($_SESSION['ip'])){// if IP session in session log exist, do..
    if($ip!=$_SESSION['ip']){// if the current IP is not in the session log, then user privileges hasn't been confirmed!
        session_regenerate_id();// make sure that current visitor will be forced to use a different session_id! // new session copy is generated!
        session_destroy();// new session copy will be cleansed from previous data to be ready for use!
        header('Location: .');// reload the current page and to begin using a new session algorithm!
    }// SESSION collision has now been avoided and replaced with SESSION+IP collision where chances of that are pretty much ZERO!!!
    // IP is checked first, even if collision does occur, it will not allow to use it because of the missing IP..
    // that means someone must be in a public place that uses same IP for all PC's and within that amount of PCs a collision can occur!
}else{// if session doesn't exist, start new log!
    $_SESSION['ip']=$ip;// visitor session log file has been updated!
    echo 'NEW SESSION!<br>';
}

Is this a good solution? Is it a great solution? Is it the best solution? What do you guys think? :)
Note, that I didn't include the cleaning up process in this scenario, so you should use your imagination! But with this script you shouldn't even be needing a script to clean up sessions. All you'll need is to go to FileZilla, enter SESSION folder, select 1 file, press ctrl+a to select all session log files and then delete them all once a month or even once a year or so!? Also the storage directory for SESSIONS is just an example that you can put them in some nearby location closer to your development folder. But public_html is kinda PUBLIC and therefore you should use it along with .htaccess file and restrict access to the SESSIONS folder to secure it.

Since nobody else has chimed in here, I figured I would elaborate on my comments above.  If your stated goal is to:

have sessions that last indefinitely
have sessions that are more secure than default settings
have sessions that have have less chance of collision than default settings

Then this is the wrong approach.  You are actually not meeting any of these goals, and in fact are making security and chance of collision worse.

First,  these sessions will not be indefinite.  You are setting session cookie lifetime with value of 0 which means that, if the browser is compliant with this setting, the cookie will only last as long as the browser is open.  If that user closes their browser and then opens it back up 10 minutes later, they will get a new session.  There is no setting that will make a session last forever, as sessions are not really designed to last forever.  If you need data to persist between what truly are "sessions" then you need to look for another solution.  I am not suggesting you change this setting at all, as for most use cases when using sessions as intended, a value of 0 for cookie lifetime is a reasonable setting.

So, now that we have established that you are not truly getting "everlasting" sessions, let's get into the security aspects.

Here are a few PHP documentation links I will refer to:

Securing session INI
PHP Session Management Basics

We will start with garbage collection. Since you are not getting long-lasting sessions, that fact that you want to collect session data files (or database records if one were to use a database to store the data) indefinitely, means that you are, in essence, accumulating attack vectors against your application.  Ideally you should use an appropriate session lifetime and an aggressive approach to invalidating and pruning old records to minimize security risk.  One comment from the session.gc_maxlifetime section of the securing INI link above seems very pertinent here:

  If auto login feature is required, implement your own secure auto
  login feature. Never use long life session ID for it.

Long sessions should not be purposed for long-term data persistence or used for  authentication/authorization as it seems you are trying to do.

As I mentioned earlier accumulating these session files is like accumulating attack vectors, as you increase the opportunity for someone to execute a session hijacking or session fixation attack against your application.  This coupled with the fact that you open the application to using session cookies over non-secure (HTTP) connection, means it would be trivial for a packet sniffer or man in the middle to grab a session ID from a legitimate user's request and use it FOREVER.

And while destroying/regenerating session id's across IP change boundaries is a reasonable security measure, it is by no means enough.  That attacker could be sitting in a Starbucks along with the legitimate user using a wifi network which has the same exact IP address.  A sophisticated attacker can also easily spoof their IP address.

Also, keep in mind that there are some legitimate use cases where a user might change IP address during a session (a user on a mobile device for example), so while, IP address can be used as one of the triggers for session invalidation, if you use some of the other takeaways about handling session securely, you might decide that it's importance as a criteria for session invalidation might not be worth a potentially bad user experiences if you had a high number of mobile users.

Accumulating these session files also does increase your risk of a session collision.  Now that may be a pretty minute risk on a website with low traffic level anyway, but you are certainly making the probability of it happening worse, not better, with your approach.  And the IP check has nothing to do with mitigating against session id collision as mentioned in your comments.  That IP value is stored inside the session data and in no way helps mitigate a collision on the session id itself.

Let's talk about the session file save path.  I know there was some back and forth on this in the comments, with me suggesting that putting these files in a web directory is a bad security practice.  You are right that one could limit access to this directory in other ways, but why would you want to introduce a security vulnerability that you need an additional security measure to mitigate?  You are making your application security more complex and more fragile to someone not setting things up properly than it has to be.

Additionally, it just makes no practical sense to be putting this data into your application web directory anyway, just from a straight separation of concerns/access standpoint.  One doesn't typically configure database files, application logging, or other similar functionality that might read-write system files to do so into the application space, so why would you do this with session data?

Ideally, you write session data into a directory only available to the root user or user under which your application is running under to access and you don't let administrators working with the production system have root/sudo access in production unless absolutely necessary.

You should be using session.use_strict_mode if not specified as such in your ini file (this is not shown here obviously,so I am not sure what your current approach is).  This will help mitigate against session fixation, but will not help if an attacker uses one of the sessions id's you are collecting as cruft.

If you are using PHP 7.1, consider using session_create_id() to establish better connection between indvidual user and their session id.  This allows allows better usage of prefix index for database lookups if ever using a relational database as session storage medium.

Even for a case of using long sessions (or true browser-controlled session lengths as you are doing), you should regenerate session id at regular intervals to mitigate against session fixation/hijacking.  This means you have to start managing a timestamp in the session data.  There really is no way around this in PHP sessions.  I know one of your stated goals was to not have to get into this, but there really is no alternative if you want your sessions to be secure.  Please read the "Session ID Regeneration" section of the second link given above.

There is a pertinent quote there:

  Developer must NOT rely on session ID expiration by
  session.gc_maxlifetime. Attackers may access victim's session ID
  periodically to prevent expiration and keep exploiting victim's
  session ID including authenticated sessions.

  Instead, you must implement time-stamp based session data management by yourself.

So a bad system would rely on garbage collection alone to provide mitigation against session fixation.  A really bad system would not use garbage collection at all.

Again... you have to use timestamps if you want secure sessions.

Additionally, session_regenerate_id() combined with session_destroy() in PHP doesn't magically work by itself.  See the warning note in the session_regenerate_id() documentation:

  You should not destroy old session data immediately, but should use
  destroy time-stamp and control access to old session ID. Otherwise,
  concurrent access to page may result in inconsistent state, or you may
  have lost session, or it may cause client(browser) side race condition
  and may create many session ID needlessly. Immediate session data
  deletion disables session hijack attack detection and prevention also.

Again... you have to use timestamps if you want secure sessions.

Continuing in the area of session data management. Let's talk data deletion.  Please heed this text from the "Session Data Deletion" section of the second link above:

  Obsolete session data is better to be removed as soon as possible.
  However, active sessions MUST NOT be removed immediately. To satisfy
  these requirements, you MUST implement time-stamp based session data
  management by yourself.

  Set and manage expiration time-stamp in $_SESSION. Prohibit access to
  obsolete session data. When obsolete session data access is detected,
  it is advised to remove all authenticated status from the user's
  sessions and force them to re-authenticated. Obsolete session data
  access could be an attack. To do this, you must keep track active
  sessions per user.

  In short, do not destroy session data by session_regenerate_id() nor
  session_destroy(), but use time-stamp to control access to session
  data. Let session_gc() to remove obsolete data from session data
  storage.

Again... you have to use timestamps if you want secure sessions.

session_name('session');// now make sure that the same session will not be regenerated across sub-domains and stay the same!

This doesn't really do what the comment says it does.  This function simply determines that name of the session cookie to be used.  This doesn't guarantee any cross-domain interactivity of the cookie unless this same name is used for the cookie in all those sub-domains.  I think naming your sessions is totally fine, but this is really not necessary to achieve this effect. For example, if every subdomain was a PHP app with the same INI setting for this, this line would be unneccessary.  Similarly, if you had other subdomains that were not PHP apps, this code here does nothing to help you, as you would need to use whatever means the language in use on that subdomain uses to set this cookie name.

If all of this seems overly complex to manage in a secure manner, that is understandable.  As I said in my comments to the post above, PHP does not make secure session management easy in an out of the box manner. It requires work on the part of the developer.

You might consider using a good session management library if you want to simply drop some functionality into your application and not worry so more about the minutiae of managing the sessions.


Angular controller to handle showing or hiding multiple widgets I have 7 variables controlling 7 widgets showing or hiding from the frontend. I want to put them together and control all of them using one function. Here is what I have. However, I feel like even though this way works, the code is really ugly and should have space for improvements.

$scope.showWidget = function(showA, showB, showC, showD, showE, showF, showG) {
                    if (showA !== null) {
                        $scope.showshowA = showA;
                    }
                    if (showB !== null) {
                        $scope.showshowB = showB;
                    }
                    if (showC !== null) {
                        $scope.showshowC = showC;
                    }
                    if (showD !== null) {
                        $scope.showD = showD;
                    }
                    if (showE !== null) {
                        $scope.showshowE = showE;
                    }
                    if (showF !== null) {
                        $scope.showshowF = showF;
                    }
                    if (showG !== null) {
                        $scope.showG = showG;
                    }
                }

E.G. I will use the following code snippet for controlling showG variable on the fly.

 $scope.showWidget(null, null, null, null, null, null, true);

You could change your ng-show to use a function:

ng-show="isShown('widgetX')" ng-click="toggleWidget('widgetX')"

And then in your controller:

var widgets = {
    widgetA: {
        shown: true
    },
    widgetB: {
        shown: true
    },
    ...
};

$scope.isShown = function(widgetID) {
    return widgets[widgetID].shown;
}

$scope.toggleWidget = function(widgetID) {
    widgets[widgetID].shown = !widgets[widgetID].shown;
}


Range Minimum Query Implementation in Scala I would like to heat some feedback on this. I'm coming from a Java background and this is my first program in Scala. I solved Range Minimum Query problem.

object Solution {

  def main(args: Array[String]){
    val readInfo = readLine.split(" ");
    val numberofElements = Integer.parseInt(readInfo(0))
    val numberOfQuery = Integer.parseInt(readInfo(1))
    val storeElemets: Array[Int] =readLine.split(" ").map(_.toInt);
    val j =0;
    for(j<-0 until numberOfQuery){
      val z = readLine.split(" ")
      val from = Integer.parseInt(z(0))
      val to = Integer.parseInt(z(1))+1
      val i=from
      val getNumber = (from until to).map{i => storeElemets(i)}
      println(getNumber.min);
    }
  }
}

I would like to know, What are the best practices and is there any better way to implement this solution ?

For starters,
  your spacing is rather inconsistent: val j =0 val i=from,
  typos abound: storeElements, numberOfElements numberOfQueries,
  and you terminate some statements with ;.

Be aware that a lambda foo => bar(foo) is similar to the declaration def anonymous(foo) = bar(foo). That is, the parameter name foo is already declared by being a parameter – your val i = from is a absolutely unnecessary. Similarly,  val j = 0 is not needed because j <- ... in the for-comprehension already amounts to a declaration.

In Scala, curly braces {} and parens () are rather similar. Use curlies only when using a multi-statement lambda.

In one place, you do _.toInt, in another you Integer.parseInt(_). I'd rather use the first solution, as it is shorter.

If we clean up these minor problems (and remove some unnecessary variables), we end up with this code:

object Solution {
  def main(args: Array[String]) {
    val readInfo = readLine.split(" ")
    val numberOfElements = readInfo(0).toInt
    val numberOfQueries  = readInfo(1).toInt
    val storeElements = readLine.split(" ").map(_.toInt)
    for (i <- 0 until numberOfQueries) {
      val z = readLine.split(" ")
      val from = z(0).toInt
      val to   = z(1).toInt + 1
      val getNumber = (from until to).map(i => storeElements(i))
      println(getNumber.min)
    }
  }
}

A number of things appears suspect, for example the name z which should better be range, and the +1 for the upper bound of the range. The a until b method excludes the upper bound [a, b). However, there is also a a to b function, which produces an inclusive range [a, b].

The getNumber variable has a rather weird name, and I wouldn't even assign this value to a name. The minimum is more interesting than the slice.

What (z(0).toInt to z(1).toInt).map(i => storeElements(i)) calculates is a slice of elements in the array. There is the builtin method slice for this use case (which takes the inclusive lower and exclusive upper bound as arguments).

We now have the code:

object Solution {
  def main(args: Array[String]) {
    val problemSize = readLine.split(" ")
    val elements = problemSize(0).toInt
    val queries  = problemSize(1).toInt
    val input = readLine.split(" ").map(_.toInt)
    for (i <- 0 until queries) {
      val range = readLine.split(" ")
      val minimum = input.slice(range(0).toInt, range(1).toInt + 1).min
      println(minimum)
    }
  }
}

But is this a Range Minimum Query? No, it just returns the minimal element in that slice. We are not interested in the minimal value, but rather in the position of the minimal value.

Because this is just a code review, I won't explain an algorithm for performing the RMQ, but the external links in the Wikipedia article you linked to contain a few algorithms.


Calculating sum of manhattan distances in a sliding puzzle I would like some feed back on a method which calculates the sum of Manhattan distances for each tile in a sliding puzzle to its goal position in the goal puzzle.

Here is the code:

public static int calcManhattanDistance(int[] ps, int[] goal) {
    int rowSz = calculatePuzzleRowSize(ps);
    int manhattanDistanceSum = 0;
    for (int i = 0; i < ps.length; i++) {
        for (int j = 0; j < ps.length; j++) {
            if (ps[i] == goal[j])
                manhattanDistanceSum += (Math.abs(i/rowSz - j/rowSz) + Math.abs(i%rowSz - j%rowSz));
        }
    }
    return manhattanDistanceSum;
}

Please tell me if you think something should be changed or what I can do to increase efficiency.

Here is the calculatePuzzleRowSize(ps) method:

public static int calculatePuzzleRowSize(int[] ps) {
    int sz = ps.length;
    int rowSz = (int) Math.sqrt(sz);
    return rowSz;
}

Here is the newly implemented calculateManhattanDistanceSum

public static int calculateManhattanDistanceSum(int[] ps, int[] goal) {
    int rowSize = (int) Math.sqrt(ps.length);
    int manhattanDistanceSum = 0;
    Map<Integer, Integer> psMap = new HashMap<Integer, Integer>();
    for (int i : ps)
        psMap.put(ps[i], i);
    for (int j : goal)
        manhattanDistanceSum += Math.abs(psMap.get(j)/rowSize - j/rowSize) + Math.abs(psMap.get(j)%rowSize - j%rowSize);
    return manhattanDistanceSum;
}

Assuming you hold the right order of the puzzle pieces in the goal-array, wouldn't it be natural that the right order itsself is 0 to #pieces - 1?

If that's the case, you could calculate the manhatten distance by just iterating over ps[]:

for (int i = 0; i < ps.length; i++)
    if( ps[i] == i ) continue;
else
    manhattanDistanceSum += (Math.abs(ps[i]/rowSz - i/rowSz) + Math.abs(ps[i]%rowSz - i%rowSz));


Unit of work and common implementation I have some questions regarding a common implementation of the UoW, Repository Pattern and EF:

public interface IAppUow
{
    void Commit();
    IRepository<Customer> Customer{ get; } // IRepository<T>
    IOrderRepository Order{ get; }         // implements IRepository<T>
}

My questions are:

If I need to add a new repository of whatever type, I have to change the UoW.  Is this a good practice? If not, is there a better way?
If I run Commit() { context.SaveChanges(); }, it will save the state of all the context's repositories. Is this a good way?

This question is on the edge of being off-topic, there's really not much code to review here.

However your IAppUow interface has a number of flaws:

Naming: I don't see a need for the App part. I think a better name would be more like IUnitOfWork.
Inconsistency: You have IRepository<Customer>, and then IOrderRepository - why don't you have IRepository<Order> or IOrderRepository? When/if you do add repositories to your UoW interface, do you add a IRepository<Entity> or a IEntityRepository? This inconsistency makes it very hard to tell.

  If I need to add a new repository of whatever type, I have to change the UoW. Is this a good practice? If not, is there a better way?

An interface shouldn't be designed to change, as every change to an interface is a breaking change. I think you're mixing up the abstraction with its implementation; UoW encapsulates a transaction - and these things can be committed or rolled back. Thus, if I were to wrap the EF UoW into another layer of abstraction (I'll get back to that), I'd have a void SaveChanges() and void DiscardChanges() methods on the UoW interface.

  If I run Commit() { context.SaveChanges(); }, it will save the state of all the context's repositories. Is this a good way?

This is how Entity Framework works. DbContext is a unit of work; if you Dispose() the context before saving any changes, nothing gets to the database. If you SaveChanges(), the unit of work commits all changes made.

DbContext is an implementation of a unit of work - it exposes repositories (IDbSet<TEntity>) and a method to commit changes (and stuff about entity state management and model generation).

This might not be what you want to hear, but this answer sums up my thoughts pretty well: https://softwareengineering.stackexchange.com/a/214022/68834 - in short, you don't need the pattern if you're using an Object/Relational Mapper.


Refactoring a series of if-return statements. In my model, I have things called Reports that have known workflow associated with them.

I've got a requirement to output at which state a report currently is.

public abstract partial class Report{
   public virtual string StateStatus(){
     if (Check.IsSuccessful) return "Approved";
     if (Check.AreClarificationsSent) return "Clarifications";
     if (Check.IsInProgress) return "Check in progress";
     if (IsTemplateSent) return "Sent";
     if (IsReceived) return "Received";
     return "Not received";
   }
  ...
}

I dislike this method. It won't break, but leaves some nausea in my head, just doesn't feel right.

Got any recommendations for improvement?

It seems that you are wanting your Check enum to be able to provide a little more information for you without the messiness of the if-then clutter. What about combining the status strings with the enumerator? Something like:

public class CheckStatus{
    public static readonly CheckStatus
      IsSuccessful = new CheckStatus { Value = "Approved" },
      AreClarificationsSent = new CheckStatus { Value = "Clarifications" },
      IsInProgress = new CheckStatus { Value = "Check in progress" },
      IsTemplateSent = new CheckStatus { Value = "Sent" },
      IsReceived = new CheckStatus { Value = "Received" },
      NotReceived  = new CheckStatus { Value = "Not Received" };

    private CheckStatus() { }
    public string Value{ get; private set; }
}

Very easy to maintain and puts the minor implementation details behind the purpose of the Check enumerator. Provides much the same benefit of an enumeration. I would have liked to make this a struct, but I would not (without going into IL) have a parameterless private ctor.


Recursive program for generating and printing all permutations of the numbers 1, 2, ..., n for given integer number n I've just written code for generating all permutations of the numbers from 1 to n in Java. It seems to work, but I think it's a bit more complex than it needs to be.

import java.util.*;

public class ProblemFour {
    private static int n;

    private static void printResult(int[] result) {
        Set<Integer> set = new HashSet<>();
        // I am using Hashset<> to avoid repetitions
        Integer[] nums = new Integer[result.length];
        for (int i = 0; i < n; i++) {
            nums[i] = result[i];
        }

        Collections.addAll(set, nums);
        if(set.size() == n) System.out.println(Arrays.toString(nums));
    }

    private static void permute(int[] result, int index) {
        if (index == result.length) {
            printResult(result);
            return;
            // Recursion bottom
        }
        for (int i = 1; i <= n; i++) {
            result[index] = i;
            permute(result, index+1);
        }
    }

    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);

        System.out.print("From 1 to: ");
        n = input.nextInt();

        int[] result = new int[n];

        permute(result, 0);
    }
}

Single responsibility principle
Think about the possible reuse of your code. More specifically, the permute method. Do you think that the caller will always want to print out the result? Right now storing the permutations in a variable is impossible, so I cannot use this as a building block for bigger programs. What about printResult it does not only print the result, it also takes care of duplicates.
In general each method does many things, even asking for global variables around the class (printResult accesses n)
I would write follow this stub:
public class Permutation extending Array {
    public static Sequence[Sequence[Any]] permutations(Sequence[Any] array) {
        // implementation
    }
}

public class PermutationUptoApplication {
    public static void main(String[] args) {
        int upperLimit = (new Scanner(System.in)).nextInt()
        System.out.println(Range(1, upperLimit).toArray().permutations().join("\n"));
}

You see that I wrote this in pseudo-code, but the main concept is that now you can import the Permutation class from anywhere and call it and reuse the result.
The application class just handles input and output and generates the range of numbers.


Parsing a string with specific pattern into json I've a string as an input which is having some specific patterns and I'm trying to parse it into a desired json. Here is my code:

const input = "status:all,applied_date:2019-04-15--to--2019-04-15,screen_status:SR|NS";

    const output = { and: {} };

    const dateSeparator = RegExp("--to--");

    function parseStr(str) {
      const arr = str.split(",");
      for (let i=0; i<arr.length; i++) {
        findPattern(arr[i]);
      }
      console.log(JSON.stringify(output));
    }

    function findPattern(subStr) {
      const subArr = subStr.split(":");
      const newKey = subArr[0].replace("_", ".");
      output["and"][newKey] = {};
      if(dateSeparator.test(subArr[1])) {
        output["and"][newKey]["between"] = subArr[1].split("--to--");
      } else if(subArr[1].split("|").length > 1) {
        output["and"][newKey]["inq"] = subArr[1].split("|");
      } else {
        output["and"][newKey]["eq"] = subArr[1];
      }
    }

    parseStr(input);

I think my approach is very naive and it's not good for complex strings(e.g. deep nesting) and also the logic I've written cannot identify if the given input conform the patterns and will fail if wrong input is provided.

Is there any efficient way to parse strings like these?

I could have criticized the most lines of the initial approach in terms of naming, relations, structuring and performance, but I believe it should be completely rewritten.

The optimized approach:

const input = "status:all,applied_date:2019-04-15--to--2019-04-15,screen_status:SR|NS";
const dateSep = /--to--/;

/**
 * Parse input string to build JSON representing
   a rule-set of AND clauses for SQL query.
 * The return value is JSON output.
 * @param {string} str - input string
 * @param {string} [entry_sep] - entry separator, defaults to ","
 */
function strToAndClause(str, entry_sep = ",") {
    let entries = str.split(entry_sep);
    let output = { and: {} },
        and_clause = output.and;

    entries.forEach((entry) => {
        let [key, val] = entry.split(":"), rule = {};
        key = key.replace("_", ".");
        and_clause[key] = rule;

        if (dateSep.test(val)) {
            rule.between = val.split(dateSep);
        } else if (val.indexOf("|") !== -1) {
            rule.inq = val.split("|");
        } else {
            rule.eq = val;
        }
    });
    return JSON.stringify(output);
}

console.log(strToAndClause(input));


Converting an angle in degrees to a pretty string representing it in radians Given an angle in degrees, output a pretty string representation of its value in radians as a fraction.

Where pretty means:

Simplified as much as possible.
With no unnecessary ones.
Using the Unicode character that represents pi: π
With a single minus in front if negative (a minus at the denominator is not allowed).

For example: degrees_to_pretty_radians(-120) #=> '-2π/3'

I wanted to write this in Python too to compare it to how it looked implemented in Javascript

The code is pretty straightforward and passes a lot of test-cases, but I am still interested in any kind of feedback:

def degrees_to_pretty_radians(angle: "in degrees") \
                    -> "Pretty string for the value in radians.":
    """
    >>> tests = [0, 1, 18, 31, 45, 60, 120, 180, 270, 360, 480]
    >>> all_tests = tests + [-x for x in tests]
    >>> for angle in all_tests: print(angle, degrees_to_pretty_radians(angle))
    0 0π
    1 π/180
    18 π/10
    31 31π/180
    45 π/4
    60 π/3
    120 2π/3
    180 π
    270 3π/2
    360 2π
    480 8π/3
    0 0π
    -1 -π/180
    -18 -π/10
    -31 -31π/180
    -45 -π/4
    -60 -π/3
    -120 -2π/3
    -180 -π
    -270 -3π/2
    -360 -2π
    -480 -8π/3

    """
    gcd = fractions.gcd(angle, 180)
    denominator = "" if 180 // gcd == 1 else "/{}".format(180 // gcd)
    numerator = "" if abs(angle // gcd) == 1 else abs(angle // gcd)
    sign = "-" if angle < 0 else ""
    return "{}{}π{}".format(sign, numerator, denominator)

Here are some thoughts on your code:

I'm not too fond of function annotations – Maybe it's just me, but I find the function annotations a slightly confusing syntax addition. I would rather extend the docstring with this information.
Compress the doctests a little – You get bonus points for using doctests, but I would compress the output somewhat. Maybe build an array of result values, and do a comparison against that. Or simply reduce the number of values. As it stands, it is more doctests code than actual code doing something useful
Avoid duplicating calculations – You do the 180 // gcd and abs(angle // gcd) twice. One for the if and one as a result. This could possibly read better using temporary variables to both avoid the double calculation, and to simplify the ternary statements.

However there is an even neater solution to avoid most of your code, and that is to use all of the capabilities of the fractions module. Using this module your function can be reduced to:

rads = fractions.Fraction(angle, 180)
return "{}π/{}".format(angle, rads.numerator, rads.denominator)

Of course, this does return mostly the same as your function returns, but it does return some of the basic cases like "0π/1" or "1π/3". These could be handled using some ternary operations, which leads us back to code similar to your original code (but without exposing the calculations, and with compression (not reduction) of test cases):

def degrees_to_pretty_radians(degrees):
    """Return the degrees in fractions of π:

    >>> tests = [0, 1, 18, 31, 45, 60, 120, 180, 270, 360, 480]
    >>> all_tests = tests + [-x for x in tests]
    >>> expected_result = ['0π', 'π/180', 'π/10', '31π/180',
    ...        'π/4', 'π/3', '2π/3', 'π', '3π/2', '2π', '8π/3',
    ...        '0π', '-π/180', '-π/10', '-31π/180',
    ...        '-π/4', '-π/3', '-2π/3', '-π', '-3π/2', '-2π', '-8π/3']
    >>> actual_result = [degrees_to_pretty_radians(d) for d in all_tests]
    >>> actual_result == expected_result
    True
    """

    rads = fractions.Fraction(degrees, 180)
    numerator = "" if rads.numerator == 1 else \
                "-" if rads.numerator == -1 else \
                rads.numerator
    denominator = "" if rads.denominator == 1 else \
                  "/{}".format(rads.denominator)
    return "{}π{}".format(numerator, denominator)


PHP Script - Converting string of calculations to result So I am making a little script to convert a string of a basic maths calculation eg:

"2+3+4-2+2-12+42"

and it should return the total.

What I have so far :

<?php

function my_operator($a, $b, $char) {
    switch($char) {
        case '-': return $a - $b;
        case '+': return $a + $b;
        default: return $a;
    }
}

$string = "1+41-3+5-12+5+10";
//Ans = 47

$Arr = str_split($string);
$total = 0;
$cache1 = "";
$cache2 = "";
$lastOperator ="";

foreach ($Arr as $char) {
    if ($char == "+") {
        if ($cache2 == "") {
            $total = $cache1;
        }
        $cache2 = $cache1;
        $cache1 = "";
        $total = my_operator($total, (int)$cache2, $lastOperator);
        $lastOperator = "+";
    } else if ($char == "-") {
        if ($cache2 == "") {
            $total = $cache1;
        }
        $cache2 = $cache1;
        $cache1 = "";
        $total = my_operator($total, (int)$cache2, $lastOperator);
        $lastOperator = "-";
    } else {
        $cache1 = $cache1.$char;
    }
}

$total = my_operator($total, (int)$cache1, $lastOperator);

echo $total;

What are some improvements I can make? Thanks.

I really should be discussing your code here, but ones I got started I took it a bit too far. So before I do that, let's talk about your code.

names

Names of variables and functions give meaning to your code. They should tell the reader exactly what is in them, or what they do. A name like $string or $Arr clearly doesn't do that. Most names are good, but they all need to be good for the code to be easily read. You'll notice that readability is very important to me.

functions

You've clearly mastered the usage of functions, but you're not using them effectively. Functions allow you to write code with another level of abstraction and meaning, making it easier to understand. Functions also isolate variables from the global scope, which will become important when your application grows.

don't repeat yourself (DRY)

See: https://en.wikipedia.org/wiki/Don%27t_repeat_yourself

I've noticed the following structure in your code:

if ($char == "+") {
    .....
    $lastOperator = "+";
} else if ($char == "-") {
    .....
    $lastOperator = "-";
} else {
    $cache1 = $cache1.$char;
}

the ..... represent two pieces of code that are exactly the same. Why not write:

if (($char == "+") || ($char == "-")) {
    .....
    $lastOperator = $char;
} else {
    $cache1 = $cache1.$char;
}

that way you don't have to repeat the same code twice.

choice of algorithm

You process input characters one by one. This is hard. No really, it is. Would this really be the best algorithm? I don't think so. How would a person do the calculation? They would split the string like this: 1,+,41,-,3,+,5,-,12,+,5,+,10, and then simply perform the operations sequentially. Why don't you? With explode() you could split the string like this:

function splitQuestion($question)
// split a math question into numbers, plus and minus
{
  return explode('#',str_replace(['+','-'],['#+#','#-#'],$question));
}

// example code
$question = '1+41-3+5-12+5+10';
echo '<pre>'.print_r(splitQuestion($question),TRUE).'</pre>';

The trick here is to first insert the hashes and only then explode on that. After this writing an algorithm to do the calculation will be much easier because you don't need to cache any characters.

error checking

Your code almost seems to assume the input is valid. There's little checking of, and tolerance for, syntax errors in the input. A programmer should always think about possible errors in the input. They will always occur, and in my experience unexpected input is the most common 'cause' of bugs.

playtime

I couldn't resist to write a very short calculator for you:

// user input
$question = '1+41-3+5-12+5+10';
// get the answer
eval('$answer = '.$question.';');
// show question and answer
echo $question.' = '.$answer;

There are several reasons to be scared of the eval() function in PHP, read the warning here: http://php.net/manual/en/function.eval.php

This warning is appropriate and should be taken notice of. So, normally you really shouldn't use it, but in the case of a calculator it can be very powerful. You can basically do everything which PHP can do, and that's a lot (hence the danger).

As long as you hardcode the math question in PHP there's no problem, but that's not the point of a calculator, you want user input. So instead of working hard on deciphering the question, why not restrict and validate the user input and use eval() to calculate the result?

For instance if only the characters 1234567890+- are allowed in the question string nothing damaging can happen. This would only add one line to the code:

// user input
$question = '1+41-3+5-12+5+10';
// filter everything except numbers, plus and minus
$question = filter_var($question,FILTER_SANITIZE_NUMBER_INT);
// get the answer
eval('$answer = '.$question.';');
// show question and answer
echo $question.' = '.$answer;

You would want to put this code in a function for two reasons: It isolates the eval() even more, and it makes your code easier to read. Like so:

function calculate($question)
// perform a calculation with numbers, plus and minus
{
  return eval('return '.filter_var($question,FILTER_SANITIZE_NUMBER_INT).';');
}

// user input
$question = '1+41-3+5-12+5+10';
// show question and answer
echo $question.' = '.calculate($question);

This can be easily extended to multiplications and devision, or the use of brackets. Do with it what you want.


CRC without knowing the data type or size I wrote a function to calculate an 8-bit CRC/checksum for unknown data types. The only restriction is the data type must be at most 4bytes/32bits.

uint8_t crc(void* _crc, ssize_t len, uint8_t _crc_seed) {
    void* _crc_cpy = malloc(len);
    memcpy(_crc_cpy, _crc, len);
    uint32_t* _crc_cpy_pointer = static_cast<uint32_t*>(_crc_cpy);
    uint8_t _crc_u[len];
    for(int i = len; i > 0; i--) {
        _crc_u[len-i] = ((*_crc_cpy_pointer)&(0xff << 8*(i-1))) >> (8*(i-1));
    }
    uint8_t crc_seed = _crc_seed;
    uint8_t crc_u;
    for(int i = 0; i < len; i++) {
        crc_u = _crc_u[i] ^= crc_seed;
        for(int i = 0; i < 8; i++) {
            crc_u = (crc_u & 0x80) ? 0x7 ^ (crc_u << 1) : (crc_u << 1);
        }
        crc_seed = crc_u;
    }
    return crc_u;
}

How can I do this better? What are things that I did incorrectly?

Its always informative to comment the specific crc polynomial. An 8 bit crcs is only marginally better than 8 bit checksum.

First, remove the malloc() and memcpy(). A crc is a calculation that does not affect the input data. There is no need to allocate and copy the input data. Also, your code has a memory leak because its not releasing the allocated memory.

Second, just read each byte and update the crc calculation. There is nothing to gain with an arbitrary restriction on the input data size.

Third, replace the bit by bit calculation loop with a byte wide calculation.


Meta functions for sequences of exponents of 2 I have implemented some meta functions to create sequence for exponents of 2. I would love to know if there is any way to make the code more optimized.
Note: int main() should not be modified. From the requirements of my school:

This program must implement three definitions: sequence<Ts...>, print() and printing_type().

Hence, the name of functions should not be modified.
#include<iostream>

template<size_t... Ts>
struct int_seq{};

template<size_t TCount, size_t...Ts>
struct sequence
{
    using type = typename sequence<TCount - 1,  1, (2 * Ts)...>::type;
};

template<size_t...Ts>
struct sequence<0, Ts...>
{
    using type = int_seq<Ts...>;
};

template<size_t TCount>
using create_int_seq  = typename sequence<TCount + 1>::type;

template <typename T>
void print(T val)
{
    std::cout << val << " ";
}

template <typename T, typename... Ts>
void print(T val, Ts... t)
{
    std::cout << val << " ";
    print(t...);
}

template <
    template <size_t...> class T,
    size_t... S
>
void printing_type(T<S...>)
{
    print(S...);
}

int main()
{
    constexpr size_t N = 63;
    create_int_seq<N> dum{};
    printing_type(dum);
}

Certainly your definitions of printing_type and print could be shortened. You're currently doing
print(t...)  // print(a,b,c,d)

but what you actually want to happen is
print(t)...  // print(a), print(b), print(c), print(d)

So then you have to do a bunch of extra work to transform the former into the latter. Instead of all that extra work, you could just write
void print(size_t val) {
    std::cout << val << " ";
}

template<template<size_t...> class T, size_t... S>
void printing_type(T<S...>) {
    (print(S) , ...);
}

Notice that the single-argument print doesn't have to be a template, because val is always a size_t.
In fact, we can inline print into printing_type if we want:
template<template<size_t...> class TT, size_t... Values>
void printing_type(TT<Values...>) {
    ((std::cout << Values << " ") , ...);
}

The (ts , ...) syntax is a C++17 fold-expression. If you aren't on C++17, then you can use an initializer list to accomplish the same thing:
template<template<size_t...> class TT, size_t... Values>
void printing_type(TT<Values...>) {
    int dummy[] = {
        ((std::cout << Values << " "), 0) ...
    };
}

Your definition for sequence is pretty confusing. Your code could benefit from some unit tests. Compile-time unit tests are easy:
static_assert(std::is_same<create_int_seq<0>, int_seq<1>>::value, "");
static_assert(std::is_same<create_int_seq<1>, int_seq<1,2>>::value, "");
static_assert(std::is_same<create_int_seq<2>, int_seq<1,2,4>>::value, "");
static_assert(std::is_same<create_int_seq<3>, int_seq<1,2,4,8>>::value, "");
static_assert(std::is_same<create_int_seq<4>, int_seq<1,2,4,8,16>>::value, "");

As a bonus, these five lines serve as excellent documentation about what this code does... to the extent that I no longer mind the lack of code comments explaining how it might work.
FWIW, if I wanted to make these test cases pass, I'd implement create_int_seq like this:
template<class> struct powers_of_two;
template<size_t... Is>
struct powers_of_two<std::index_sequence<Is...>> {
    using type = int_seq<(size_t(1) << Is)...>;
};

template<size_t N>
using create_int_seq =
    typename powers_of_two<std::make_index_sequence<N+1>>::type;

Relevant blog post of mine: "Template metaprogramming: Iteration is better than recursion" (July 2018).


Count all vowels in string For a homework assignment, I was asked to complete this CodeStepByStep problem. It's coded correctly and gave me a right answer. I'm just wondering if there's an easier way to solve this problem without the big blocky code in the if statement. The fact that I have to check each lowercase vowel AND THEN the uppercase vowel seems verbose.

Write a function named vowelCount that accepts a string and returns
the number of vowels (a, e, i, o, or u) that the string contains.
For example, the call of vowelCount("kookaburra") should return 5 (two
o's, 2 a's, and one u). When passed a string without any vowels (such
as an empty string, "01234", or "sky"), 0 should be returned.

function vowelCount(string) {
    if(string.length == 0) { return 0; }
    let count = 0;
    for(let i = 0; i < string.length; i++) {
        let v = string[i];
        if(v == "a" || v == "e" || v == "i" || v == "o" || v == "u" ||
           v == "A" || v == "E" || v == "I" || v == "O" || v == "U") {
            count += 1;
        }
    }
    return count;
}

An alternative route is to use string.replace() and Regular Expressions to strip everything but the vowels from the string. Then count the length of the resulting string. This avoids iteration altogether.

const vowelCount = s => s.replace(/[^aeiou]/gi, '').length

console.log(vowelCount('The quick brown fox jumps over the lazy dog'))

The regex is /[^aeiou]/gi which means match everything that's NOT (^) in the set (aeiou), matching globally (g flag) and without regard to case (i flag). string.replace() then uses this pattern to replace all matching characters with a blank string. What remains are your vowels.


XML editing and merging I have only started working with XML files and PowerShell. I feel that while there are many ways to accomplish the same thing that I might be doing the following inefficiently.

This is all based on the following SO post which I have an answer to:

$numberOfCores = Get-WmiObject -class win32_processor numberOfCores | Select-Object -ExpandProperty numberOfCores

$path = "C:\Windows\Microsoft.NET\Framework64\v4.0.30319\Config\machineTest.config"
[xml]$machineConfig = Get-Content $path

# Remove the elements we are going to be replacing
$node = $machineConfig.SelectNodes("/configuration/system.web")
[void]($node.RemoveChild(($node.SelectSingleNode("processModel"))))
[void]($node.RemoveChild(($node.SelectSingleNode("httpHandlers"))))

# Create the element processModel and set attributes
$processModelxml = $machineConfig.CreateElement("processModel")
$processModelxml.setAttribute("maxWorkerThreads",370)
$processModelxml.setAttribute("maxWorkerThreads",370)
$processModelxml.setAttribute("maxIoThreads",370)
$processModelxml.setAttribute("minWorkerThreads",50)
$processModelxml.setAttribute("minIoThreads",50)
[void]($node.AppendChild($processModelxml))

# Create the element httpRuntime and set attributes. Adjust values based on number of cores
$httpRuntimexml = $machineConfig.CreateElement("httpRuntime")
$httpRuntimexml.setAttribute("minFreeThreads",90 * $numberOfCores)
$httpRuntimexml.setAttribute("minLocalRequestFreeThreads",80 * $numberOfCores)
[void]($node.AppendChild($httpRuntimexml))

# Build the <system.net> section
[xml]$systemnetxml = @"
  <system.net>
    <connectionManagement>
      <add address = "*" maxconnection = "$(200 * $numberOfCores)" />
    </connectionManagement>
  </system.net>
"@

# Import into config
$machineConfig.configuration.AppendChild($machineConfig.ImportNode($systemnetxml."system.net",$true)) | Out-Null

# Save changes
$machineConfig.Save("c:\temp\testing.xml")
# Change back to $path to write back to original file.

If you wanted to test this you need to change $path to that of a local machine.config in a .Net build folder. Also the last step saves the file in an alternate location for testing. Changing it to $machineConfig.Save($path) would be a production approach.

In my defense this code was gear towards a new user so there are a few lines that are created to be more verbose on purpose. I am mostly interested in the XML focus of this code but will welcome any criticism towards the rest (in case I am being complacent.) Was designed using PowerShell v4.

You code looks fine to me. I'll just make some random comments.

You can split long lines like this for the sake of readability:

$numberOfCores = Get-WmiObject -class win32_processor numberOfCores |
    Select-Object -ExpandProperty numberOfCores

This comment is confusing to me:

# Remove the elements we are going to be replacing

You are removing <httpHandlers> but adding back <httpRuntime>. Was that intentional or a bug?

Why remove and create a new <processModel>? Why not just update the attributes in the original node? You would save on a few lines of code.

What happens if these nodes already have some attributes set? If you blow away the nodes, you will blow away those attributes. Was that intentional?

Why do you suddenly change style when creating <system.net>? Why not continue using CreateElement and so on?

In most places, you cast to [void] to get rid of the return values. In one place you use Out-Null. It's best to be consistent. Personally I would go with Out-Null since it is more the PowerShell way, and I think it looks nicer.

If you like, you can use the Select-Xml cmdlet instead of SelectNodes. Something like this:

$node = $machineConfig | Select-Xml /configuration/system.web

If you are going to overwrite the original file, it might be a good idea to make a copy of the original first. Add a timestamp to the filename or something.

It might be overkill in this case, but I would be inclined to make the code data-driven. That is, put all the changes to be done in a list at the top of the code and walk through the list in the code making the changes. That way people could see at a glance what the script does without having to read through all the code.


Assembler-program which reverses the content of EAX Following exercise:

  Write a program that takes a number (of size 4 bytes) x as input, and then
      reverses all the bits of x, and outputs the result. By reversing all bits we
      mean that the bit with original location i will move to location 31-i.
      Small example (for the 8 bit case):
        if x == {01001111}_2, then the output is {11110010}_2.
        In this example we reversed only 8 bits. Your program will be able to
        reverse 32 bits.

Full exercise-description can be seen here: XORPD GitHub

I tinkered out the following idea. Please take into account my comments too.

format PE console
entry start

include 'win32a.inc'

; ===============================================
section '.text' code readable executable

start:
    mov     eax,    0x4f    ; 0x4f is equal to 01001111 (from the exercise-description example).
    mov     cl,     0x1f    ; cl becomes the control variable. 0x1f == 31 decimal
    xor     edx,    edx     ; edx will accumulate the different states of ebx during the runtime.
process_bit:
    shr     eax,    0x1     ; Kick the right-most bit out ...
    jc      add_one         ; If it was a 1 jump to 'add_one' ...
    mov     ebx,    0x0     ;  ... otherwise write a 0 ...
    jmp     now_rotate
add_one:
    mov     ebx,    0x1
now_rotate:
    rol     ebx,    cl      ; The right-most bit has been fresh written. Now move it n-positions to the left.
    or      edx,    ebx     ; "Save" or "Add" the current positive bits (1-bits) of ebx to edx.
    loop    process_bit

    mov     eax,    edx
    call    print_eax_binary

    ; Exit the process:
    push    0
    call    [ExitProcess]

include 'training.inc'

I guess it works right.

Please compare the result on the screenshot with the example-value from the exercise-description.

What to think about my solution?

Is it valid? Or does it have to be improved?

Is there a better way to solve the described task?

Looking forward to read your hints and comments.

In assembler it is often possible to avoid jmp instructions. This is done to improve branch prediction and thereby performance.

Your code can be written without the jump to add_one:

xor ebx, ebx
shr eax, 1
adc ebx, 0

These three instructions replace the whole jumping. The adc (add with carry) instruction's original purpose is to support chained addition, but it can be used creatively for many other purposes. Note that I had to move the xor ebx, ebx to the top since it updates the carry flag.

There are also the rcr and rcl instructions that efficiently use the carry flag as a one-bit register. You can just repeat 32 times:

rcr eax, 1
rcl ebx, 1

And you're done. When you inline this loop, you have a constant-time operation that finishes in 64 instructions. Each of them probably takes a single cycle, therefore 64 cycles. That's already acceptable, but there are probably better ways.

On the electrical, hardware level, the bitswap operation can be implemented by just swapping the wires, therefore chances are high that there is some machine instruction that makes this task more efficient. Just read through the whole processor manual to see if there is anything related. Have a look for the keywords "swap", "shift", "mask".

If you want to get a really fast program, you should use the bswap eax instruction, followed by code that reverses the bit order. This can probably be found in the excellent book Hacker's Delight.

The basic idea is to take every second bit and shift it to the left. At the same time, take the remaining bits and shift them to the right. Like this:

bits0 = (x & 0x55555555) << 1
bits1 = (x >> 1) & 0x55555555
x = bits0 | bits1

Then do the same thing with groups of 2 bits, and then once more with groups of 4 bits. The groups of 8 and 16 bits are done by the bswap instruction, you don't need to implement them on your own.


A non-blocking lock decorator in Python I needed to impose on a Python method the following locking semantics: the method can only be run by one thread at a time. If thread B tries to run the method while it is already being run by thread A, then thread B immediately receives a return value of None.

I wrote the following decorator to apply these semantics:

from threading import Lock

def non_blocking_lock(fn):
    fn.lock = Lock()

    @wraps(fn)
    def locker(*args, **kwargs):
        if fn.lock.acquire(False):
            try:
                return fn(*args, **kwargs)
            finally:
                fn.lock.release()

    return locker

This works in my testing so far. Does anyone notice any gotchas or have any suggestions for improvements?

Revised version

After the suggestions by @RemcoGerlich, I have added a docstring and kept the lock local to the decorator:

from threading import Lock

def non_blocking_lock(fn):
    """Decorator. Prevents the function from being called multiple times simultaneously.

    If thread A is executing the function and thread B attempts to call the
    function, thread B will immediately receive a return value of None instead.

    """
    lock = Lock()

    @wraps(fn)
    def locker(*args, **kwargs):
        if lock.acquire(False):
            try:
                return fn(*args, **kwargs)
            finally:
                lock.release()

    return locker

I think this will work fine and it's very close to how I would write it myself.

A few small things come to mind:

There's no documentation of any kind that explains what the semantics are, and they're not explicit either (the return None if the lock isn't acquired is entirely implicit). I would put the short explanation you put in this question into a docstring, and/or add an explicit else: return None to the if statement.
is there any reason why the lock object is exposed to the outside world by making it a property of the function (fn.lock) ? I would simply make it a local variable, so that it's hidden. But I'm not sure.


Bernoulli trials using a condition in a vectorized operation I was wondering how to vectorize the following code instead of using the for-loop and still be able to check for a condition.

# n is a numpy array which keeps the 'number of trials' parameter for the binomial distribution
# p is a numpy array which keeps the 'probability of success' parameter for the binomial distribution
# baf is a numpy array that will keep the results from the np.random.binomial() function

baf = np.repeat(np.nan, n.size)
for index, value in enumerate(n):
    if value >= 30:
        baf[index] = np.random.binomial(value, p[index]) / value

My own vectorized solution is:

baf = np.repeat(np.nan, n.size)
indices = np.where(n >= 30)[0]
baf[indices] = np.random.binomial(n[indices], p[indices]).astype(float) / n[indices]

However, I was wondering whether there are other more efficient solutions?

To initialize all entries of an array to the same value, np.repeat is probably not the fastest option, and definitely not what most people would expect to find. The more proper way would probably be:

baf = np.full((n.size,), fill_value=np.nan)

And for cases like this it is typically faster to use a boolean mask for indexing, skipping the call to np.where to extract the numerical indices. This should do the same faster:

mask = n >= 30
baf[mask] = np.random.binomial(n[mask], p[mask]).astype(float) / n[mask]


Calculating top position of elements based on their heights I have a data representing several elements (that could be rendered at some point as div), composed of y position (property top) and its height (property height).

These elements are always positioned vertically one next/on-top of each others based on their order.

At the moment I am using the following script to calculate their top position accordingly to their height, script works fine but I kindly ask you your revision in order to improve if possible:

Adjustment for speed as I would need interaction over thousand of objects.
Possibility of an alternative approach to solve this problem if you know any.

Example here, please look at the console for final result
http://jsbin.com/xamagidebe/1/

<html xmlns="http://www.w3.org/1999/xhtml">
<head>

    <title></title>
    <style>
    </style>
    <script>
        window.app = {
            data: [
                    { id: 'h', order: 0, top: null, height: 10 },
                    { id: 'c', order: 1, top: null, height: 20 },
                    { id: 'f', order: 2, top: null, height: 30 }
            ],
            start: function () {
                this.data.sort(function (a, b) {
                    if (b.order > a.order) {
                        return 1;
                    }
                });
                var len = this.data.length;
                var i = 0;
                var x = 0;
                var sum = 0;
                while (i < len) {
                    var item = this.data[i];
                    while (x < len) {
                        var itemX = this.data[x];
                        if (itemX.order < item.order) {
                            sum += itemX.height;
                        }
                        x++;
                    }
                    item.top = sum;
                    i++;
                    sum = 0;
                    x = 0;
                }
                this.data.sort(function (a, b) {
                    if (a.order > b.order) {
                        return 1;
                    }
                });
                console.log('after', this.data);
            }
        };

    </script>
</head>
<body onload="window.app.start();">

</body>
</html>

If you sort your elements ascending by order, you can get rid of the nested loop and just keep a running sum as you loop through:

start: function () {
    // sort ascending by order
    this.data.sort(function (a, b) {
        if (b.order < a.order) {
            return 1;
        }
    });
    var len = this.data.length;
    var i = 0;
    var sum = 0;
    while (i < len) {
        var item = this.data[i];

        // the top of this element is the sum of all previous element's heights
        item.top = sum;

        // don't reset sum here, just keep adding to it
        sum += item.height;
        i++;
    }
    console.log('after', this.data);
}

I have also removed the re-sorting at the end of the function as it is no longer needed due to the initial sort being ascending.


Maybe getting a character from a list of strings I'm trying to transition from JavaScript to PureScript (a Haskell spinoff that transpiles to JavaScript). For starters I coded this:

getAt :: [String] -> Number -> Number -> String
getAt strings x y = case strings !! y of
    Nothing -> " "
    Just line -> case charAt x line of
        Nothing -> " "
        Just char -> fromChar char

-- getAt ["abc", "def"] 0 1 -- returns "d"

It retrieves a character at position (x, y) from a "grid" of characters given as an arrays of strings. I can't help but think that the same function would be just return strings[y][x] in JavaScript assuming that (x, y) are always within bounds - which is true in my case. I can't get rid of these functions that return Maybe, but can I at least write this in a more concise way?

I haven't used PureScript, but in Haskell the most straightforward way to do this would be to leverage the Monad instance for Maybe and write your function using do-notation. From toying around with http://try.purescript.org/ I think this should work.

getAt :: [String] -> Number -> Number -> String
getAt strings x y = fromMaybe " " $ do line <- strings !! y
                                       char <- charAt x line
                                       return $ fromChar char

Note the use of fromMaybe, after you've done your computation in the Maybe monad, you get back out by specifying a "default" to use in the case that any value of Nothing cropped up, causing the whole do-block to return Nothing.


Find Missing Element in integer List Problem statement: Given an integer list from 0 to 100 find the missing Element.

I saw this problem on reddit on a compilation of interview questions, tried it out for fun. Mainly tried to generalize the code for any boundry(not just 100) and multiple missing arguments. Focused on readability and writing docstrings based on Google's recommendations. Did I hit my targets? How can and should my code be improved. Thanks!

"""
    Problem: Given a list from 0 to 100, find the missing element
"""

def makeIntArray(excludedValues, boundry):
    return [i for i in range(boundry + 1) if i not in excludedValues]

def compareToComplete(partial):
    """ Compares a possibly partial list to its complete version

        Args:
            partial: List containing integer type, all duplicates will be ignored

        Returns:
            Set of unique  missing elements or empty set if partial is complete

        example:
             partial = [1,2,3,4,6,8,10], complete = [0,1,2,3,4,5,6,7,8,9,10]
             returns: {0,5,7,9}
    """
    partial.sort()
    return {i for i in range(partial[-1] + 1)}.difference(set(partial))

don't change the input unless explicitly stated

partial.sort() works in-place. This might cause some nasty surprises for someone who uses this function and did not expect this.

The reason you sort this is also only to take the last element. max(partial) would suffice here. If partial is sorted, you can work with a different algorithm using the sortedness

set comprehension

a set comprehension is nice, but here a set(range(partial[-1] + 1)) would suffice

set.difference

there is no need to cast partial to a set

so the complete method reduces to

return set(range(max(partial) + 1)).difference(partial)

alternative method

If partial is sorted, you can work with 2 iterators

from itertools import count

def compare_to_complete_generator(partial):
    complete_iterator = count()
    partial_iterator = iter(partial)

    element_complete= next(complete_iterator)
    element_partial = next(partial_iterator)

    while True:
        while element_partial != element_complete:
            yield element_complete
            element_complete = next(complete_iterator)
        element_complete = next(complete_iterator)
        try:
            element_partial = next(partial_iterator)
        except StopIteration:
            return

list(compare_to_complete(partial))

[0, 5, 7, 9]

For large lists, this has the advantage that you don't have to look for the max and construct a set with max elements. This also accepts any sorted iterable or iterator, not just lists. if the input is not sorted, you can use sorted, but then you lose the advantages of this method.


Checking if a word chain is correct I have made a code to check if a word chain is correct or not.

The first input is the amount of words that will following
The first and last letter of every word should be the same
Therefore a chain can look like this:  Alaska - baobab - Cadillac - drievuldigheid -  Enigma -  fotografencollectief - Gasthuisberg

If the chain is incorrect the output should be the wrong word, else it should be 'word chain is correct'.

As you'll be thinking, this is an excercise for school. I have exam in a few days and I'm doing some excercises.

My code looks like this:

import string
words = int(input())
alphabet = string.ascii_lowercase
output = ''

for i in range(words):
    woord = input()

    #for the first word
    if i == 0:
        #make that the alpabet last letter is the starting letter of the word
        alphabet = alphabet[alphabet.index(woord[0].lower())+1:] + alphabet[0:alphabet.index(woord[0].lower())+1]

    #check if the last letter of the word is equal to the first  letter
    #check if the last letter of the word is equal to the last letter of the alphabet
    if woord[0].lower() == woord[-1].lower() and alphabet[-1] == woord[0].lower():
        #if so: move the alphabet one Index
        alphabet = alphabet[1:] + alphabet[:1]

    elif output == '':
        output = woord

if output == '':
    print('word chain is correct')
else:
    print(output)

I'm wondering if there is some room for improvement, as it looks my code isn't pythonic at all.

A few remarks:

You can do the .lower() one time by using woord = input().lower()
You could check if they are in alphabetic order by keeping track of the    previous one like this:

previouschar=''
for s in ('aaa', 'bbb', 'ddd', 'efe', 'geg'):
    if previouschar != '' and ord(s[0]) != ord(previouschar) + 1:
        print('Out of order')
    previouschar = s[0]


Insert into two tables during user registration using PHP I'm working on creating a user registration script in PHP.  I have two tables that need to be inserted into.  One that stores some general user information, and another that stores their login credentials.  My issue is that my code to perform these insert is a mess of if/else blocks and I'm trying to figure out a good way to condense this down into a logical and clean code block.

Note that for now I have just placed some echo's that will later be replaced by a real error handling system once I truly know how many paths I'm going to need.

Can someone please review this script and let me know some improvements I can make to it?  I know it's quite ugly right now.

$mysqli = getMysqlConnection();
  if ($stmt = $mysqli->prepare("INSERT INTO users (username, email, regtime, emailverified, type) values (?,?,?,?,?);")) {
      date_default_timezone_set("America/New_York");
      $dateStr = date("m-d-Y h:i:s");
      $emailverified = 0;
      $type = 0;
      $rc = $stmt->bind_param('sssii', $username, $email, $dateStr, $emailverified, $type);
      if($rc === true){
        $rc = $stmt->execute();
        if($rc === true)
        {
          if ($stmt = $mysqli->prepare("INSERT INTO usercreds (username, hash) VALUES (?, ?);")) {
              $rc = $stmt->bind_param('ss', $username, $hash);
              if($rc === true){
                $rc = $stmt->execute();
                if($rc !== true)
                {
                  echo "so close...";
                }
              }
              else
              {
                echo "well, shit...";
              }
          }
          else {
            {
              echo "whoops...";
            }
          }
        }
        else {
          echo "noooooooooo!";
        }
      }
  }
  else {
    echo "uh oh";
  }

  echo "done";

You should return early to reduce nesting and make it more obvious when what values are returned:

if (!$stmt = $mysqli->prepare("INSERT INTO users (username, email, regtime, emailverified, type) values (?,?,?,?,?);")) {
    echo "uh oh";
}
[...]

if($stmt->bind_param('sssii', $username, $email, $dateStr, $emailverified, $type) !== true){
    // you actually didn't have an else for this
}
[etc]

You should also replace your echoes with exceptions with proper error messages.

Misc

your indentation is off, and you should use a minimum of 4 spaces.
rc is not a good variable name, and you shouldn't reuse variables for different things.
you should also extract this into two functions: insertUser($username, $email, ...) and insertUserCredentials($username, $hash).


Code Challenge: basic arithmic operations using only incrementation operator Challenge

  Write a function that takes 2 integer operands left and right
  (both >= 0) and evaluates one of the following basic arithmic
  operations:

  addition
  multiplication
  exponentation

  The challenge is you are prohibited from using any built-in operator,
  except for an incrementation (++ or +1). The entropy is all
  results that are in bounds of the maximum value of an integer.

Provided Classes

public enum Operation
{
    Addition,
    Multiplication,
    Exponentation
}

public static class Expression
{
    public static int Evaluate(int left, int right, Operation operation)
    {
        throw new NotImplementedException(); // .. this should be implemented
    }
}

Unit Tests

[TestClass]
public class Fixtures
{
    [TestMethod]
    public void Fixture()
    {
        Assert.AreEqual(5, Expression.Evaluate(2, 3, Operation.Addition));
        Assert.AreEqual(6, Expression.Evaluate(2, 3, Operation.Multiplication));
        Assert.AreEqual(8, Expression.Evaluate(2, 3, Operation.Exponentation));
    }
}

My Solution

I have decided to use the hyperoperation (Wikipedia Link). The only arithmic operator used is in the first branch (if (n == 0) -> b + 1).

  $$ H_n(a,b) = a[n]b =
 \begin{cases}
                       b + 1 & \text{if } n = 0 \\
                           a & \text{if } n = 1 \text{ and } b = 0 \\
                           0 & \text{if } n = 2 \text{ and } b = 0 \\
                           1 & \text{if } n \ge 3 \text{ and } b = 0 \\
   H_{n-1}(a, H_n(a, b - 1)) & \text{otherwise}
 \end{cases}$$

And my implementation of Evaluate:

public static class Expression
{
    public static int Evaluate(int left, int right, Operation operation)
    {
        return Hyper(left, (int)operation + 1, right);
    }

    static int Hyper(int a, int n, int b)
    {
        // https://en.wikipedia.org/wiki/Hyperoperation
        // a = left operand   (a >= 0)
        // n = hyper operator (n >= 0)
        // b = right operand  (b >= 0)

        if (a < 0) throw new ArgumentOutOfRangeException(nameof(a));
        if (b < 0) throw new ArgumentOutOfRangeException(nameof(b));
        if (n < 0) throw new ArgumentOutOfRangeException(nameof(n));
        if (n == 0) return b + 1;
        if (b == 0) return n == 1 ? a : n == 2 ? 0 : 1;

        return Hyper(a, n - 1, Hyper(a, n, b - 1));
    }
}

Questions

Are there any overlooked flaws in my solution?
Could this be optimized for performance?
Are there perhaps better alternatives, and why would they be better?
Any general review of my code, style, conventions are invited.

public enum Operation
{
    Addition,
    Multiplication,
    Exponentation
}

If you gave these explicit values:

  public enum Operation
  {
    Addition = 1,
    Multiplication = 2,
    Exponentation = 3
  }

... you could avoid the + 1 for the operation:

  return Hyper(left, (int)operation, right);

Because Hyper(...) is private, and you are supposed to know how to use it, there is no need for checking the input values in there. It is safe to leave it to Evaluate(...) to do that:

public static int Evaluate(int left, int right, Operation operation)
{
  if (left < 0) throw new ArgumentOutOfRangeException(nameof(left));
  if (right < 0) throw new ArgumentOutOfRangeException(nameof(right));
  if (!Enum.IsDefined(typeof(Operation), operation)) throw new ArgumentOutOfRangeException(nameof(operation));

  return Hyper(left, (int)operation, right);
}

it is safe because:

  if (n == 0) return b + 1;
  if (b == 0) return n == 1 ? a : n == 2 ? 0 : 1;

will catch zeros for n and b while a never changes so no arguments will never be lesser than zero - as you can only decrement by one.

Personally I would change the order of b and n to be the same as for Evaluate(...):

static int Hyper(int a, int b, int n) {...}

All in all in my writing it would look like:

  public enum Operation
  {
    Addition = 1,
    Multiplication = 2,
    Exponentation = 3
  }

  public static class Expression
  {
    public static int Evaluate(int left, int right, Operation operation)
    {
      if (left < 0) throw new ArgumentOutOfRangeException(nameof(left));
      if (right < 0) throw new ArgumentOutOfRangeException(nameof(right));
      if (!Enum.IsDefined(typeof(Operation), operation)) throw new ArgumentOutOfRangeException(nameof(operation));

      return Hyper(left, right, (int)operation);
    }

    static int Hyper(int a, int b, int n)
    {
      if (n == 0) return b + 1;
      if (b == 0) return n == 1 ? a : n == 2 ? 0 : 1;

      return Hyper(a, Hyper(a, b - 1, n), n - 1);
    }
  }

More edge/test cases?

BtW Hyper() stack overflows relatively quickly: for instance for a == 8 and b == 5 for Exponentation.

An alternative algorithm could be the below "semi-recurs-iterative":

It has a recursive depth of max 3:

public static int HyperIter(int a, int b, int n)
{
  if (n == 1)
  {
    while (b > 0)
    {
      a++;
      b--;
    }

    return a;
  }
  else
  {
    int res = n - 1;
    res--;
    while (b > 0)
    {
      res = HyperIter(res, a, n - 1);
      b--;
    }

    return res;
  }
}

I'm though not sure if it completely satisfies the criteria of only using + 1, because it uses a local variable (res) and -- - but the original also uses - 1 in the recursive calls, so...?


Design a O(1) data structure How would I implement a data structure that supports the following in constant time? I got this question at a job interview, and the following is my solution. Please check my approach, or suggest a better alternate approach, if you have one.

////////////////////////////////////////////////////////////////
// Implement a container that have the following methods:
// Java:
//     public class Container<E>
//     {
//         // data memebers
//         ...
//         public Container();
//             // constructor
//             // Complexity: Constant
//
//         public int size();
//              // return the total number of elements in the container
//              // Complexity: Constant
//
//         public E get(int index);
//              // get an element from the container by its index
//              // Complexity: Constant
//
//         public void set(int index, E element);
//              // set an element in the container by its index. index >= 0 && index < size()
//              // Complexity: Constant
//
//         public void add_front (E element);
//              // add a new element to the front of the container i.e. its index is 0
//              // Complexity: Constant (amortized time)
//
//         public void remove_front ();
//              // remove the element at the front of the container
//              // Complexity: Constant
//
//         public void add_back (E element);
//              // add a new element to the back of the container i.e. its index is size()
//              // Complexity: Constant (amortized time)
//
//         public void remove_back ();
//              // remove the element at the back of the container
//              // Complexity: Constant
//     }
//
// Examples:
//   at beginning   => []

//   add_front(0)   => [0]
//   add_front(-1)  => [-1, 0]
//   add_back(1)    => [-1, 0, 1]
//   add_back(2)    => [-1, 0, 1, 2]
//   get(0)         => -1
//   get(3)         => 2
//   set(3, 8)      => [-1, 0, 1, 8]
//   remove_front() => [0, 1, 8]
//   remove_back()  => [0, 1]
////////////////////////////////////////////////////////////////

My approach

Use a Hashtable for storing values:

Hashtable<Integer, Integer> storage = new Hashtable<>();

Have two variable called front and back, which signify the start and end of the data structure's storage range.

add_front(E element)

low--;
storage.put(low, element);

add_back(E element)

high++; storage.put(high, element);

public void remove_front ();

low++;

public void remove_back ();

high--;

public E get(int index);

index = index-low; return storage.get(index);

public void set(int index, E element);

index = index-low; storage.put(index, element);

public int size();

return high-low+1;

You can use the union of two different data structures to have an O(1) data structure (thinking in terms of # of operations):

You need random access to element K in constant time (get and set), a hash table is a good candidate for this.
You want constant time inserts and deletes at the front and back of the structure (add_front, remove_back, etc.). This is what a deque is good for (double-ended queue).

So you can use both structures at the same time, and you'll solve some operations using one or the other structure. The key part is how to keep them synchronized in constant time, and the answer to that is: you only insert/remove elements using the deque, so once you insert/remove an element in the deque (which is O(1)) you go and insert/remove from the hash table too (which is also O(1)).

Some considerations:

This solution uses O(n) memory, but that is not a restriction of the statement of the problem.
A hash table's insert isn't actually O(1), they call it "amortized constant time" because at times the hash table needs to resize its internal storage to accommodate for the new element, which takes O(n) time.
A hash table's get/set is not O(1) either, it depends on the hash function and your elements (worst case scenario all elements fall in the same bucket and you have O(n) search time).


Circular Buffer C++ Implementation I was asked this question in an interview, and didn't go through. I know, it needs some improvements, but probably I am missing something big? I would highly appreciate if someone can review it for me, so that I can get to know what I need to work on.

The problem is to complete this skeleton class to implement a circular buffer. I could have done the C++ and header in a different source, but they insisted on doing it all here, assuming it is not part of a big project.

class CircularBuffer {
public:
    CircularBuffer(int size)
        : data_(size, 0)
        , next_(0)
        , head_(-1)
    {
    }

    ~CircularBuffer()
    {
    }

    bool empty()
    {
        return size() == 0;
    }

    int size()
    {
        // TODO: check if head is valid.
        // TODO: check emptiness properly.
        int sz = 0;
        if (head_ >= 0)
        {
            if (next_ > head_)
                sz = next_ - head_;
            else
                sz = next_ + (data_.size() - head_);
        }
        return sz;
    }

    void push(char val)
    {
        // TODO: check if circularbuffer is full.
        if( size() == data_.size())
        {
            std::cout<<"Buffer is full, can't push anything"<< std::endl;
            return;
        }
        data_[next_++] = val;
        if(head_ < 0)
            head_ = 0;
        if (next_ == data_.size())
            next_ = 0;

        displayBuffer();
    }

    char pop()
    {
        // TODO: check if head_ is valid.
        if (head_ < 0)
        {
            std::cout<<"Empty Buffer, nothing to pop" << std::endl;
            return -1;
        }

        // TODO: adjust head_ after pop.
        char popVal = data_[head_];
        data_[head_++] = 0; // Clean the popped memory- display   purpose

        //Wrap around head_ move
        if (head_ == data_.size())
            head_ = 0;

        // Check if the buffer is empty now?
        if (head_ == next_)
        {
            head_ = -1; //Buffer is empty now
            next_ = 0;
        }

        return popVal;
    }

    void displayBuffer()
    {
        std::cout << std::endl;
        for (auto c: data_)
        {
            std::cout << c << ' ';
        }
        std::cout << "  |   HEAD = " << head_ << "  NEXT = " << next_ << std::endl;
    }
private:
    std::vector<char> data_;
    int next_;
    int head_;
};

#include

The code was in error until I prefixed it with

#include <vector>
#include <iostream>

~CircularBuffer()

There's no need to define a destructor that does nothing - just let the compiler generate a default one.

If you wanted this to be a base class, you would provide a virtual destructor, but you can explicitly default it:

virtual ~CircularBuffer() = default;

I'd say that you are right to omit copy constructor and assignment operator, as the compiler-provided ones will work correctly.

empty()

The reason containers have empty() is so that it can be quicker than testing size is non-zero.  So implementing empty() in terms of size() should be a last resort.  In this case, we set head negative to indicate an empty buffer, so we can just test that:

bool empty() const
{
    return head < 0;
}

Note that I've made this a const method, as we may want to call it on a buffer we don't intend to modify.

size()

There's no need for the sz variable, as we set it only to return immediately:

size_t size() const
{
    if (head < 0)
        return 0;
    if (next > head)
        return next - head;
    return next + (data.size() - head);
}

I've made this const, too, and changed the return type to be more in line with what users will expect.  If I were to be really pedantic, I'd consider std::vector<char>::size_type, and adjust the head and next members accordingly.

You could write a single return with a tertiary expression here, but I think the if ladder is slightly clearer - that's obviously a judgement call.

push()

As in empty(), it's inefficient to call size() when we just want to know whether the container is full.  Instead, we can just test next == head.  If you must produce output, send it to std::cerr, not std::cout!  Generally, in a library, it's better to report the error to the caller (by exceptions, or perhaps as a return value) rather than to output diagnostics ourselves, but I'll let that stand for now.

It wasn't obvious at first reading that if head was negative, then we could depend on next being zero.  I would probably not made that assumption, and written something like:

    data[next] = val;
    if (head < 0)
        head = next;
    if (++next == data.size())
        next = 0;

pop()

Most of the critique of push() applies here, so I won't repeat it.  It's not clear why -1 is chosen as the value to return on failure - was this specified in the requirements?

main()

You provide no main() function, which is a shame - it would have been nice to use your unit tests when refactoring.

Modified version

I changed the index types to size_t, and defined a constant to indicate empty (as size_t doesn't have negative values).

#include <vector>
#include <stdexcept>

class CircularBuffer
{
    static const size_t EMPTY = ~0u;

    std::vector<char> data;
    size_t next = 0;               // index at which next insert will occur
    size_t head = EMPTY;           // index of next pop, or EMPTY marker

public:
    CircularBuffer(size_t size)
        : data(size)
    {
    }

    virtual ~CircularBuffer() = default;

    bool empty() const
    {
        return head == EMPTY;
    }

    size_t size() const
    {
        if (empty())
            return 0;
        if (next > head)
            return next - head;
        return data.size() - head + next;
    }

    void push(char val)
    {
        if (next == head)
            throw std::logic_error("Buffer overrun");

        data[next] = val;
        if (empty())
            head = next;
        if (++next == data.size())
            next = 0;
    }

    char pop()
    {
        if (empty())
            throw std::logic_error("Buffer underrun");

        const char popVal = data[head];

        if (++head == data.size())
            head = 0;

        if (head == next)
            head = EMPTY;

        return popVal;
    }
};

Future directions

If you want to set yourself some more challenges, I'd recommend looking at the standard containers (especially std::queue) and seeing how you can make the interface look more like theirs.  Implement front() and back() (easy!), emplace(), and some iterators.

Also, you might want to make this a template class, so it can hold any kind of element, not just char.


Fetching AJAX results from three endpoints, with error handling I have a requirement in which I need to get data by making AJAX calls. First search for data in endPoint1, if not found then in endPoint2, if not found then in endPoint3. But if any of the endPoint returns data, then there is no need to make further REST calls.

I have achieved the functionality, but the code looks spaghetti.

If possible, please suggest a better approach to reduce this call indentation.

function myAjaxFunction(endPoint) {
    // returns promise
}

function checkForData() {
    myAjaxFunction(endPoint1)
        .then(function (data) {
            if (data.d.results.length > 0) { return "Data found"; }
            else {
                myAjaxFunction(endPoint2)
                    .then(function (data) {
                        if (data.d.results.length > 0) { return "Data found"; }
                        else {
                            myAjaxFunction(endPoint3)
                                .then(function (data) {
                                    if (data.d.results.length > 0) { return "Data found"; }
                                    else {
                                        console.info("sorry data not exist");
                                    }
                                }, function (e) {
                                    console.error()
                                });
                        }
                    }, function (e) {
                        console.error()
                    });
            }
        },
        function (data) {
            if (data.d.results.length > 0) { return "Data found"; }
            else {
                return "not found.";
            }
        });
}

I would suggest passing in an array of endpoints so you can have one loop that calls an endpoint and looks at the result without repeating any code:

function checkForData(endPoints) {
    let i = 0;

    function next() {
        if (i < endPoints.length) {
            return myAjaxFunction(endPoints[i++]).then(function(data) {
                if (data && data.d && data.d.results && data.d.results.length > 0) {
                    return data.d.results;
                } else {
                    return next();
                }
            });
        } else {
            return null;
        }
    }

    return Promise.resolve().then(next);
}

checkForData([endPoint1, endPoint2, endPoint3]).then(function(result) {
    // result here
    if (result) {
        console.log(result);
    } else {
        console.log("no data found");
    }
}).catch(function(err) {
    // error here
});

Note: I also put .then() on the same line as its parent function to avoid an extra level of indent.


Update URL with new parameter value in controller action I have an action that takes an id as a parameter and gets the corresponding item with the help of a web service. The web service's method that returns the item will either return the item with the requested id, or will create a new item (new version of the item) with a new id. So, in the case that a new item is created, the response's header should be updated with the new item's id.

Here's what the route corresponding to that controller action looks like:

public override void RegisterArea(AreaRegistrationContext context)
{
    context.MapRoute(
        "default",
        "MyArea/{Controller}/{action}/{id}",
        new { controller = "MyController", action = "Index", id = UrlParameter.Optional }
    );
}

So when an user requests the item with the id 12, the request's path will look like:

  MyArea/MyController/MyAction/12

Let's say the web service creates a new version of the above item, and its id is 13, I'd like to update the URL accordingly. Here's what my action looks like:

[HttpGet]
public async Task<ActionResult> EditItem(int id)
{
    try
    {
        Item myItem = await _myWS.Channel.GetItemAsync(id);

        // If a new version has been created, change the url in order to show the new id
        if (myItem.Id != id)
        {
            Response.StatusCode = 301;
            string newUrl = Request.Path;
            int lastSlashIndex = newUrl.LastIndexOf("/");
            if (lastSlashIndex > 0)
                newUrl = newUrl.Substring(0, lastSlashIndex + 1) + myItem.Id;

            Response.AddHeader("Location", newUrl);
            Response.End();
        }

        return View("MyView", (ItemViewModel)myItem);
    }
    catch (Exception ex)
    {
        throw new ApplicationException(ex.Message);
    }
}

I was wondering if there was a better way to reconstruct the URL with the updated id. I also tried reconstructing the URL with the Request.RequestContext.RouteData.Values, but I thought the code was less readable and more complicated, but I don't like the string replace neither, because I think it's less long term friendly.

Bonus

The reason I reconstructed the URL this way instead of a simple redirection it's because I thought the way I was doing avoided a second call to the action, but I was wrong. Is there a "hack" to avoid redirection and to just update the shown URL?

I believe you want RedirectToActionPermanent:

if (myItem.Id != id)
{
    return RedirectToActionPermanent("EditItem", new { id = id })
}


Listing an optional Python dependency in setup.py I have a small project that requires a third-party module that's rarely updated (once every other year or so) and not maintained by any package management system. The context, here, is cognitive science & research, the third party module is for linking proprietary hardware to custom scripting, in the house project, potentially, may end up seeing wider (eventually open source) use within cog. psych so I want to plan for distribution.

Can anyone poke holes in or thumbs up this approach:

#!/usr/bin/env python

from distutils.core import setup

try:
    import third_party_package
    install_packages = ['in_house_package']
except ImportError:
    print "Warning: 'third_party_package' not found in PYTHON_PATH, installing version in_house_package v. <VERSION>"
    install_packages = ['in_house_package', 'in_house_package.third_party_package']

setup(
    name='InHousePackage',
    version = '0.1',
    description = 'A framework for building psychological experiments in Python',
    author = 'me',
    author_email = 'my_address',
    url = 'a local git',
    packages=install_packages,
    requires = ['numpy']
    )

A few small comments:

The error message looks wrong to me.

This is what the logic looks like to me:

if (third party package is installed):
    install(in-house package)
else:
    install(in-house package)
    install(in-house variant of third-party package)

but the printed warning tells me that the in-house package isn’t installed. Shouldn’t it tell me the third-party package isn’t installed?
An ImportError doesn’t necessarily mean a module isn’t installed.

It just indicates any error when the module was imported. For example, if I have a file foo.py that contains:

import bar

and no module bar.py, I get an ImportError when trying to import foo, even though the foo file exists:

>>> import foo
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "foo.py", line 1, in <module>
    import bar
ImportError: No module named bar

You can jump through hoops with import tools, or check the error message of the ImportError. That may or may not be something worth worrying about; just remember that this edge case is there.
Trivial nitpicks.

I’m sure this is just an artefact of posting on CR, and it isn’t actually called in_house_package, but you’ve misspelt it in the error message.
When specifying keyword arguments to a function (such as setup()), it’s common to omit the spaces around the equals sign.