Utility class for an HttpURLConnection I am creating a utility class to do an HttpURLConnection. I want to make this class generic as in I pass the URL and the POST params. It works with any range of these two values.

public class HttpUrlConnection {

    String url, params;

    public HttpUrlConnection(String url, String params) {
        this.url = url;
        this.params = params;

        try {
            sendPost();
        }catch(Exception e){
            System.out.println("Exception"+e);
        }
    }

    private String sendPost() throws Exception {

        String USER_AGENT = "Mozilla/5.0";
        URL obj = new URL(url);

        HttpURLConnection con = (HttpURLConnection) obj.openConnection();

        //add reuqest header
        con.setRequestMethod("POST");
        con.setRequestProperty("User-Agent", USER_AGENT);
        con.setRequestProperty("Content-Type", "application/x-www-form-urlencoded");

        String urlParameters = params;
        // Send post request
        con.setDoOutput(true);
        DataOutputStream wr = new DataOutputStream(con.getOutputStream());
        wr.writeBytes(urlParameters);
        wr.flush();
        wr.close();

        int responseCode = con.getResponseCode();
        System.out.println("\nSending 'POST' request to URL : " + url);
        System.out.println("Post parameters : " + urlParameters);
        System.out.println("Response Code : " + responseCode);

        BufferedReader in = new BufferedReader(
                new InputStreamReader(con.getInputStream()));
        String inputLine;
        StringBuffer response = new StringBuffer();

        while ((inputLine = in.readLine()) != null) {
            response.append(inputLine);
        }
        in.close();

        //print result
        System.out.println("rvsp" + response.toString());

        return response.toString();
    }

}

I am unsure if I am following the best practice above.

Should I keep the cosntructor short as:

public HttpUrlConnection(String url, String params) {
        this.url = url;
        this.params = params;
    }

and start the POST using connection.sendPost where in some other class I do:

HttpUrlConnection connection;
connection = new HttpUrlConnection(url,params);
connection.sendPost();

If you would pass in your paramaters at sendPost(), you could reuse your created object for other URLs and Parameters, and would not need a specific constructor.

Generally, calling a Method in a constructor is defying the purpose of a constructor - to initialize an object. When rereading your code 2 months later, you would be wondering, where you'd be calling the actual POST.

Clean Code wise, it's generally a bad sign if you create subsections in your method using comments, such as //add reuqest header. Create seperate private methods in your class just for doing that. Your code will be more readable and reusable.

Exceptions; use them! Throw detailed Exceptions and declare them as such. So you can react in your other code to that exception.

String USER_AGENT = "Mozilla/5.0"; this should be either declared as private static final in your class, because when introducing other methods such as sendGet you'd be using the same variable again.

Generally, don't use System.out.println other than in very early stages of your code. Create a Response class, where you set the things such as the response code or others and return this, don't just write it to the console, you can't use it and your console is going to be littered with such output everywhere in your program, which makes it difficult to debug.

You could generalize the method creating your params String as well, so you could pass in an arbirtrary amount of parameters and it would serialize these parameters just right.


String length calculation implementation in Assembly (NASM) I just made a working implementation of calculating a string's length, but I'm not exactly sure it's the most efficient way. I divide the string up in blocks of four bytes each (32 bits), and add 4 to EAX. Once I hit a completely NULL block, I roll back the count of EAX by 4 bytes, and then calculate the final block's size one byte at a time. It only works using ASCII characters, and is limited to 32 bit computing (though that can be easily upgraded to 64 bit soon). I'm using assembly on a Linux maching using NASM. Here's the code:

; Function to compute the length of a string in blocks
; this only works in ASCII, or else it may give the wrong character count.
; input register: EAX - the string

section .data

    BYTE_COUNT equ 4        ;4 bytes = 32 bits
    NULL_TERMINATOR equ 0 ;\0, aka 0

section .text

    global  _strlen ;main entry point

        _strlen:

            push ebp             ; c calling convention: preserve ebp
            mov     ebx, eax  ; mov to ebx the string
            jmp _NextBlock ; go to _NextChar

        _NextBlock:
            cmp     word [eax], NULL_TERMINATOR ; Compare whether or not this block is completely null (e.g. 0)
            jz      _RollbackFinalBlock ; if it is, jump to _RollbackFinalBlock
            ; if not...
            add eax, BYTE_COUNT ; Add to EAX the block size
            jmp     _NextBlock; repeat loop

        _RollbackFinalBlock:

            sub eax, BYTE_COUNT ;Subtract the block size from EAX, because it is null
            jmp _FinalizeFinalBlock ; go to _FinalizeFinalBlock

        _FinalizeFinalBlock:
            cmp     byte[eax], NULL_TERMINATOR ;Compare the characters of the final block to the null byte.
            jz      _Exit ;if this byte is null, proceed to _Exit
            ; if not...
            inc eax ;increment EAX
            jmp     _FinalizeFinalBlock ;repeat loop

        _Exit:

            dec eax ; We will have a null terminator at the end. remove it.
            sub     eax, ebx ; compute difference to get answer, and store result in EAX
            pop     ebp ; load preserved EBP value from the stack
            ret             ; exit this function

You can test this function by linking it with a c program, and calling it from there.

The problem I have is that I have an extra jump to _RollbackFinalBlock and _FinalizeFinalBlock. Is there a way to make this implementation faster and more efficient?

I have a number of comments that I hope you find helpful.

Code comments

First, your code is generally well commented and I had little difficulty in understanding what the code was doing or why.  That's a good thing.  A few minor points, though.  First, your comment for the overall function says

; input register: EAX - the string

however, it's probably useful to point out that EAX is actually a pointer to the string and isn't intended to contain a whole string.

Also, comments should tell something not obvious about the code.  So this:

        jmp     _NextBlock; repeat loop

is arguably OK, but this:

        inc eax ;increment EAX

is not really helpful.  Instead you should say why the code is incrementing EAX rather than simply repeating what the instruction does.

Structure

Overall, the structure was OK, but the line

        jmp _NextBlock ; go to _NextChar

near the top of the function is not needed, since _NextBlock is the next instruction anyway.  The same is true of the jmp just before _FinalizeFinalBlock.  Generally speaking, you should strive to eliminate unconditional jumps.  So for example, your code includes this:

    _FinalizeFinalBlock:
        cmp     byte[eax], NULL_TERMINATOR ;Compare the characters of the final block to the null byte.
        jz      _Exit ;if this byte is null, proceed to _Exit
        ; if not...
        inc eax ;increment EAX
        jmp     _FinalizeFinalBlock ;repeat loop

    _Exit:

But you could easily restructure that to eliminate the unconditional jmp like this:

        dec eax ;
    _FinalizeFinalBlock:
        inc eax ;increment EAX
        cmp     byte[eax], NULL_TERMINATOR ;Compare the characters of the final block to the null byte.
        jnz     _FinalizeFinalBlock ; keep going until final block is NULL

    _Exit:

Algorithm

The most significant problem I see with the code is the algorithm that relies on strings having multiple trailing NUL bytes.  This may be true for code that calls this function, but it's not generally true of C strings which typically end with only a single NUL character.

Instruction set use

The repnz scasb instruction is probably going to be a lot more efficient that the loop you've constructed.  I'd recommend reading about it and then incorporating that into your code.

Register use

The ebx register is destroyed and not saved by the function.  This may be fine for your purposes, but it should be documented in comments at the top of the function.

Also, the code doesn't actually alter ebp so there isn't any point to pushing it onto the stack and popping it off at the end of the routine. (Maybe you meant ebx?)

Update:

On my machine (quad core running 3.2GHz under 64-bit Linux) I found this version to be slightly (15%) faster than the corresponding repnz scasb version:

strlen2:
    mov edx, [esp + 4]
    xor eax,eax ; count = 0
    jmp overit
looptop:
    inc edx
    inc eax
overit:
    cmp byte[edx],0
    jnz looptop
    ret


Sqlite.swift -- Selecting all rows and returning them in an array of tuples Link to SQLite wrapper

func queryAll() -> [(String, String, String)]  {
    do {
        let sql = try DB?.prepare("SELECT city, zip , temp FROM weather")
        var arr : [(city : String, zip : String, temp : String)] = []
        for row in sql! {
            arr += [(city: row[0] as! String, zip: row[1] as! String, temp: row[2] as! String)]
        }
        return arr;
    }
    catch {
        print("\(error)")
    }
    return [("","","")]
}

Grabbing all the rows, and returning them as a tuple for later use.
I've read the docs but I cannot think of a more efficient way to return all the rows to a TableView

override func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    let cell : CustomTableViewController = self.tableView.dequeueReusableCell(withIdentifier: "Cell")! as! CustomTableViewController
    let DB : WeatherDataStorage = WeatherDataStorage()
    var arr = [(city : String, zip : String, temp : String)]()
    arr = DB.queryAll()
    cell.cityLabel.text = arr[i].city
    cell.zipLabel.text = arr[i].zip
    cell.temperatureLabel.text = arr[i].temp
    i += 1
    return cell
}

EDIT: GIST

Don't use tuples like this. The purpose of tuples is to make simple, short lived collections of values. For a use case like this, it is preferable to use a struct or a class to contain your results. There are many disadvantages.
Don't return non-sensical default values as a way of error handling. "" isn't a city, and "" isn't a valid zip code, so why try to claim them as such? This is what optionals are for, use them.
Narrow your do block to contain only the function calls that throw.
Avoid force casting if you can. Handle your errors sensibly.
Currently, you manually create an empty array, and repeatedly add to it in a for loop. This is discouraged for several reasons:

arr must be mutable.
It's a lot of boiler-plate code.
You can incur a lot of reallocation overhead, because you didn't call reserveCapacity on your array.

Using map is preferable instead.
Improve your naming. sql is a result set, it's not an sql code snippet, nor is it an sql connection. Name it so.
Don't use the pattern of: array += [newElement]. This technique causes an unnecessary array allocation. Instead, just use array.append(newElement).
Change your DB variable to not be an Optional. If there is something like a connection issue that causes DB to be nil, this should be handled at the time that you attempt to open the connection. Leaving it as an Optional, and using optional chaining DB?.doSomething() is a terrible idea. It makes the nil case fail silently, leading to very annoying to catch bugs.
"weather" is an awkward name for a table. Try something like "WeatherReading"

Here's how I would write this code:

struct WeatherReading {
    let city: String
    let zipCode: String
    let temperature: String
}

func queryAllWeatherReadings() -> [WeatherReading]?  {
    guard let queryResults = try? DB.prepare("SELECT city, zip, temperature FROM WeatherReading") else {
        debugPrint(error)
        return nil
    }

    let weatherReadings = queryResults.map{ row in
        guard let city = row[0] as? String,
              let zipCode = row[1] as? String,
              let temperature = row[2] as? String else {
            fatalError("There was a type error in the received result set: \(row)")
        }
        return WeatherReading(city: city, zipCode: zipCode, temperature: r temperature)
    }

    return weatherReadings
}


Verifying the Entity Data Model schema against a production database We have an entity data model (database first) for a legacy product we are migrating to .Net from Delphi. We recently deployed to a server where some of the tables had been modified and columns deleted. The results were catastrophic. I'm working on a way of validating the EDM schema against the live SQL server database.

public Validation(ADbContext db)
{
    _connectionString = db.Database.Connection.ConnectionString;
}

private readonly string _connectionString;

public ILookup<string, List<string>> Run()
{
    // A tolerance to deal with Entity Framework renaming
    var modelValidation = GetModelProperties<ADbContext>(tolerance);
    var isValid = !modelValidation.Any(v => v.Any(x => x.Count > 0));
    if (!isValid)
        Logger.Activity(BuildMessage(modelValidation));
    return modelValidation;
}

public string BuildMessage(ILookup<string, List<string>> modelValidation)
{
    // build a message to be logged
}

public List<string> GetMissingColumns(IEnumerable<string> props, IEnumerable<string> columns, int tolerance)
{
    // compare whether the entity properties have corresponding columns in the database
    var missing = props.Where(p => !columns.Any(c => p.StartsWith(c) && Math.Abs(c.Length - p.Length) <= tolerance)).ToList();
    return missing;
}

public string[] GetSQLColumnNames(Type t)
{
    SqlConnection connection = new SqlConnection(_connectionString);
    var table = t.Name;
    DynamicParameters dparams = new DynamicParameters();
    dparams.Add("Table", table);
    var query = "SELECT COLUMN_NAME FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_NAME = @Table ";
    // Using dapper to retrieve list of columns from that table
    List<string> columns = connection.Query<string>(query, dparams).ToList();
    return columns.ToArray();
}

static string[] GetEntityPropertyNames(Type t)
{
    var properties = t.GetProperties(BindingFlags.Instance | BindingFlags.Public)
            .Where(p => p.CanRead && !p.PropertyType.FullName.Contains("My.Namespace") && !p.PropertyType.FullName.Contains("Collection"))
            .Select(p => p.Name)
            .ToArray();
    // these conditions excludes navigation properties: !p.PropertyType.FullName.Contains("My.Namespace") && !p.PropertyType.FullName.Contains("Collection")
    return properties;
}

ILookup<string, List<string>> GetModelProperties<T>(int tolerance, T source = default(T))
{
    var properties = typeof(T).GetProperties(BindingFlags.Instance | BindingFlags.Public)
            .Where(p => p.PropertyType.IsGenericType)
            .Select(p => p.PropertyType.GetGenericArguments()[0])
            .Select(p => new
            {
                Entity = p.Name,
                Properties = GetEntityPropertyNames(p),
                Columns = GetSQLColumnNames(p),
            })
            .ToArray();

    return properties.ToLookup(p => p.Entity, p => GetMissingColumns(p.Properties, p.Columns, tolerance));
}

Is there a better way?
Does Entity Framework already provide a mechanism for achieving this?
Would this be better in the data layer or in another assembly?
Is adding Dapper (just for this purpose) overkill, does it add unnecessary complexity to the project?

Update
I've created a Github repository with a library I've been using that implements this. To minimise dependencies, I've used reflection in favour of Entity specific features.

Is there a better way?
  Does Entity Framework already provide a mechanism for achieving this?

In fact this is one question. If EF would provide a mechanism, undoubtedly that would be better, because you'd know for sure it yields something that EF is happy with. Unfortunately, no, EF hasn't got such a mechanism.

As far as EF checks the consistency of the database with an existing model, it only compares a previously stored model hash in the database with the current model hash. It never compares actual database objects. That would take far too much time in checking whether a migration should be applied. And then, all these methods are internal.

There is however a shorter and more reliable way to get entity and property names from a context. More reliable, because you'll only get the mapped entities and properties without having to jump through hoops to exclude non-mapped ones. I'll give you the heart of it, so you can fit it in appropriately.

var oc = ((IObjectContextAdapter)dbcontext).ObjectContext;

var items = oc.MetadataWorkspace.GetItems(DataSpace.CSpace).OfType<EntityType>();
foreach (var entityType in items)
{
    var props = string.Join(",", entityType.Properties);
    Debug.WriteLine(string.Format("{0}: {1}", entityType.Name, props));
}

  Would this be better in the data layer or in another assembly?

I would make the part that provides table and column names part of the data layer, because there you have all required references. I would not include the part that queries the database, because it's done with Dapper, and...

  Is adding Dapper (just for this purpose) overkill, does it add unnecessary complexity to the project?

No, Dapper is great. But since this is a third-party dependency I would use it in a separate assembly that has a reference to your DAL (to obtain the database object names), so your DAL doesn't have this dependency.


Ask user for a number, and count from that number up to 300 I'm learning Javascript and created the below to take input from a user. If the number is between 150 and 300 it will print out the numbers from that number up till and including 300 in sequential order.

var num1;
conDition();

function conDition(){

 num1 = prompt("Please enter a number: ");

 if(num1 >= 150 && num1 <= 300){

     outNum();

 }else if(num1 < 150){
  alert("The number is too low for this function.");
  num1 = 0;
  conDition();

 }else if(num1 > 300){
  alert("The number is too high for this function.");
  num1 = 0;
  conDition();

 }else{
  alert("That is not a number!");
  num1 = 0;
  conDition();
 }
}

function outNum(){
 do
 {
     document.write(num1 + "
");
     num1++;
 }while (num1 <= 300);

 document.write("############################
");
 return true;
}

Can you help me make this better (more efficient)?

I would avoid making num1 a top level variable. Instead make it a local and pass it around. You also don't need to clear num1 every time. It will be cleared by the prompt method.

var num1 = prompt("Please enter a number: ");
outNum(num1)
function outNum(num1) {
  ...

Instead of do..while I would recommend a for loop.

Use more meaningful variable names.

This is a minor point for a learning exercise, but if efficiency is your goal try to reduce the number of times you modify the DOM (use document.write or anything similar s it is relatively expensive.

Lastly, though it is a somewhat personal preference and there is no hard and fast rule, I dislike using recursion to solve this type of input problem.

I would write this something like:

function run() {

  while (true) { // this loop runs forever

    var input = prompt("Please enter a number or press enter to exit: ");
    if (!input)
      break;        // leave the loop

    input = parseInt(input)

    if (isNaN(input)) {
        alert("That is not a number!");

    } else if (input < 150) {
        alert("The number is too low for this function.");

    } else if (input > 300) {
        alert("The number is too high for this function.");

    } else {
        printNumbers(input);

    }

  }  // end of while loop

}

function printNumbers(number) {
  var output = "";

  for (; number <= 300; number++) {
    output = output + number + "
"
  }

  output = output + "############################
";
  document.write(output); // Just change the document once
}

run();


Simple TCP client-server solution I have implemented a simple client-server solution using Java sockets and useful classes from java.util.concurrent.

The communication protocol is rather simple:

Client sends a string with the appended request_info#.
A server responds to a client with the message that provides a description on how to use a server.
If client sends server_time then a server will send back a current server time.
If client sends a quit - a server will process a client disconnect.
A default case: client sends an arbitrary string then a server simply bounces it back to a client.

I have tested my solution by spawning a fixed number of client threads. Each of them sends a command from a txt file to a server.

Some technical remarks:

A configuration for the client and server (such as port number and host address) is loaded from an .xml file.
To start a server: java –jar app.jar –server –config config.xml.
To start a client: java –jar app.jar –client –config config.xml.

I would like to seek any suggestions on whether I have correctly implemented the following class (serves as an abstraction for a message between client and server):

public class Message {
    private Socket socket;
    private String message;
    private static Logger logger = Logger.getLogger(Message.class);

    public void setSocket(Socket socket) {
        this.socket = socket;
    }

    public String getMessage() {
        return message;
    }

    public Socket getSocket() {
        return socket;
    }

    public void setStringMessage(String message) {
        this.message = message;
    }

    public void setMessageInputSources(InputStream inputStream) throws IOException {
        String s;
        BufferedReader bufferRead = new BufferedReader(new InputStreamReader(inputStream));
        s = bufferRead.readLine();
        this.message = s;
    }

    public synchronized String getCommand() {
        String stringMessage = this.getMessage();
        return StringUtils.upperCase(stringMessage.split("#")[0]);
    }

    public void send(String message) throws IOException{
        DataOutputStream dataOutputStream = new DataOutputStream(this.socket.getOutputStream());
        dataOutputStream.writeUTF(message);
        dataOutputStream.flush();

        this.setStringMessage(message);
    }

    public Message receive() throws IOException {
        String data;
        DataInputStream dataInputStream = new DataInputStream(this.socket.getInputStream());
        data = dataInputStream.readUTF();
        this.setStringMessage(data);
        return this;
    }

    @Override
    public String toString() {
        return this.message;
    }
}

Do I properly manage a case when all client threads are done with their jobs and I shut down an application? Are there any improvements to be made?

public class MainTest {

    private static final int NUMBER_OF_THREADS = 4;

        public static void main(String[] args) throws IOException, InterruptedException {
            CountDownLatch countDownLatch = new CountDownLatch(NUMBER_OF_THREADS);

            Thread serverThread = new Thread(() -> Server.start(new File("config.xml"), countDownLatch));
            serverThread.setName("server_thread");

            Thread clientThread = new Thread(() -> {
                try {
                    new Client(new File("config.xml"), new FileInputStream("commands.txt")).start(NUMBER_OF_THREADS, countDownLatch);

                } catch (ExecutionException | IOException | InterruptedException e) {
                    e.printStackTrace();
                }
            });
            clientThread.setName("Running client threads, " + 10);
            serverThread.start();
            clientThread.start();
            if(countDownLatch.getCount() == 0){
                System.exit(0);
            }
        }
    }

Here is the link to my project: GitHub Repository

Just a few things I would look into if this code will be used for any real purposes. The Client does not handle failing to connect to the Server. It assumes the Server will always be up and available. I don't see any kind of error handling if the Server/Client connection gets broken while sending messages. You should definitely add code to clean up your sockets properly after you are done with them.

Design wise I would separate the Socket connection from the actual data being sent. Maybe add connect/disconnect methods to the Client and Server to handle the Sockets and use an internal Thread that gets started after you have a valid connection. Should a data message know how to encode/decode itself? Should a data message know how to send/recv itself? In my opinion a separate Client/Server Service should be created to manage these functions. Data should be as dumb as possible. You also have a lot of Static state throughout your code I would remove that in favor of local class instance variables. Doing this will give you a lot more flexibility in the long run.

Another thing to consider is a Factory class which can create the Client and Servers. Use the Factory class to handle the configuration of the Client and Server rather than having them configure themselves. This will allow you to test the Client and Servers easier so you don't have to have a specific configuration file for testing.


Promises with NodeJS and BlueBird I'm using bluebird for promises in my Node/Express application and wrote an API call in which the user passes in a JSON Web Token that contains their user information, and then I decode the token, and pull up the events that user should see based off their userId.

If anyone could give me advice on how I can clean this code up while also using the proper promise practices, that would be great.

/routes.js

app.post('/api/events/', require('./views/api/index').events);

/views/api/index.js

var B_Promise = require('bluebird');
var jwt = require('jsonwebtoken');

exports.events = function(req, res) {
  var results = {};
  var errors = [];

  var validateEmptyFields = function() {
    return new B_Promise(function(resolve, reject) {
      var token = req.body.token || req.param('token') || req.headers['x-access-token'];
      if (!token) {
        return reject('Please provide the token parameter');
      }

      resolve(token);
    });
  };

  var getUser = function(token) {
    return new B_Promise(function(resolve, reject) {
      jwt.verify(token, req.app.config.api.secret, function(err, decoded) {
        if (err) {
          return reject(err);
        }

        resolve(decoded);
      });
    });
  };

  var getEvents = function(user) {
    return new B_Promise(function(resolve, reject) {
      req.app.db.getConnection(function(err, connection){
        if (err) {
          return reject(err);
        }

        /* jshint multistr: true */
        connection.query('SELECT e.* FROM events e \
          INNER JOIN event_to_groups etg ON e.id=etg.event_id \
          INNER JOIN user_to_groups utg ON utg.group_id=etg.group_id \
          WHERE utg.user_id=?', user.id, function(err, events) {
            if (err) {
              return reject(err);
            }

            results.events = events;
            resolve();
        });

        connection.release();
      });
    });
  };

  validateEmptyFields()
    .then(getUser)
    .then(getEvents)
    .catch(function(err) {
      errors.push(err);
    })
    .finally(function() {
      res.json({results: results, errors: errors});
    });
};

I would just name the variable for Bluebird promises as Promise as it mostly acts like the standard Promise object in ES6. That way, if you happen to run an ES6-compatible Node.js, you can just remove the import, and you'll be set.
In the promise constructor, you need not do a return. The code will also feel awkward to a new developer, thinking Promise needs a return. You can simply do:

jwt.verify(token, req.app.config.api.secret, function(err, decoded) {
  if (err) reject(err);
  else resolve(decoded);
});

Bluebird has a static method called Promise.promisify which converts callback-style APIs into promise-returning ones. Just make sure you follow it's guidelines in that:

  function should conform to node.js convention of accepting a callback as last argument and calling that callback with error as the first argument and success value on the second argument.

If your operation needs to return a promise, but it's not async, then you can simply return a resolved or rejected promise instantly using Promise.resolve and Promise.reject static methods, respectively.

var validateEmptyFields = function() {
  var token = req.body.token || req.param('token') || req.headers['x-access-token'];
  return token ? Promise.resolve(token) : Promise.reject('Please provide the token parameter')
};

In getEvents, I suggest you resolve with events. Then assemble results in the same way you assembled errors in catch. The idea is that your promise-generating functions should not be causing side-effects. It should only be aware of the fact that you called it with some arguments, and it resolves or rejects and nothing more.
Move out that SQL query from the logic. It's messy.

And so, without further ado:

var Promise = require('bluebird');
var jwt = require('jsonwebtoken');

var EVENT_QUERY = '\
SELECT e.* FROM events e \
INNER JOIN event_to_groups etg ON e.id=etg.event_id \
INNER JOIN user_to_groups utg ON utg.group_id=etg.group_id \
WHERE utg.user_id=?';

exports.events = function(req, res) {
  var results = {};
  var errors = [];

  var validateEmptyFields = function() {
    var token = req.body.token || req.param('token') || req.headers['x-access-token'];
    if (!token) Promise.reject('Please provide the token parameter');
    else Promise.resolve(token);
  };

  var getUser = function(token) {
    return new Promise(function(resolve, reject) {
      jwt.verify(token, req.app.config.api.secret, function(err, decoded) {
        if (err) return reject(err);
        else resolve(decoded);
      });
    });
  };

  var getEvents = function(user) {
    return new Promise(function(resolve, reject) {
      req.app.db.getConnection(function(err, connection) {
        if (err) return reject(err);

        connection.query(EVENT_QUERY, user.id, function(err, events) {
          if (err) return reject(err);
          else resolve(events);
        });

        connection.release();
      });
    });
  };

  validateEmptyFields()
    .then(getUser)
    .then(getEvents)
    .then(function(events) {
      results.events = events;
    }, function(err) {
      errors.push(err);
    })
    .finally(function() {
      res.json({
        results: results,
        errors: errors
      });
    });
};


Drag Drop, get filename and last 23 characters I want to get the filename and last 23 characters when I drag into a textbox.

I have it working, but I would like to just have one line.  Currently, I think I am doing a bit of a workaround. As I must use a string array, and I only want one string, so I don't really need an array.

private void First_DragDrop(object sender, DragEventArgs e)
{
    string[] fileList = e.Data.GetData(DataFormats.FileDrop) as string[];
    string filename = Path.GetFileNameWithoutExtension(fileList[0]);
    First.Text = filename.Substring(filename.Length - 23, 23);
}

As you can see, I must do it in 3 steps. First, get the string array.  Then, get one string from that array and turn it into a filename.  FInally, I need to get the last 23 characters of that string.

I think I should be able to just get everything immediately. Though maybe it's not possible?  It just seems weird that I must use an array that contains one thing.

Yes, you can write everything in one line, but you will not be happy with the result. And no, .Net creators did not think of implementing a single call which would return exactly 23 characters of dropped file name. :) Silly them. :)

Meanwhile, your code makes sense. I see only two possible issues:

You do not handle multiple files being dropped (that is why there is an array)
You do not handle fileList being null.


Randomly split a vector into N bins of random lengths As the title says, I want a function that takes a vector, as well as the number of bins, and splits the vector in that number of bins, with a minimum length of 1 for each bin.

def split_into_bins(nbin, vector):
    """
    Randomly split vector into nbin number of bins, each of random size
    """
    permutation = list(np.random.permutation(vector))

    # Location of the splits
    splits = sorted(np.random.choice(range(1,len(vector)), nbin-1, replace=False))

    # Initializing empty bins
    bins = [[]]*nbin

    start = 0
    end = splits[0]

    for i in range(nbin):
        bins[i] = permutation[start:end]
        start = end
        try:
            end = splits[i+1]
        except IndexError:
            end = len(vector)

    return bins

I'm wondering if there's a cleaner way to split the vector into bins besides randomly selecting split locations. My method of splitting up the list given the location of the splits also seems pretty messy. Performance does matter, so I'm wondering if I should initialize the empty bins outside of the function.

I also don't want there to be any bias with regards to the size of the bins; they should all have the same size on average. I'm pretty sure this method isn't biased, however.

np.random.choice can just take an int, so that can simplify your code - no need to build up a range():

splits = sorted(1 + x
    for x in np.random.choice(len(vector)-1, nbins-1, replace=True))

When you build up the bins, you don't need a try/except (you know up front what the size is). You just need to iterate over the actual splits:

bins = []

last = 0
for split in splits:
    bins.append(permutation[last:split])
    last = split
bins.append(permutation[split:])


JS Function warning before closing browser I have a function that makes sure the user wants to actually leave the page. I am not a jQuery expert. Is the code below the most efficient way to do this function? Or can it be simplified much more to run smoother? Any advice is greatly appreciated.

$(document).ready(function() {
  formmodified=0;
    $('form *').change(function(){
        formmodified=1;
    });
  window.onbeforeunload = confirmExit;
    function confirmExit() {
      if (formmodified == 1) {
          return "Are you sure you want to leave this page? This will abandon any progress on changes to document preferences";
      }
  }
  $("input[name='ModifyRecord']").click(function() {
        formmodified = 0;
    });
    $("input[name='DeleteRecord']").click(function() {
        formmodified = 0;
    });
    $("input[name='DeleteChecked']").click(function() {
        formmodified = 0;
    });
});

If you don't need the confirmExit function anywhere else, you can just assign it to to window.beforeunload.

window.onbeforeunload = function() {
    if (formmodified == 1) {
        return "Are you sure you want to leave this page? This will abandon any progress on changes to document preferences";
    }
}

Make sure your indentation is consistent, it's confusing to read when brackets don't match up.

Last but not least, it might be helpful (saves you the trouble of writing the same thing multiple times) and more performant to use a class for elements that should have a click handler to reset the "modified" state of the form:

$("input.formreset").click(function() {
    formmodified = 0;
});


Euler-Mascheroni Single Thread Speed Improvements The below code was written to generate γ, for educational purposes.

My general methodology is as follows:
Compute Gamma via the accepted answer's algorithm here.

In order to do this I need to compute the natural log of a large k.

Compute Pi via the AGM as here.
Compute the natural log of 2 via the last algorithm here.
Compute the natural log of a large k via Gauss's AGM method here.
Compute Gamma as described in the accepted answer here.
I am very grateful to the community for sharing the knowledge and I have improved the speed by orders of magnitude compared to the previous version (because of the accepted answers improvements).

One can pip3 install tqdm to see progress, I've wrapped a few ranges with it.

import decimal
#non-builtin
from tqdm import tqdm

D = decimal.Decimal

def agm(a, b):      #Arithmetic Geometric Mean
    a, b = D(a),D(b)
    for x in tqdm(range(prec)):
        a, b = (a + b) / 2, (a * b).sqrt()
    return a

def pi_agm():       #Pi via AGM and lemniscate
    print('Computing Pi...')
    a, b, t, p, pi, k = 1, D(2).sqrt()/2, 1/D(2), 2, 0, 0
    while 1:
        an    = (a+b)/2
        b     = (a*b).sqrt()
        t    -= p*(a-an)**2
        a, p  = an, 2**(k+2)
        piold = pi
        pi    = ((a+b)**2)/(2*t)
        k    += 1
        if pi == piold:
            break
    return pi

def factorial(x ,pfact, pfactprd):
    x = int(x)
    if pfact == (x-1):
        return pfactprd*x
    else:
        factorial = D(1)
        for i in range(1, x+1):
            factorial *= i
        return factorial

def lntwo():        #Fast converging Ln 2
    print('Computing Ln(2)...')
    def lntwosum(n, d, b):
        logsum, logold, e = D(0), D(0), 0
        while 1:
            logold = logsum
            logsum += D(1/((D(b**e))*((2*e)+1)))
            e += 1
            if logsum == logold:
                return (D(n)/D(d))*logsum
    logsum1 = lntwosum(14, 31, 961)
    logsum2 = lntwosum(6, 161, 25921)
    logsum3 = lntwosum(10, 49, 2401)
    ln2 = logsum1 + logsum2 + logsum3
    return ln2

def lnagm(x):   #Natural log of via AGM,
    try:
        if int(x) == 1:
            return 0
        if int(x) == 2:
            return lntwo()
    except:
        pass
    m = prec*2
    ln2 = lntwo()
    decimal.getcontext().prec = m
    pi = D(pi_agm())
    print('Computing Ln(x)...')
    twoprec = D(2**(2-D(m)))/D(x)
    den = agm(1, twoprec)*2
    diff = m*ln2
    result = (D(pi/den) - D(diff))
    logr = D(str(result)[:m//2])
    decimal.getcontext().prec = prec
    return logr

def gamma():   #Compute Gamma from Digamma Expansion
    print('Computing Gamma!')
    k = D(prec//2)
    lnk = lnagm(k)
    upper = int((12*k)+2)
    print('Summing...')
    # First Sum
    logsum = D(0)
    pterm = D((k**2)/2)
    for r in tqdm(range(1, upper)):
        r = D(r)
        logsum += pterm
        nterm = D(((-1)*D(k)*D(r+1))/(r*(r+2)))*pterm
        pterm = nterm
    logsum1 = D(0)
    print('...')
    pfact, pfactprd = 1, 1
    for r in tqdm(range(1, upper)):
        calfact = factorial((r-1), pfact, pfactprd)
        pfact, pfactprd = (r-1), calfact
        logsum1 += D((D(-1)**D(r-1))*(k**D(r+1)))/D(calfact*D(D(r+1)**2))
    twofac = D(2)**(-k)
    gammac = str(D(1)-(lnk*logsum)+logsum1+twofac)
    return D(gammac[:int(prec//6.66)])

#Calling Gamma
prec = int(input('Precision for Gamma: '))*8
decimal.getcontext().prec = prec
gam = gamma()
print('\n')
print(gam)

Firstly, this code has a couple of dozen PEP8 formatting violations. Following conventions generally helps readability.

Also on the subject of readability, comments providing references for the formulae used should be considered essential in mathematical software.

def agm(a, b):      #Arithmetic Geometric Mean

The conventional way of documenting a function is with a docstring. That also allows you to document the expected input types (decimal.Decimal? float?).

def pi_agm():       #Pi via AGM and lemniscate
    print('Computing Pi...')
    a, b, t, p, pi, k = 1, D(2).sqrt()/2, 1/D(2), 2, 0, 0
    while 1:
        an    = (a+b)/2
        b     = (a*b).sqrt()
        t    -= p*(a-an)**2
        a, p  = an, 2**(k+2)
        piold = pi
        pi    = ((a+b)**2)/(2*t)
        k    += 1
        if pi == piold:
            break
    return pi

The while loop is unnecessarily ugly: while True would be preferable. I would say that it would be even better to use the loop index as a loop index with for k in itertools.count(), but actually that variable is wholly unnecessary.

If find it unhelpful to initialise six variables in one line where some of them are quite complicated. On the other hand, it could be more helpful to combine some of the updates in the loop body. Perhaps the happy medium is something like

    a, b, t = 1, D(0.5).sqrt(), 1
    p, pi = 1, 0
    while True:
        a, b, t = (a+b)/2, (a*b).sqrt(), t - p*(a-b)**2
        p, piold, pi = 2*p, pi, (a+b)**2 / t

I'm not entirely convinced by

        if pi == piold:
            break

Sometimes iterative approaches in finite data types oscillate around the solution rather than converging definitively. It might be more robust to track the last two or three values and, on finding a loop, return the average of the values in the loop.

def factorial(x ,pfact, pfactprd):
    x = int(x)

Do you expect to pass a non-int? See previous note about using docstrings to document types.

    if pfact == (x-1):
        return pfactprd*x
    else:
        factorial = D(1)
        for i in range(1, x+1):
            factorial *= i
        return factorial

Is the else ever actually used? Might it be more maintainable to remove this function entirely?

    def lntwosum(n, d, b):
        logsum, logold, e = D(0), D(0), 0
        while 1:
            logold = logsum
            logsum += D(1/((D(b**e))*((2*e)+1)))
            e += 1
            if logsum == logold:
                return (D(n)/D(d))*logsum

Here the previous comment about itertools.count is relevant.

I'm confused as to why D is invoked where it is. Without any comments to justify it, it appears to be done at random.

def lnagm(x):   #Natural log of via AGM,
    try:
        if int(x) == 1:
            return 0
        if int(x) == 2:
            return lntwo()
    except:
        pass

??? Are you expecting int(x) to throw an exception?

    pi = D(pi_agm())

See previous comments about appearing to use D at random. Here, if pi_agm() returns a decimal.Decimal then it's unnecessary, and if it doesn't then surely that would be a bug because pi won't have the necessary precision? I don't see any further polishing of its error.

    decimal.getcontext().prec = m
    ...
    logr = D(str(result)[:m//2])
    decimal.getcontext().prec = prec
    return logr

Would the following work?

    decimal.getcontext().prec = m
    ...
    decimal.getcontext().prec = prec
    return D(result)

    k = D(prec//2)
    ...
    upper = int((12*k)+2)

Why not just upper = 6*prec + 2 with no need to coerce?

Other earlier comments also apply to gamma.

#Calling Gamma
prec = int(input('Precision for Gamma: '))*8
decimal.getcontext().prec = prec
gam = gamma()
print('\n')
print(gam)

It's a Python best practice, which serves to make the file reusable as a library, to guard this with if __name__ == "__main__":.

A comment explaining the *8 would be useful. At a guess, prec is in bits?


Python Power function I'm a freshman in Python world, I used to program in C.

I wrote a function/method that returns the power of a number:

def pow(x, y):

    powered = x

    if y == 0:
        return 1

    while y > 1:
        powered *= x

        y -= 1

    return powered

Mine is a logic question (correctly): if I set y = 0 then it returns \$1\$, but on the last line of the code I wrote return powered then = x.

Does return work like as break?

return means "Eureka! I have found the answer! It is …."  It hands that result back to the caller and stops executing any more code in this function.

Unlike x**y, your function gives incorrect results for negative or non-integer values of y.  You should note that caveat in a docstring.

The standard way to do counting loops in Python is to use range() (or, in Python 2, xrange()).

To reduce the complexity of your code, you should avoid unnecessary special cases.

def pow(x, y):
    """Raise x to the power y, where y must be a nonnegative integer."""
    result = 1
    for _ in range(y):   # using _ to indicate throwaway iteration variable
        result *= x
    return result

This particular looping pattern (starting with some initial value and repeatedly applying an operation to the result) could be reduced to a one-liner:

from functools import reduce  # Optional in Python 2, required in Python 3

def pow(x, y):
    """Raise x to the power y, where y must be a nonnegative integer."""
    return reduce(lambda result, _: result * x, range(y), 1)

This solution is admittedly foreign-looking to a C programmer.  The reduce() function uses 1 as the starting value (equivalent to result = 1 in the first solution).  For each _ in range(y), it does result = result * x.  lambda is just a way to define a simple function on the spot.

A more efficient algorithm would be repeated squaring.


Generic function with same logic but different input structure I have a data structure that looks like this:

interface Node {
  originalWeight: number;
  currentWeight: number;
}

where both properties are floats between 0 and 1.

To check if a node has been modified, I wrote a simple isNodeModified function:

isNodeModified(node: Node): boolean {
  return Math.abs(node.originalWeight- node.currentWeight) > this.EPSILON;
}

where EPSILON is my tolerance.

However, I also need to do a very similar comparison in a slightly different scenario, which was originally handled like this:

if (Math.abs(event.value - node.originalWeight) > this.EPSILON) {
  // do something
}

where event is another object with a value property.

To avoid duplication, I replaced both methods with something like the following:

isNodeModified(node: Node, event?: Event): boolean {
  let original = node.originalWeight;
  if (event) {
    original = event.value;
  }
  return Math.abs(original - node.currentWeight) > this.EPSILON;
}

So that I can call isNodeModified(node) or isNodeModified(node, event) depending on what I need.

Is there a better / cleaner solution to this problem?

You could create a more general function which simple compares two values.
Like:

hasDifference(oldValue: number, newValue: number, offset?: number): boolean {
  const offsetToUse: number = offset === undefined ? this.EPSILON : offset;

  return Math.abs(oldValue - newValue) > offsetToUse;
}


Testing whether a number is nearly a multiple of 10 I'm going through a basic python exercise, stumbled upon this query.

  Given a non-negative number "num", return True if num is within 2 of a multiple of 10.

Here's the my solution:

def near_ten(num):
  return (num % 10) in range(8, 10) or (num % 10) in range(0, 3)

It shows "All Correct", but I'm feeling like my code is bit digressive as I have hard-coded the "numbers within 2 of a multiple of 10".

Can one please review this line if its the right approach?

I would suggest you to go further in  hard coding those values:
I mean I would write:

def is_near_ten(num):
   return (num % 10) in [0, 1, 2, 8, 9]

You can see visually the difference (less instructions) and you can confirm that:

>>> import timeit
>>> timeit.timeit('(128 % 10) in [0, 1, 2, 8, 9]')
0.23481132300003082
>>> timeit.timeit('(128 % 10) in range(8, 10) or (128 % 10) in range(0, 3)')
1.3901589560000502

You can notice that further hard coding those values as suggested makes your function more than 5 times faster.


Armstrong numbers in C I have very little experience with C, so I decided to code a simple numerical program.

An Armstrong number with \$N\$ digits is a number that is equal to the sum of its digits to the \$N^{th}\$ power, in pseudo-code:

sum { power(digit, N) for digit in number } == number

My program finds all the Armstrong numbers below the given limit.

#include <stdio.h>
#include <stdbool.h>
#include <string.h>
#include <stdlib.h>
#include <assert.h>

int power(int a, int b) {
    int original_a = a;
    while (b --> 1) { a *= original_a;}
    return a;
}

int digits_len(int n) {
    int length = 0;
    while(n) {
        n /= 10;
        length++;
    }
    return length;
}

int sum_digits_to_the_pow(int n, int exp) {
    int sum = 0;
    while (n) {
        sum += power((n % 10), exp);
        n /= 10;
    }
    return sum;
}

bool is_armstrong(int n) {
    return n == sum_digits_to_the_pow(n, digits_len(n));
}

int main(int argc, char *argv[]) {
    assert(power(4, 3) == 64);
    assert(digits_len(4812) == 4);
    assert(sum_digits_to_the_pow(214,2) == 4+1+16);
    assert(is_armstrong(153) == true);

    for (int i=0; i < atoi(argv[1]); i++) {
        if (is_armstrong(i)) {
            printf("%d ", i);
        }
    }
}

Run my program with (it takes 22 seconds):

gcc  -std=c99 -O3 armstrong.c; time ./a.out 100000000

This is a well-written program.  It is clear and easy to follow, which are attributes of good software.  With that said, there are still some things that you can do to speed up the performance of the code and there are a few minor comments on formatting.  I'm hoping they will help you improve your code.

Avoid peculiar formatting

I have been programming in C for many, many years, but this line threw me for a moment:

while (b --> 1) { a *= original_a;}

It was the --> that was odd, but of course, it is actually equivalent to this:

while (b-- > 1) {
    a *= original_a;
}

Formatting for clarity, including keeping operators attached to their variables and avoiding one-liners helps others read and understand your code.

Avoid expensive mathematical operations where practical

To speed things along, we can replace this code

int digits_len(int n) {
    int length = 0;
    while(n) {
        n /= 10;
        length++;
    }
    return length;
}

With this:

int digits_len(int n) {
    if (n < 10) return 1;
    else if (n < 100) return 2;
    else if (n < 1000) return 3;
    else if (n < 10000) return 4;
    else if (n < 100000) return 5;
    else if (n < 1000000) return 6;
    else if (n < 10000000) return 7;
    else if (n < 100000000) return 8;
    return 9;
}

It's longer but it avoids the computationally costly division and avoid a loop.  Note, however, that it does not produce sensible results for negative numbers.  It seemed likely that this was acceptable for the increase in speed gained for this program.

Use a table lookup instead of a loop

The power routine, in context, has the property that neither a nor b are greater than 9. With that observation, it's possible to replace the routine with a simple table lookup.  Here's what I used:

const int powtable[10][10] = {
    { 0, 0, 0*0, 0*0*0, 0*0*0*0, 0*0*0*0*0, 0*0*0*0*0*0, 0*0*0*0*0*0*0, 0*0*0*0*0*0*0*0, 0*0*0*0*0*0*0*0*0 },
    { 0, 1, 1*1, 1*1*1, 1*1*1*1, 1*1*1*1*1, 1*1*1*1*1*1, 1*1*1*1*1*1*1, 1*1*1*1*1*1*1*1, 1*1*1*1*1*1*1*1*1 },
    { 0, 2, 2*2, 2*2*2, 2*2*2*2, 2*2*2*2*2, 2*2*2*2*2*2, 2*2*2*2*2*2*2, 2*2*2*2*2*2*2*2, 2*2*2*2*2*2*2*2*2 },
    { 0, 3, 3*3, 3*3*3, 3*3*3*3, 3*3*3*3*3, 3*3*3*3*3*3, 3*3*3*3*3*3*3, 3*3*3*3*3*3*3*3, 3*3*3*3*3*3*3*3*3 },
    { 0, 4, 4*4, 4*4*4, 4*4*4*4, 4*4*4*4*4, 4*4*4*4*4*4, 4*4*4*4*4*4*4, 4*4*4*4*4*4*4*4, 4*4*4*4*4*4*4*4*4 },
    { 0, 5, 5*5, 5*5*5, 5*5*5*5, 5*5*5*5*5, 5*5*5*5*5*5, 5*5*5*5*5*5*5, 5*5*5*5*5*5*5*5, 5*5*5*5*5*5*5*5*5 },
    { 0, 6, 6*6, 6*6*6, 6*6*6*6, 6*6*6*6*6, 6*6*6*6*6*6, 6*6*6*6*6*6*6, 6*6*6*6*6*6*6*6, 6*6*6*6*6*6*6*6*6 },
    { 0, 7, 7*7, 7*7*7, 7*7*7*7, 7*7*7*7*7, 7*7*7*7*7*7, 7*7*7*7*7*7*7, 7*7*7*7*7*7*7*7, 7*7*7*7*7*7*7*7*7 },
    { 0, 8, 8*8, 8*8*8, 8*8*8*8, 8*8*8*8*8, 8*8*8*8*8*8, 8*8*8*8*8*8*8, 8*8*8*8*8*8*8*8, 8*8*8*8*8*8*8*8*8 },
    { 0, 9, 9*9, 9*9*9, 9*9*9*9, 9*9*9*9*9, 9*9*9*9*9*9, 9*9*9*9*9*9*9, 9*9*9*9*9*9*9*9, 9*9*9*9*9*9*9*9*9 }
};
inline int power(int a, int b) {
    return powtable[a][b];
}

The multiplications in the table are all done at compile-time rather than at runtime, so they cost essentially nothing.  (I could, of course, have simply placed the appropriate numbers into the table, but I'm very lazy and prefer to make the computer do all the work!)

Avoid unused arguments

The main routine has the usual arguments of argc and argv but argc is never used.  If you compile with all warnings turned on (which I would recommend), the compiler will correctly complain about the unused variable.  I decided to use it and also to give a hint to the user as to how to use the program:

if (argc < 2) {
    puts("Usage: armstrong maxint\n");
    return 0;
}

Keep using assert

It's refreshing to see code from an avowed beginner at C using assert rationally and correctly.  Bonus points for you!  Also, although I didn't, you might wish to use asserts to assure that the parameters passed to power are indeed both less than 10.

Results

On my machine, the original code, when compiled with -O2 using gcc version 4.9.2 on a 64-bit Linux box, the code runs in 8.1 seconds.  With the changes mentioned above, it runs in 2.4 seconds, or almost 4x faster.


Generate the name of a computer from its properties I am writing a small application to generate the domain name of computers, from their known properties, and business rules.
My Asset class is:
public enum AssetSite
{
    Rotterdam,
    Sydney,
}

public class Asset
{
/// <summary>
/// Gets the Asset Tag of the asset.
/// </summary>
public string AssetTag { get;}

/// <summary>
/// Gets the site to which the asset belongs.
/// </summary>
public AssetSite AssetSite { get; }

/// <summary>
/// Gets a value indicating whether the asset is a laptop.
/// </summary>
public bool IsLaptop { get; }

/// <summary>
/// Gets the name of the asset.
/// </summary>
public string Name
{
    get
    {
        var nameBuilder = new StringBuilder();
        nameBuilder.Append('I'); // Convention: All names start with I

        // Second letter indicates the location of the PC.
        if (this.AssetSite == AssetSite.Sydney)
        {
            nameBuilder.Append('S');
        }
        else if (this.AssetSite == AssetSite.Rotterdam)
        {
            nameBuilder.Append('R');
        }
        else
        {
            throw new ArgumentOutOfRangeException("Invalid site");
        }

        nameBuilder.Append("PC"); //Then, all PCs have 'PC' in their name.

        if (this.IsLaptop)
        {
            nameBuilder.Append("L"); //Laptops get a "L" in their name
        }
        else
        {
            nameBuilder.Append("D"); //Desktops get a "D"
        }

        nameBuilder.Append(this.AssetTag); //Unique identifier of the computer

        return nameBuilder.ToString();
    }
}

Asset objects are created through the deserialization of a JSON file
I am worried about my implementation of the Name getter. It feels way too long and complex, but at the same time, extracting each letter generation to a dedicated method feels way to complex as well.
My main concerns here are readability and maintainability.

I would highlight two things:

It is not really common to throw exception inside a property getter
Because the Name has only just a getter then you can consider to expose it as a method instead.

From implementation point of you, you can improve readability if you do something like this:
var nameBuilder = new StringBuilder();
nameBuilder.Append('I');

var siteWhitelist = new AssetSite[] { AssetSite.Sydney, AssetSite.Rotterdam };
if (!siteWhitelist.Contains(this.AssetSite))
    throw new ArgumentOutOfRangeException("Invalid site");

string site = this.AssetSite switch
{
    AssetSite.Sydney    => "S";
    AssetSite.Rotterdam => "R";
};

nameBuilder.Append(site);
nameBuilder.Append("PC");

string computerType = this.IsLaptop ?  "L" : "D";

nameBuilder.Append(computerType)
nameBuilder.Append(this.AssetTag);

return nameBuilder.ToString();

Of course you can further improve it, by introducing a Dictionary<AssetSite, string> where you can store the mapping between sites and letters.


Function to set nested property by dot-path, like _.set As part of a Stack Overflow answer, I whipped up a quick example implementation of a function to set a nested property of an object using a dot notation string. The goal was to show how that could work in a small amount of clean code:

function set (object, property, value) {
  if (typeof property === 'string') property = property.split('.')
  if (property.length > 1) {
    set(object[property[0]], property.slice(1), value)
  } else {
    object[property[0]] = value
  }
}

// Usage:
set(object, 'position.x', 1)

I could have used property.shift() but I wanted to not make the function unnecessarily non-pure and I think that mutating the array would also have made the code harder to follow.

This caused me to get the following comment:

  That own-rolled version has potential prototype pollution problems, has a time complexity problem, and can cause a stack overflow. – Ry-

To which I responded:

  Can you elaborate on the prototype pollution and time complexity? Is the latter about the repeated slice? Regarding stack overflow it's of course true but we'd have to have an insanely long property path, I'm trading that chance for simplicity.

  This version also doesn't check for existence of sub-paths obviously, so yes I could do for example set(something, { length: 123 }, x) or set(something, { slice: 'oh no' }, x) and such and it would crash, but it would also crash on set(something, 'nonexisting.abc', x) at the moment – CherryDT

I then decided to remove the code for the time being and instead open this question here, because I'd be interested to get more feedback about that but the SO question is not the right location for it.

So, I'd be happy to hear your thoughts about it.

EDIT: I realize now, the prototype pollution issue is in case of property being untrusted input, I could write set(something, '__proto__.toString', 'yikes') for example, right? Still, I'd be interested in any other feedback too, now that we are already at it.

Your code looks quite reasonable, there are only a few minor things that stand out to me.

You're constructing a new array every time a recursive call is made. For example, if the property passed is foo.bar.baz.qux.quuz, you will be constructing the following arrays of properties:

['foo', 'bar', 'baz', 'qux', 'quuz'] // initial call to `set`
['bar', 'baz', 'qux', 'quuz'] // 2nd call (recursive)
['baz', 'qux', 'quuz'] // 3rd call (recursive)
['qux', 'quuz'] // 4th call (recursive)
['quuz'] // 5th call (recursive)

These are all entirely separate arrays - when you .slice, you create a new array, it's not just a reference to a particular subset of the old array's indicies. Admittedly, it's unreasonable for a property list to be large enough for this to be a problem, but it seems a bit inelegant for space complexity. If you want to keep going the recursive route, you could fix it by creating the array only once in the initial call, then passing along an index to access.

Since the property is actually expected to be an array of properties, it would make more sense to pluralize the variable name. Also, since you're worried about purity, if you want to make it a bit more functional, rather than reassigning the parameter (which is better avoided when possible), put the result into a different variable, maybe by using a default argument:

function set (object, propertyStr, value, properties = propertyStr.split('.'), i = 0) {
  if (i === properties.length - 1) {
    object[properties[i]] = value;
  } else {
    set(object[properties[i]], '', value, properties, i + 1);
  }
}

const object = {
  position: {
    x: 5
  }
};
set(object, 'position.x', 1);
console.log(object);

But note that the function is still fundamentally impure, because its purpose is to mutate an argument. (That's not a problem unless you want it to be one, just something to keep in mind when discussing purity)

I think the recursion aspect makes things a bit more confusing than they need to be. I'd prefer to use reduce to iterate over the properties and access the last object, then assign the value to the last property on that last object:

const object = {
  position: {
    nested: {
      x: 5
    }
  }
};

function set(object, propertyStr, value) {
  const properties = propertyStr.split('.');
  const lastProperty = properties.pop();
  const lastObject = properties.reduce((a, prop) => a[prop], object);
  lastObject[lastProperty] = value;
}

set(object, 'position.nested.x', 'with pop');
console.log(object);

// If you want to avoid the .pop mutation, then:
function setNoPop(object, propertyStr, value) {
  const properties = propertyStr.split('.');
  const lastObject = properties.slice(0, properties.length - 1)
    .reduce((a, prop) => a[prop], object);
  lastObject[properties[properties.length - 1]] = value;
}

setNoPop(object, 'position.nested.x', 'no pop');
console.log(object);

The code will currently throw an error if an intermediate property doesn't exist or, in strict mode, is not an object. If you want an error to be thrown in this situation, that's OK, but you might prefer for the function to return a boolean indicating success or failure.

const object = {
  position: {
    nested: {
      x: 5
    }
  }
};

const isObj = param => typeof param === 'object' && param !== null;
function set(object, propertyStr, value) {
  const properties = propertyStr.split('.');
  const lastProperty = properties.pop();
  const lastObject = properties.reduce((a, prop) => isObj(a) ? a[prop] : null, object);
  if (isObj(lastObject)) {
    lastObject[lastProperty] = value;
    return true;
  } else {
    return false;
  }
}

const success1 = set(object, 'position.nested.x', 'with pop');
console.log(success1);
const success2 = set(object, 'position.doesNotExist.x', 'with pop');
console.log(success2);


Controller method - Ajax request I'm quite new to Laravel, and I'm not sure what am I doing is the best practice.

I'd like to return JSON if the request is Ajax, or return a view otherwise. This is the way I made it, and it works fine, but I'm not sure if this is the best way(it seems long winded). I'd really like to hear your suggestions.

public function store()
{
    $input = Input::all();

    if(!$this->settings->fill($input)->isValid())
    {
        if ( Request::ajax() ){
            return $this->jsonFailure(array(
                'errors' => $this->settings->errors
            ));
        }
        else{
            return Redirect::route('admin.settings.create')
                ->withInput()
                ->withErrors($this->settings->errors);
        }
    }

    $this->settings->save();

    if ( Request::ajax() )
        return $this->jsonSuccess('success');
    else
        Redirect::route('admin.settings.index');
}

The only problem with this approach is that as you add more methods, you are going to end up with an awful lot of repeated if (Request::ajax()) ... blocks all over your code.

Here's what I do for my sites:

<?php
class ApiController extends Controller {
    protected function makeResponse($data, $isError = false, $redirectToIfNotAjax = '/') {
        if (Request::ajax()) {
            if ($isError) {
                return $this->jsonFailure(array(
                    'errors' => $data
                ));
            } else {
                return $this->jsonSuccess($data);
            }
        } else {
            if ($isError) {
                return Redirect::route($redirectToIfNotAjax)
                    ->withInput()
                    ->withErrors($data);
            } else {
                Redirect::route($redirectToIfNotAjax);
            }
        }
    }
}

(This is not exactly the same as what I am using, but it's pretty close.)

Then, inherit your controllers from ApiController instead of just Controller.

Here is what your example would look like:

public function store()
{
    $input = Input::all();

    if(!$this->settings->fill($input)->isValid())
    {
        return makeResponse($this->settings->errors, true, 'admin.settings.create')
    }

    $this->settings->save();

    return makeResponse('success', false, 'admin.settings.index')
}

Much cleaner!


Split String by delimiter in Haskell I wrote a split by delimiter function in Haskell and wanted some feedback on this piece of code.
Since I come from an imperative programming background, I often write too complex functions in haskell.
split :: Char -> String -> [String]
split c str = fst $ splitInternal c ([], str)

splitInternal :: Char -> ([String], String) -> ([String], String)
splitInternal _ (result, "")  = (result, "")
splitInternal c (result, str) = splitInternal c (
    result ++ [takeWhile (/= c) str],
    case dropWhile (/= c) str of
        "" -> ""
        rest -> tail rest
    )

My questions are

Is it bad to have splitInternal function? I couldn't figure out a way without it.
Is there maybe a simpler way to write the function?
Any other feedback is welcome as well

Is it bad to have splitInternal function? I couldn't figure out a way without it.

Well, according to your procedure, I think it is necessary, but you can improve the readibility by writing some small functions and then combining them together. Besides, if there are consecutive delimiters, your split function doesn't work as expected. The code can be rewritten as following:
splitInternal :: Char -> ([String], String) -> ([String], String)
splitInternal _ (result, "") = (result, "")
splitInternal c (result, remain) = splitInternal c (getBefore c remain, getAfter c remain)
  where
    getBefore delimiter rest = result ++ [takeWhile (/= delimiter) rest]
    getAfter delimiter rest = dropWhile (== delimiter) . dropWhile (/= delimiter) $ rest

Is there maybe a simpler way to write the function?

Yes, you can use the break and span function defined in Prelude:
split :: Char -> String -> [String]
split _ "" = []
split delimiter str =
    let (start, rest) = break (== delimiter) str
        (_, remain) = span (== delimiter) rest
     in start : split delimiter remain

So in this case, your splitInternal is unnecessary.

Any other feedback is welcome as well

Well, if you are dealing with string, then a better choice is Text from Data.Text. Text is more efficient than String when you are dealing with string. In the module Data.Text, there is a pre-defined function splitOn that works almost as you expect:
ghci> :seti -XOverloadedString

ghci> splitOn "," "123,456,789"
["123","456","789"]

ghci> splitOn "," "123,,,456,789"
["123","","","456","789"]           -- This is what I mean "almost", since splitOn doesn't use the consecutive delimiters. Maybe this is what you want.
```


Protect from people bruteforcing the PHPSESSID The PHPSESSID is stored in the clients cookie so I don't consider it as secure. Someone might bruteforce it and perform some action (like a Facebook status post) whenever a session was successfully hijacked.

I was wondering how I can protect against attacks like that. Maybe by locking out suspect users? My solution I came up with is a bit different:

<?php

    $lifetime = 1 * 60 * 60; // seconds

    // Rename the PHPSESSID cookie for convenience and set the maximum lifetime
    ini_set('session.name', 'session_id');
    ini_set('session.gc_maxlifetime', $lifetime);
    session_set_cookie_params($lifetime);

    // Create a new session if necessary
    if ( ! isset($_COOKIE['session_id'])) {
        @session_start() or die(); // Don't output anything on invalid cookie forgery attempts

        // Generate a random lowercase alphanumeric string
        $sessionKey = substr(str_shuffle(str_repeat('0123456789abcdefghijklmnopqrstuvwxyz', 5)), 0, 32);

        // Save it in the session and as a cookie
        setcookie('session_key', $sessionKey);
        $_SESSION['session_key'] = $sessionKey;

        $_SESSION['foo'] = 'bar'; // Just for testing – see the last echo statement in this script
    }
    else {
        @session_start() or die(); // Don't output anything on invalid cookie forgery attempts

        // Destroy the session completely (including client cookies) if the session keys don't match
        if ($_SESSION['session_key'] !== $_COOKIE['session_key']) {
            session_unset();
            session_destroy();
            setcookie('session_id', null, time() - 1);
            setcookie('session_key', null, time() - 1);
        }
    }

    echo isset($_SESSION['foo']) ? $_SESSION['foo'] : 'baz';

?>

What do you think about it? Can I consider this as secure or are any other steps required? Will it protect me successfully from the described attacks?

Depending of the attack, your code might be either may be more secure than the traditional use of PHP sessions.

If the attacker gets the session_id cookie from an XSS attack, he also gets access to the session_key one, and your code won't protect sessions from being hijacked. You can avoid this by setting the HttpOnly flag on your cookies, making them not accessible by JavaScript through document.cookie :

Using ini_set('session.cookie_httponly', 1); will configure PHP to add the flag on the session_id cookie.
Setting the seventh parameter of the setcookie function to true will set the HttpOnly flag on the session_key cookie. More information about all the parameters can be found on the official documentation : http://php.net/manual/function.setcookie.php

Anyway, that won't protect from the man in the middle attacks.
PHP has a function named session_regenerate_id which changes the current session id by a new one, making the hijacked cookies obsolete.

All you need to do is calling this function once you've started the session : http://php.net/manual/en/function.session-regenerate-id.php

If the attacked gets the session_id cookie from bruteforce, then your solution is indeed more secure, but you should still have a look to the two settings mentioned above.

Note that substr(str_shuffle(str_repeat('0123456789abcdefghijklmnopqrstuvwxyz', 5)), 0, 32); doesn't generate a true random lowercase alphanumeric string since each character won't appear more than 5 times. substr(str_shuffle(str_repeat('0123456789abcdefghijklmnopqrstuvwxyz', 32)), 0, 32); will work.

There are also more php.ini settings to add more entropy to PHP sessions ids :

The session.hash_function setting changes the hash function used by PHP to generate the id.
The session.entropy_file and session.entropy_length settings will tell PHP to use an external file as a source of additional entropy when generating the id.


Finding number pairs where the sum of their divisors are each other Given a limit \$n\$, find all the pairs of values less than, or equal to n, which have the the property:

pair(a,b)

where a = sum of divisors of b
  and b = sum of divisors of a

I need to speed up my double for-loop but I don't see how to do it in this case.

def diviseurs_propres(n):
    t = [1]
    for i in range(2,n):
        if n%i == 0:
            t.extend([i])
    return t

def somme_diviseurs_propres(n):
    t = diviseurs_propres(n)
    S = sum(list(t))
    return S

def trouveur(n):
    t = []
    for i in range(0,n+1):
        for p in range(0,n+1):
            if p != i and somme_diviseurs_propres(i) == p and i == somme_diviseurs_propres(p):
             t.extend([p]) and t.extend([i])
    return t

Mike is correct that you are repeating the sum multiple times for each value. A simple array with the stored sum will make a huge difference.

On the other hand, you are also doing a huge amount of unnecessary work in here:

def diviseurs_propres(n):
    t = [1]
    for i in range(2,n):
        if n%i == 0:
            t.extend([i])
    return t

def somme_diviseurs_propres(n):
    t = diviseurs_propres(n)
    S = sum(list(t))
    return S

There is no need to store the actual values in an array, it can be simplified to just:

def somme_diviseurs_propres(n):
    sum = 1
    for i in range(2, n):
        if n % i == 0:
            sum += i
    return sum

With a little bit of math, you can significantly reduce the number of iterations by iterating to the square-root of the value:

def somme_diviseurs_propres(n):
    sum = 1
    root = int(math.ceil(math.sqrt(n)))
    for i in range(2, root):
        if n % i == 0:
            sum += i + (n / i)
    if root > 1 and root * root == n:
        sum += root
    return sum

The way the above code works, is each time you have a value \$a\$ that is a factor of \$n\$, then there must be a value \$n / a\$ that is also an factor.

Thus, for large values of \$n\$, like 1000000, you only need to iterate from 2 to 1000 (exclusive) to sum the factors. Now, because 1000000 is an exact square, the root is an actual integer too, and the last if-statement above adds in the value 1000 to the sum.

In addition to the above, you can also rearrange the double-loops so that the inner loop is limited by the outer loop, and then add the pairs twice to the results. This changes the order of the results, but not the actual content.... Finally, your use of extend should be replaced with append...

def trouveur(n):
    t = []
    for i in range(0, n+1):
        for p in range(0, i):
            if somme_diviseurs_propres(i) == p and i == somme_diviseurs_propres(p):
                t.append(p) and t.append(i)
                t.append(i) and t.append(p)
    return t

Now, there's a bug in there, I believe. The code:

t.extend([p]) and t.extend([i])

does not add the second value i. You only add the i on the second time through, when i == p and p == i .... Your code above is equivalent to just:

t.extend([p])

Using a dictionary

As an update, I have considered a second soltuion, that should significantly reduce the amount of iteration in the inner loops, at the expense of lookups in a dictionary.

The dictionary maps the sum-of-factors to the values that have that sum. For example, the sum of factors for 6 and 25 are both 6. (sum of factors of 6 are 3 + 2 + 1, and for 25 it is 5 + 1). The mapping looks like:

6 -> [6, 25]

Using that mapping, we can look for sums that have cross-references... We can loop through just the sums of the factors, and for each sum, in their set of input values, see if those values exist as sums of other values. If they do, then see if the original sum is one of them.... This makes sense if you see an example:

220 -> [284]
284 -> [220]

the top value shows that the sum of factors for 284 is 220, and the sum of factors for 220, is 284.

Taking the first value, we know the sum of factors, and we take all the values that sum to that (in this case, just 284), and check whether 284 happens to be the sum of some other value's factors. Then we check to see whether one of those other values is 220.

def trouveur(n):
    sommeMap = {}
    for i in range(1, n + 1):
        s = somme_diviseurs_propres(i)
        if s in sommeMap:
            sommeMap[s].append(i)
        else:
            sommeMap[s] = [i]

    t = []
    for i,ps in sommeMap.items():
        for p in ps:
            if i < p and p in sommeMap and i in sommeMap[p]:
                t.append(i)
                t.append(p)
    return t

Performance results:

Your initial code, with a limit of 500

Result: [284, 220]

real    0m4.851s
user    0m4.840s
sys     0m0.012s

Using Mike's suggestion of saving away the sums:

def trouveur(n):
    somme = []
    t = []
    for i in range(0,n+1):
        somme.append(somme_diviseurs_propres(i))

    for i in range(0,n+1):
        for p in range(0,n+1):
            if p != i and somme[i] == p and i == somme[p]:
             t.extend([p]) and t.extend([i])
    return t

This produces the times:

Result: [284, 220]

real    0m0.064s
user    0m0.056s
sys     0m0.008s

Including the changes I suggest

no arrays for the sum of factors
change the loops
separate the i and p appends

Code:

import math

def somme_diviseurs_propres(n):
    sum = 1
    root = int(math.ceil(math.sqrt(n)))
    for i in range(2, root):
        if n % i == 0:
            sum += i + (n / i)
    if root > 1 and root * root == n:
        sum += root
    return sum

def trouveur(n):
    somme = []
    t = []
    for i in range(0, n + 1):
        somme.append(somme_diviseurs_propres(i))

    for i in range(0, n + 1):
        for p in range(0, i):
            if somme[i] == p and i == somme[p]:
                t.append(p)
                t.append(i)
    return t

print "Result: {}".format(trouveur(500))

and results:

Result: [220, 284]

real    0m0.028s
user    0m0.016s
sys     0m0.008s

Note that it is now possible to run the code up to 10000 as well....

Result: [220, 284, 1184, 1210, 2620, 2924, 5020, 5564, 6232, 6368]

real    0m2.453s
user    0m2.444s
sys     0m0.004s

Using dict (sommeMap)

Result: [220, 284]

real    0m0.024s
user    0m0.016s
sys     0m0.008s

and for 10,000 it runs in:

Result: [220, 284, 1184, 1210, 2620, 2924, 5020, 5564, 6232, 6368]

real    0m0.092s
user    0m0.088s
sys     0m0.000s

Which is significantly faster, and we can extend that even further now, to say 1,000,000 values:

Result: [[220, 284], [1184, 1210], [2620, 2924], [5020, 5564], [6232, 6368],
    [10744, 10856], [12285, 14595], [17296, 18416], [63020, 76084], [66928, 66992],
    [67095, 71145], [69615, 87633], [79750, 88730], [100485, 124155], [122265, 139815],
    [122368, 123152], [141664, 153176], [142310, 168730], [171856, 176336],
    [176272, 180848], [185368, 203432], [196724, 202444], [280540, 365084],
    [308620, 389924], [319550, 430402], [356408, 399592], [437456, 455344],
    [469028, 486178], [503056, 514736], [522405, 525915], [600392, 669688],
    [609928, 686072], [624184, 691256], [635624, 712216], [643336, 652664],
    [667964, 783556], [726104, 796696], [802725, 863835], [879712, 901424],
    [898216, 980984]]

real    0m55.310s
user    0m55.135s
sys     0m0.164s

Code Drop

This is the final version of the code I have. The code above has gone through a number of iterations, and was inconsistent in places:

import math

def somme_diviseurs_propres(n):
    sum = 1
    root = int(math.ceil(math.sqrt(n)))
    for i in range(2, root):
        if n % i == 0:
            sum += i + (n / i)
    if root > 1 and root * root == n:
        sum += root
    return sum

def trouveur(n):
    sommeMap = {}
    t = []
    for i in range(1, n + 1):
        s = somme_diviseurs_propres(i)
        if s in sommeMap:
            sommeMap[s].append(i)
        else:
            sommeMap[s] = [i]

    for i, ps in sommeMap.items():
        for p in ps:
            if i < p and p in sommeMap and i in sommeMap[p]:
                t.append([i, p])
    return t

print "Result: {}".format(trouveur(100000))


Vertical cycle slideshow I want to make a vertical cycle slideshow. This works perfectly for me but I'm just wondering if it's possible to achieve the same functioning with a better code. I don't know if my code would provoke a problem in some situations or not.

jsFiddle

var pare = $('#slider'); // selecting the holder element;
var ob = $('#slider > li'); // selecting items to handle;
var elm_show = 3; //  number of imgs to show at once;
var spd = 1000; // spd of scrolling;

var nex = ob.eq(0).height();
var leng = ob.length;
var jleng = leng-1;

ob.eq(jleng).css('top',-1*(leng*nex));// returning the last item to the first case;
pare.height(nex*elm_show); // define the holder length to display the desired number of item to be shown;

function anime(){
ob.each(function(i,item){$(item).animate({"top":"+="+nex+"px"},spd,

    "linear",function(){if ($(item).position().top == jleng*nex)
   {$(item).css('top',-1*nex*(i+1))}});})
 setTimeout(anime, 0);
 }

 anime();

I found your code a little hard on the eyes because of the lack of formatting.

function anime(){
ob.each(function(i,item){$(item).animate({"top":"+="+nex+"px"},spd,

  "linear",function(){if ($(item).position().top == jleng*nex)
{$(item).css('top',-1*nex*(i+1))}});})
setTimeout(anime, 0);
}

Here is what it looks like with good formatting

function anime(){
    ob.each(function(i,item){
        $(item).animate(
            {"top":"+="+nex+"px"},
            spd,
            "linear",
            function(){
                if ($(item).position().top == jleng*nex) {
                    $(item).css('top',-1*nex*(i+1))
                }
            }
        );
    })
    setTimeout(anime, 0);
}


Run the needle program for a query against the subject files and return the result I have written the script to run locally the needle program for a query against the subject files. It returns the highest percentage result file as a string. In addition it also converts to html file (write_html function).

Can you help me to improve the script?

subject_path = settings.MEDIA_ROOT+"subject_files/"
query_path = settings.MEDIA_ROOT+"query_file/"

def run_bug():
    """The script runs the needle program for a query file against all the subject files in a folder and keeps only
    the highest identity score alignments.
    """

    query_list_fastafiles, subject_list_fastafiles = start_function()
    files_ending_with_one = filter_files_ending_with_one(subject_list_fastafiles)

    for j in range(len(query_list_fastafiles)):
        initial = 0
        needle_alignment_result = ''

        #take the scaffold sequence one by one
        for i in range(len(files_ending_with_one)):

            #path of the files
            file1 = query_path+ordered_query_fastafiles[j]
            file1 = subject_path+files[i]
            needle_output = run_needle_two_sequences(file1, file2)

            if needle_output == None:
                return needle_alignment_result = needle_output
            else:
                #print(ordered_query_fastafiles[j],ordered_scaffold_fastafiles[i])
                identity_percentage, result_file_as_string = needle_output
                #print(identity_percentage, results)
                identity_percentage = float(identity_percentage)

                if identity_percentage > initial:
                #keeps the highest percentage result among the files
                    initial = identity_percentage
                    needle_alignment_result = result_file_as_string

    write_html(needle_alignment_result)
    return needle_alignment_result

A very short review, as I don't have the time for a more extensive one.

Enumerate

Some things I see right off the bat is range(len()). Instead of writing this, consider using enumerate. This will allow you to work with any iterable, not just countable, indexable objects.

Unnecessary else

I see this piece of code next:

if needle_output == None:
    return needle_alignment_result = needle_output
else:
    ... code here ...

The else here is unnecessary, as you exit the function after returning in the if. Just move the code in the else right after the if, and remove the unnecessary else, like so:

if needle_output == None:
    return needle_alignment_result = needle_output
... code here ...

is None vs == None

Below is from an amazing StackOverflow answer.

  A class is free to implement
  comparison any way it chooses, and it
  can choose to make comparison against
  None means something (which actually
  makes sense; if someone told you to
  implement the None object from
  scratch, how else would you get it to
  compare True against itself?).

Practically-speaking, there is not much difference since custom comparison operators are rare.  But you should use is None as a general rule.

Updated Code

def run_bug():
    """
    The script runs the needle program for a query file against all the
    subject files in a folder and keeps only the highest identity score alignments.
    """
    needle_alignment_result = ''

    query_list_fastafiles, subject_list_fastafiles = start_function()
    files_ending_with_one = filter_files_ending_with_one(subject_list_fastafiles)

    for j, _ in enumerate(query_list_fastafiles):
        initial = 0

        # Take the scaffold sequence one by one
        for i, _ in enumerate(files_ending_with_one):

            # Path of the files
            file1 = query_path + ordered_query_fastafiles[j]
            file1 = subject_path + files[i]
            needle_output = run_needle_two_sequences(file1, file2)

            if needle_output is None:
                return needle_alignment_result == needle_output

            identity_percentage, result_file_as_string = needle_output
            identity_percentage = float(identity_percentage)

            # Keeps the highest percentage result among the files
            if identity_percentage > initial:
                initial = identity_percentage
                needle_alignment_result = result_file_as_string

    write_html(needle_alignment_result)
    return needle_alignment_result


Enable or disable two icons I need two icons:

If the email is verified, show an icon without a hyperlink and set an "activated" CSS class for green style and checked.
If the email is not verified, the icon is linked to the verify page, and the default is a gray color style.

Which of these solutions is better and standard?

Solution #1:

<a href="<?php echo ($modelStatic->isVerifiedEmail) ? 'javascript:void(0)' :
    Yii::app()->createUrl('/user/reActivate'); ?>"
   class="item<?php if($modelStatic->isVerifiedEmail) echo ' activated'; ?>">

        <?php echo Yii::t('app', 'Verify Email'); ?>

Solution #2:

if($modelStatic->isVerifiedEmail) : ?>

            <?php echo Yii::t('app', 'Verify Email'); ?>

<?php else: ?>
    <a href="<?php echo Yii::app()->createUrl('/user/reActivate'); ?>"
       class="item">

            <?php echo Yii::t('app', 'Verify Email'); ?>

<?php endif; ?>

Solution #3:

<?php if($modelStatic->isVerifiedEmail) : ?>

<?php else: ?>
    <a href="<?php echo Yii::app()->createUrl('/user/reActivate'); ?>" class="item">
<?php endif; ?>

            <?php echo Yii::t('app', 'Verify Email'); ?>

<?php if($modelStatic->isVerifiedEmail) : ?>

<?php else: ?>

<?php endif; ?>

I don't know about standard, so I'll just say what I see.

I found that the second is cleanest of all three and easiest to read.  The logic flow is very simple, with just one simple if/else.  However, it is not DRY, with all the problems that that entails.

the first and third are both a bit more complex.
the third also has the same if block defined twice (once at the beginning and once at the end), which makes it also not DRY

regarding the first, I personally don't like the idea of creating an a tag that gets javascript:void(0). It seems that it will just add clutter to the rendered page.

The other problem (with all three) is that you have your logic mixed in to the html. I would recommend something like as follows:

<?php
    //first define all the variable items
if($modelStatic->isVerifiedEmail) :
    $startTag = '
';
    $endTag = '
';
else:
    $startTag = '<a href="<?php echo Yii::app()->createUrl('/user/reActivate'); class="item">'
    $endTag = '';
endif; ?>

     <?=$startTag?>

            <?php echo Yii::t('app', 'Verify Email'); ?>

     <?=$endTag?>


Checking if multiple user-entered numbers are in the right range str, ski = int(input("text here")), int(input("text here"))

    if str < 1 or str > 50 or ski < 1 or ski > 50:
        print("did not enter between 1 and 50")

Is there anyway to shorten the if statement in that code? I find that it is a bit too long, but I don't really know how to shorten it.

Any suggestions?

This snippet is not very useful. You should not let the user continue after giving you wrong input; rather, as soon as an incorrect number is entered, you should figure out what to do next.
(As you have not included any context in your question, this is left as an exercise to you).
Your names are extremely poor:
str is a library function and should therefore never be used as a variable name.
str and ski are useless as they do not reveal what the variables are about.
Be expressive.
You are repeating yourself on a small scale: you are writing int(input(...)) twice and the two portions of the if statement follow the same logic. Get rid of the duplication.

Consider these improvements applied to your code:

Use list comprehensions and generator expressions to your advantage
Use the all(...) function with a generator expresssion to verify that all answers are in the specified range

prompts = ('text here', 'text here')
answers = [int(input(p)) for p in prompts]
if not all(0 < a <= 50 for a in answers):
    print('did not enter between 1 and 50')

# if you need to access the answers individually, use
first, second = answers


Custom log4net raising application-level events Use Case: I have a form application in which I log events to a file/database and have a need to also raise those events to the application itself (for display purposes).  Think old school updating status bar with the same content that you will log to a file.

The solution I put in place is a custom log4net class (since we're already using that for the actual file and databases logging).  This simple class just raises an event when the message payload of the caller.  This way I can use a single call to the log instead of multiple calls to different loggers.

This is a working class, I'm just looking for input on pitfalls on the code or potential performance issues that might be found (and to also share the idea for others that need it).  I'm also wondering missing some best practices or something simple that I've overlooked.

I the case of my requirements, this event may be called 30 times/sec across multiple different threads.

MemoryQueueEventArgs isn't the actual final class name, it was another direction I was going to go, I just haven't renamed it yet.

public class MemoryQueueEventArgs : EventArgs
{
  // Customize to your business requirements
  public DateTime CreateDate { get; set; }
  public string Message { get; set; }
  public string Level { get; set; }
  public string LoggerName { get; set; }
  internal int Count { get; set; }
}

public class MemoryQueueAppender : AppenderSkeleton
{
  private object objectLock = new Object();

  #region Public Methods
  public static MemoryQueueAppender GetActiveAppender(Type type)
  {
    IAppender[] appenders = log4net.LogManager.GetLogger(type).Logger.Repository.GetAppenders();
    return (appenders
     .FirstOrDefault(a => a.GetType().Name == "MemoryQueueAppender") as MemoryQueueAppender)
    ;
  }

  protected event EventHandler<MemoryQueueEventArgs> _LogUpdate;
  public event EventHandler<MemoryQueueEventArgs> LogUpdate
  {
    add
    {
      lock (objectLock)
      {
        _LogUpdate += value;
      }
    }
    remove
    {
      lock (objectLock)
      {
        _LogUpdate -= value;
      }
    }

  }
  #endregion Public Methods

  protected override void Append(log4net.Core.LoggingEvent loggingEvent)
  {
    MemoryQueueEventArgs mqea = _CreateMemoryQueueEventArgs(loggingEvent);
    this.OnLogUpdate(mqea);
  }

  protected override void Append(log4net.Core.LoggingEvent[] loggingEvents)
  {
    base.Append(loggingEvents);
  }

  #region Private Methods

  private MemoryQueueEventArgs _CreateMemoryQueueEventArgs(log4net.Core.LoggingEvent loggingEvent)
  {
    return new MemoryQueueEventArgs
    {
      CreateDate = DateTime.Now,
      Level = loggingEvent.Level.Name,
      LoggerName = loggingEvent.LoggerName,
      Message = RenderLoggingEvent(loggingEvent),
    };
  }

  protected virtual void OnLogUpdate(MemoryQueueEventArgs e)
  {
    EventHandler<MemoryQueueEventArgs> handler = _LogUpdate;
    if (handler != null)
    {
      handler(this, e);
    }
  }

  #endregion Private Methods

}

Calling Code:

public TestDevelopment()
{
  MemoryQueueAppender appender = MemoryQueueAppender.GetActiveAppender(this.GetType());
  appender.LogUpdate += appender_LogUpdateEvent;
}

void appender_LogUpdateEvent(object sender, MemoryQueueEventArgs e)
{
  System.Diagnostics.Debug.WriteLine(string.Format("[{0}] {1}", e.Level, e.Message));
}

I log events to a file/database and have a need to also raise those events to the application itself (for display purposes).

Generally this is not a bad idea but...

  I the case of my requirements, this event may be called 30 times/sec across multiple different threads.

with this many log events per second it is. No human can read it anymore unless you are writing applications for Jedis.

Use a custom named logger and send to the UI only those messages that have some value to the user not just everything that gets logged.

Or create some kind of a filter that reduces the number of messages to a reasonable count.


Table join using the repository pattern I am new to repository pattern. If I want to join two tables, where should I implement the logic? I have implemented it as shown below. Is there any better way to achieve the same?

This is my UserRepository class:

public class UserRepository : Repository<UserMaster>, IUserRepository
{
    public UserRepository(DbContext context)
        : base(context)
    {

    }
    public UserMaster GetById(int id)
    {
        return FindBy(x => x.Userid == id).FirstOrDefault();
    }

}

This is the Service layer from which I am implementing data access functions:

 public class UserService : EntityService<UserMaster>, IUserService
{
    IUnitOfWork _unitOfWork;
    IUserRepository _userRepo;
    IRolePrivilegeMapRepository _rolePrevilegeMapRepo;
    IUserRoleMapRepository _userRoleMapRepo;

    public UserService(IUnitOfWork unitOfWork, IUserRepository userRepo, IRolePrivilegeMapRepository rolePrevilegeMapRepo,IUserRoleMapRepository userRoleMapRepo)
        : base(unitOfWork, userRepo)
    {
        _unitOfWork = unitOfWork;
        _userRepo = userRepo;
        _rolePrevilegeMapRepo = rolePrevilegeMapRepo;
        _userRoleMapRepo = userRoleMapRepo;
    }

    public List<int> GetUserPrevileges(string UserName)
    {
        var rolePrevilegeMap = _rolePrevilegeMapRepo.GetAll();
        var userRoleMap = _userRoleMapRepo.GetAll();
        var userMaster = _userRepo.GetAll();

        var Privs = (from  rpm in rolePrevilegeMap
                     join urm in userRoleMap on rpm.RoleId equals urm.Roleid
                     join um in userMaster on urm.Userid equals um.Userid
                     where um.Username.Equals(UserName) && rpm.IsDeleted == false && urm.IsDeleted == false
                     select rpm.PrivilegedId).Distinct();

        if (Privs.Any())
        {
            return Privs.ToList();
        }
        else
        {
            return null;
        }

    }

}

I have injected IRolePrivilegeMapRepository, IUserRoleMapRepository, and IUserRepository using Autofac to the UserService class for joining the tables.

    var rolePrevilegeMap = _rolePrevilegeMapRepo.GetAll();
    var userRoleMap = _userRoleMapRepo.GetAll();
    var userMaster = _userRepo.GetAll();

The GetAll method is of return type IQueryable. After that, using that result, I have done the join. Is this the right way to approach? Or Is there any better way to implement the same?

I don't do EF, so I am focusing on the code only. But nevertheless maybe this will help you in doing joins with EF: how-to-join-multiple-tables-using-repository-pattern-entity-framework

Based on the naming guidelines input parameters should be named using camelCase casing. So

public List<int> GetUserPrevileges(string UserName)

should be

public List<int> GetUserPrevileges(string userName)

The same is true local varaiables like Privs but in addition you shouldn't shorten your varaiables names because you are reducing the readability of your code. So better use privileges instead of Privs.
This if..else

if (Privs.Any())
{
    return Privs.ToList();
}
else
{
    return null;
}

does not add real value. Why do you want to return null? It would be much easier for the calling code if you return an empty List for the case that there aren't any privileges which match the requirement.

If the IQuerable<T> doesn't contain any items, a call to ToList() will just create an empty list.

If you use the returned List<> to just iterate over the items, you won't even need a check if it is empty.