Echo server with CompletableFuture I recently wrote a simple echo server in Java 7 using the NIO APIs so it was asynchronous and non-blocking. Then I decided, as a learning experience, to redo it with Java 8 hoping to use a more functional style and not have nested callbacks. I'm struggling to understand how to use the new CompletableFuture class with the Futures returned by AsynchronousSocketChannel.

This code currently works but is slower than the Java 7 version. If anyone can point out ways to improve it (or maybe that I'm going about it completely wrong) it would be appreciated. The main problem, to me at least, is that I now have to call get() on three Futures, whereas before I didn't have any blocking operations.

try (final AsynchronousServerSocketChannel listener =
    AsynchronousServerSocketChannel.open()) {

        listener.setOption(StandardSocketOptions.SO_REUSEADDR, true);
        listener.bind(new InetSocketAddress("localhost", 8080));

        while (true) {

            AsynchronousSocketChannel connection = listener.accept().get();
            CompletableFuture<AsynchronousSocketChannel> connectionPromise =
                CompletableFuture.completedFuture(connection);

            CompletableFuture<ByteBuffer> readerPromise = CompletableFuture.supplyAsync(() -> {
                ByteBuffer buffer = ByteBuffer.allocateDirect(1024);
                try {
                    connection.read(buffer).get();
                    return (ByteBuffer) buffer.flip();
                } catch (InterruptedException | ExecutionException e) {
                    connectionPromise.completeExceptionally(e);
                }
                return null;
            });

            readerPromise.thenAcceptAsync((buffer) -> {
                if (buffer != null) {
                    try {
                        connection.write(buffer).get();
                        connection.close();
                    } catch (InterruptedException | ExecutionException | IOException e) {
readerPromise.completeExceptionally(e);
                    }
                }
            });
        }

    } catch (IOException | InterruptedException | ExecutionException e) {
        e.printStackTrace();
}

As far as I see supplyAsync and thenAcceptAsync uses ForkJoinPool.commonPool() which does not use more than three threads on my machine (and it depends on the number of available processors). It could be a bottleneck if you have more than three clients at the same time. Both methods can have an Executor argument which would be used instead of commonPool().
The code calls completeExceptionally in the catch block here:

readerPromise.thenAcceptAsync((buffer) -> {
    if (buffer != null) {
        try {
            connection.write(buffer).get();
            connection.close();
        } catch (InterruptedException | ExecutionException | IOException e {
            readerPromise.completeExceptionally(e);
        }
    }
});

It seems to me that it does not have any effect. Javadoc of completeExceptionally says the following:

  If not already completed, causes invocations of get() and related methods to throw the given exception.

The lambda passed to thenAcceptAsync is called when the CompleteFuture (readerPromise in this case) is already completed so it doesn't change anything. Consider the following PoC:

@Test
public void test3() throws Exception {
    final CompletableFuture<String> reader = CompletableFuture.supplyAsync(() -> {
        return "data";
    });
    reader.thenAcceptAsync(x -> {
        System.out.println("reader.thenAcceptAsync: " + x);
        boolean transitionToCompleted = reader.completeExceptionally(new RuntimeException(
                "overridden Future result"));
        System.out.println(transitionToCompleted); // prints "false"
    });

    Uninterruptibles.sleepUninterruptibly(2, TimeUnit.SECONDS);
    System.out.println("reader result: " + reader.get()); // prints "data"
}

(Note that there is an obtrudeException method but I guess you don't need that either.)
The same is true for connectionPromise.completeExceptionally(e), furthermore, connectionPromise seems completely unused in the code above. I'd remove it.
thenAcceptAsync will run only when the the previous stage completes normally. The following also could be useful:

  In all other cases, if a stage's computation terminates abruptly with an (unchecked) exception or error, then all dependent stages requiring its completion complete exceptionally as well, with a CompletionException holding the exception as its cause.

Source: CompletionStage javadoc

According to that you might be able to change the reader lambda to the following:

final CompletableFuture<ByteBuffer> readerPromise = CompletableFuture.supplyAsync(() -> {
    try {
        final ByteBuffer buffer = ByteBuffer.allocateDirect(1024);
        connection.read(buffer).get();
        buffer.flip();
        return buffer;
    } catch (final Exception e) {
        throw new RuntimeException(e);
    }
});

It never returns null, so you can remove the null check from writer lambda:

final CompletableFuture<Void> writerPromise = readerPromise.thenAcceptAsync((buffer) -> {
    try {
        connection.write(buffer).get();
        connection.close();
    } catch (Exception e) {
        throw new RuntimeException(e);
    }
});

Finally, if there's an exception, you could log that with the following:

writerPromise.exceptionally(x -> {
    x.printStackTrace(); // TODO: use logging instead
    return null;
});

(I'm not familiar with these APIs but this looks right for me.)
Note that I've removed the ugly casting with changing

return (ByteBuffer) buffer.flip();

to

buffer.flip();
return buffer;

Calling Future.get() in the lambdas does not seem to asynchronous for me too (althought they're run by another threads) but I've not idea how should it be handled. The two APIs does not seem compatible. (I guess you could get better answers on Stack Overflow with a more specific question.)


Login function in PHP I've been working on a simple login script. The registration part is still to be coded, but I'm done the login part. Are there any security holes in my code? I'm using SHA-512 right now but I think I'll change it to mcrypt later.

Class User
{
    private $db;

    public function __construct($db)
    {
        $this->db = $db;
    }

    public function Login($user_email, $user_pass, $remember = false)
    {
        $user_pass = sha1($user_pass);
        $query = $this->db->prepare("SELECT id, password FROM users WHERE email = :email");
        $query->bindValue(':email', $user_email);
        $query->execute();

        if($query->rowCount() > 0) {
            $user = $query->Fetch(PDO::FETCH_OBJ);
            if($user->password == $user_pass) {
                $_SESSION['loggd_id'] = $user->id;
                if($remember == true) {
                    setcookie('lggd_sess', hash('sha512', uniqid()), 84600);
                    return true;
                }
                return true;
            } else {
                return 'Incorrect Email/Password Combination.';
            }
        } else {
            return 'Incorrect Email/Password.';
        }

    }
}

I'm using SHA-512 right now but I think I'll change it to mcrypt later.

Neither of those are the proper tool for the job. Mcrypt is meant for symmetric encryption (AES, TwoFish, DES, etc.), not password hashing.

If you're running PHP 5.5+, use password_hash() and password_verify()

Otherwise, https://github.com/ircmaxell/password_compat is the way to go.

EDIT: Example code:

<?php
Class User
{
    private $db;

    public function __construct($db)
    {
        $this->db = $db;
    }

public function Login($user_email, $user_pass, $remember = false)
{
        $query = $this->db->prepare("SELECT id, password FROM users WHERE email = :email");
        $query->bindValue(':email', $user_email);
        $query->execute();

        if($query->rowCount() > 0) {
            $user = $query->Fetch(PDO::FETCH_OBJ);
            if (password_verify($user_pass, $user->password)) {
                $_SESSION['loggd_id'] = $user->id;
                if($remember == true) {
                    setcookie('lggd_sess', hash('sha512', mcrypt_create_iv(64, MCRYPT_DEV_URANDOM)), 84600);
                    return true;
                }
                return true;
            } else {
                return 'Incorrect Email/Password Combination.';
            }
        } else {
            return 'Incorrect Email/Password.';
        }
    }
}

When you insert a row in the DB, you want to use password_hash($user_password)


Returning a validation error from a command handler public class AddPostCommand
{
    public string Title { get; set; }
    public string Body { get; set; }
    public bool Published { get; set; }
}

public class AddPostCommandHandler
{
    public string Handle(AddPostCommand command)
    {
        var param = new
        {
            Slug = command.Title.Replace(" ", "-"),
            PublishDate = command.Published ? DateTime.Now : DateTime.MinValue,
            command.Title,
            command.Body,
            command.Published
        };

        // I removed the data access code for brevity.

        return param.Slug;
    }
}

I have a command and I have a command handler. I also have a controller that invokes the command handler.

Within the command handler I generate a slug (definition). In the database, the slug column has a unique constraint - there must be no duplicate slugs.

I need to find a clean way to inform the caller that they have effectively provided to the command handler a duplicate slug.

I do not perform any validation in the command handler yet. This is the simplest solution I can come up with:

PostRequestHandler requestHandler = new PostRequestHandler();
AddPostCommandHandler commandHandler = new AddPostCommandHandler();

[HttpPost]
public ActionResult AddPost(AddPostCommand command)
{
    string expectedSlug = command.Title.Replace(" ", "-");
    PostRequest request = new PostRequest { Slug = expectedSlug };
    PostModel post = requestHandler.Handle(request);
    if (post != null)
    {
        ModelState.AddModelError("", "A post with this title already exists.");
        return View(post);
    }

    string actualSlug = commandHandler.Handle(command);
    return RedirectToAction("Index", "Post", new { actualSlug });
}

I abhor this code.

My most pressing concern is that I do not want the caller to have to generate the slug. I am also concerned that the controller action is becoming too big.

How might I communicate to the controller that an error has occurred (so that an error message can be displayed to the user) in a clean way?

This is an interesting question because I've never seen the MVC and the command pattern implemented together.  Typically I think WebForms when I think of the command pattern.  I'm still on the fence on whether or not it is a good fit but there are a couple of MVC concepts you can use to make your code cleaner.  The most valuable of which in your case is to:

Leverage Custom Model Binding

I agree, your controller shouldn't have to do work to build the request model (slug, post request), this should all be happening in a custom model binder:

public class AddPostCommandModelBinder : DefaultModelBinder
{

    protected virtual void OnModelUpdated(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
        var command = (AddPostCommand)bindingContext.Model;
        // Add the ExpectedSlug property to the request command and have the ModelBinder do this work (as well as any other post-binding logic
        command.ExpectedSlug = command.Title.Replace(" ", "-");
    }
}

In addition to this, your PostRequestHandler is a concern much better solved using Model Binding, as the responsibility of Model Binding is to take request parameters and map them to a strongly typed object as well as validating those inputs (by default using the DataAnnotations namespace: http://msdn.microsoft.com/en-us/library/system.componentmodel.dataannotations%28v=vs.110%29.aspx.

In yet another Model Binder, you can put any custom validation logic, for example:

public override Object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
{
    var newModel = (PostRequest)bindingContext.Model;
    newModel.AttemptedSlug = bindingContext.ValueProvider.GetValue("title").AttemptedValue;

    if(string.IsNullOrWhiteSpace(newModel.AttemptedSlug))
    {
        bindingContext.ModelState.AddModelError("AttemptedSlug", "The Attempted Slug is required.");
    }
    // Other custom logic

    return newModel;
}

A point on Injection

As with anything MVC, the code should be test-able.  I'm not sure if you omitted it for brevity, but the command handlers should be constructor-injected interfaces bound to concrete types (ideally) using an IoC container.  Also, with the changes above, your controller should now look like this:

public class PostController : Controller
{
    IPostCommandHandler _postCommandHandler;

    public PostController(IPostCommandHandler addCommandHandler)
    {
        this._postCommandHandler = addCommandHandler;
    }

    [HttpPost]
    public ActionResult AddPost(AddPostCommand command, PostRequest submitRequest)
    {
        ActionResult result = null;
        if (!submitRequest.IsValid)
        {
            result = View("AddPost", submitRequest);
        }
        else
        {
            string actualSlug = this._postCommandHandler.Handle(command);
            result = RedirectToAction("Index", "Post", new { actualSlug });
        }

        return result;
    }
}

Ultimately, I follow several rules of thumb as far as my separation of concerns that have kept me mostly out of trouble.

Model binders bind all request parameters to strongly typed request objects (with a few exceptions).

Model binders handle basic data-validation and error handling for user input

Controllers accept request models, check for validity, call external data services to build response view models, and determine action (redirect, view, etc...)

Other Notes

The usage of the word "post" as part of your domain (presumably you're using the term in the sense of a blog "post"), should be re-considered.  POST carries a lot of context with it and some developers (myself included) will have to read around the common use of post when reading this code.

Consider renaming post to blogPost or forumPost or newsPost to make the code clearer.

Adding validation logic in the controller should be a red flag, but adding messaging in the controller is perfectly appropriate, as the messaging is directly related to the view (which is found, prepared and delivered by the controller).  I wouldn't do a null check as you have done, but either use a validator in the model binder (ModelState.IsValid), or providing your own as I did above as part of the PostRequest object.

You may also want to consider adding an AddPostCommand property to your PostRequest, which would then require your controller to take only one argument.  You can still have a custom model binder for each type.


Determining possible triangle from lengths of line segments This Codility challenge is to take an array of integers representing lengths of line segments, and determine whether or not it is possible to choose three distinct segments to form a triangle.  (The sum of the lengths of any two sides must exceed the length of the remaining side.)  Assume that A contains at most one million ints.

Complexity requirements:

Expected worst-case time complexity is O(n log(n));
Expected worst-case space complexity is O(n), not counting the input array A itself.

This is my solution:

import java.util.Arrays;
class Solution {
    public int solution(int[] A) {
        // write your code in Java SE 6
        int n = A.length;

        if (n>1000000000) return -1;

        Arrays.sort(A);
        int p=0;
        int q=p+1;
        int r=q+1;
        for (;r<n;p++,q++,r++){
            if(A[p]+A[q]>A[r] && A[q]+A[r]>A[p] &&  A[r]+A[p]>A[q])
               return 1;
        }
        return 0;
    }
}

I don't how to achieve 100% correctness. I get only 93%.

As has been pointed out, you must do several things to ensure you get the maximum marks.

Ensure that negative numbers are checked for.
Handle the MAX_INT overflow issue which could arise.

  You know 4 + 5 > 7 is same as 5 > (7 - 4). Yes I know you are grinning ;)

This suggested solution is correct for this.

These two individual points should allow you to achieve the maximum score. Now I would just like to quickly go over some theory behind why this solution works.

Time complexities

It seems people keep asking you to address the "consecutive elements". You have already done this through the use of Arrays.sort(). This Java sort algorithm is referred to as a "tuned Quicksort" ( See http://docs.oracle.com/javase/6/docs/api/java/util/Arrays.html#sort(int[]) ).

  The sorting algorithm is a tuned quicksort, adapted from Jon L. Bentley and M. Douglas McIlroy's "Engineering a Sort Function", Software-Practice and Experience, Vol. 23(11) P. 1249-1265 (November 1993). This algorithm offers n*log(n) performance on many data sets that cause other quicksorts to degrade to quadratic performance.
  Quicksort is well known for its O( n log n ) time complexity (excluding extreme cases).

After this you do a single iteration over the array, which is considered O(n). When we add the two time complexities together we are given O( 2n log n) where constants are considered insignificant for Big-O notation. So to clarify this simplifies down to O(n log n) which was the upper bound of the problem.

How sorting solves non-consecutive elements

Once the array is sorted it is true that only consecutive triples need to be considered.

Consider the following array: {1,2,6,6,8}

Whilst it may be true that (1, 6, 6) is triangular, and is non-consecutive. It can also be proven that if that value is triangular then any number which lies between the initial 1, and 6 will also be triangular.

From this we can see that in the sort array, for each non-consecutive triangular triples which exists there also exists an consecutive triple.

Simplification from sorting

The sort operation allows you to simplify you checks later, and decrease execution time. When checking for negative number, you only need check the lowest number in the triple, as if this is >= 0 it is given the follow values must be >= that initial value.

This mean you only need check one value for negativity, not three.

This can be extended further into the actual triangular checks. If it is true that the smallest two numbers in the triple are larger than the largest number when summed, then all three statements are true.

  1) A[P] + A[Q] > A[R]

  2) A[Q] + A[R] > A[P]

  3) A[R] + A[P] > A[Q]

Remembering the algorithm change to this, so these would be evaluated as

A[P] > (A[R] - A[Q])

Because we can guarantee that A[P] < A[Q] < A[R] due to A being sorted, we only need perform the one check above.


Please help and improve this script for displaying search results I wrote a search script, now I want a more efficient way of finding the keywords used for the search and make them bold. I used this code but it I feel it could be greatly improved on. Thanks for any suggestion.

// I use this code to select the lines to display
$lines = explode('.', trim($search_result));

// Break up the result into sentences
$j = 0; $line = '';
for($i = 0; $i < sizeof($lines); $i++) {
    // Look for the keywords in each of the line
    if (strstr($lines[$i], $keywords[0]) || strstr($lines[$i], $keywords[1]) ||
        strstr($lines[$i], $keywords[2]) || strstr($lines[$i], $keywords[3])
    ) {
        $j++;
        $line .= trim($lines[$i])."...";
    }

    if ($j == 2)
        return $line;

    // If no keyword is found in first 100 lines/sentences, just display the first 3 lines
    if ($i == 100)
        return $lines[0]."...".$lines[1]."...".$lines[2];

}

I use this code to make the keywords bold

function findAndReplace($keywords, $sentence) {
    for ($i=0; $i<sizeof($keywords); $i++)
        $words[] = "<b>".ucfirst($keywords[$i])."</b>";

    return str_ireplace($keywords, $words, $sentence);
}

A couple of small things...

I would replace this:

if (strstr($lines[$i], $keywords[0]) || strstr($lines[$i], $keywords[1]) ||
    strstr($lines[$i], $keywords[2]) || strstr($lines[$i], $keywords[3])

with something like:

foreach($keyword in $keywords) {
    if (strstr($lines[$i]) {
       $j++;
       $line .= trim($lines[$i])."...";
       break;
    }
 }

Also, this line:

       $line .= trim($lines[$i])."...";

will result in an extra trailing "..." when $j gets to 2.  I'm presuming you want something like "line1...lineN", but instead you'll get "line1...lineN...".  You probably want to chop off those extra dots, or only add them when needed.


Custom login system (PDO, sanitizing, hash) I have made a custom login form for my site, with the help of PDO (until now I used simple MySQL connection and sanitizing), and it looks like this:

session_name('NEWCOOKIE');
session_start();

require_once "config.php";

$member_username = $_POST['username'];
$member_password = $_POST['password'];
    $crypt_pass = crypt($member_password,"somesalt");

try{
    $dbh = new PDO('mysql:host=localhost;dbname='.DB_NAME, DB_USERNAME, DB_PASS, array(PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8"));
    $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch (PDOException $e) {
    echo "Fatal error.";
    file_put_contents('PDOErrors.txt', $e->getMessage(), FILE_APPEND);
}

$sth = $dbh->prepare("SELECT * FROM ".DB_PREFIX."_users WHERE username = :user AND password = :pass");
$sth->bindParam(':user', $member_username);
$sth->bindParam(':pass', $crypt_pass);
$sth->execute();
$total = $sth->rowCount();
$row = $sth->fetch();

if($total > 0){
    if($row['activated']){
        $_SESSION["user_username"] = $member_username;
        $_SESSION["user_logedIn"] = true;
        $_SESSION["user_id"] = $row['user_id'];
        $_SESSION["timeout"] = time();
        header("location: index.php");
    }
    else{
        echo "ACCOUNT NOT ACTIVE";
    }
}
else{
    echo "WRONG PASSWORD OR USERNAME";
}

Is this safe? I know that nothing is 100% safe, but I have a small number of users (around 200-300), and 1000 visitors a month, so I don't expect very "professional" attacks and programming experts.

My questions are:

As you can see, I don't use sanitizing at all.  I thought that PDO will do that part of the job, or I am wrong?
I hope that crypting of password is good (with no MD5 or SHA, is crypt() recommended)?
File config.php contains username and pass of my database. Should I protect it somehow, what permissions to set?
Any other (serious) problem with this code?

Well, let's see:

You're using PDO and prepared statements, no risk of SQL injection there. Great!
Your PDO code may throw an Exception (Specifically, a PDOException) at any time, so the whole database code block should be kept inside of the try/catch block.
crypt in on itself isn't 100% secure. See This question for more details.
Database credentials are not constant. They can change over time, and you may want to change the server, change the user, add an additional database etc in the near/far future. The solution is to use functions (or better yet, classes) and pass the credentials as variables, rather then applying them as app-wide constants.


Largest rectangle areas after drawing boundaries I was asked the following question while trying to get a Hackerrank certificate. I couldn't solve on time but I now completed the solution. Can you please check if my solution seems good? Suggestions on improvement are appreciated.
Q: You have a rectangle of width w and height h. You will start to draw boundaries. The number of boundaries is given by the length of the given array, distance. isVertical[i] represents whether or not i-th boundary is vertical or not. 1 means vertical and 0 means horizontal. distance[i] tells the distance between your boundary and the rectangle. In case of a horizontal boundary, it represents the distance from the bottom of your rectangle, and in case of a vertical boundary, it represents the distance from the left of your rectangle.
You need to return the largest areas of your rectangle as an array after drawing each of your boundaries.
ex) w = 4, h = 4, isVertical = [0,1], distance = [3,1]
After drawing the first boundary, the largest possible rectangle area is 12.
After drawing the second boundary, the largest possible rectangle area is 9.
You are not given a new rectangle for each boundary. You draw boundaries on the same rectangle.
You should return [12, 9].
My solution:

I created one grid using a 2d-array and computed the largest possible rectangle area after each boundary is drawn by choosing the larger between the area taken up by the boundary and the current largest rectangle area minus the boundary area. I used the same, one grid to do computation for every boundary.

function largestArea(w, h, isVertical, distance) {
    if (isVertical.length !== distance.length) return null;
    if (w == 0 || h == 0) return 0;
    let grid = new Array(h).fill(0).map(() => new Array(w).fill()); // -1 represents areas of the original rectangle
    let largestArea = w*h;
    let areas = [];
    for (let i = 0; i < isVertical.length; i++) {
        let boundaryArea = 0;
        if (isVertical[i] == 1) {
            boundaryArea = drawVerticalBoundary(distance[i], grid); // distance[i] for vertical boundary is distance from the left
        } else {
            boundaryArea = drawHorizontalBoundary(h-distance[i], grid); // distance[i] for horizontal boundary is distance from the bottom
        }
        largestArea = Math.max(largestArea-boundaryArea, boundaryArea);
        areas.push(largestArea);
    }
    return areas;
}

function drawVerticalBoundary(maxCol, grid) {
    let area = 0;
    for (let row = 0; row < grid.length; row++) {
        for (let col = 0; col < maxCol; col++) { // area already taken by another boundary is not part of current boundary
            if (!grid[row][col]) {
                area++;
                grid[row][col] = 1;
            }
        }
    }
    return area;
}

function drawHorizontalBoundary(maxRow, grid) {
    let area = 0;
    for (let row = 0; row < maxRow; row++) {
        for (let col = 0; col < grid[0].length; col++) {
            if (!grid[row][col]) { // area already taken by another boundary is not part of current boundary
                area++;
                grid[row][col] = 1;
            }
        }
    }
    return area;
}

I do have a solution for this question but apology because I am proficient in python
please add the answer who can write the same logic in JS also.
Thanks
what I have done is maintain array of width and height and get amximum width difference and height difference between adjcent location of H and W. which leads to maxArea of rectangle
This is not efficient solution as it was giving timelimit exceed on 3-4 test cases.so appreciate if anyone can optimize it.
def getMaxArea(w, h, isVertical, distance):
    # Write your code here
    def insort(test_list, k):
        for i in range(len(test_list)):
            if test_list[i] > k:
                break
        return test_list[:i] + [k] + test_list[i:]

    maxy = 0
    wline = [0,w]
    hline = [0,h]
    res = []
    for i in range(len(isVertical)):
        if isVertical[i]:
            wline = insort(wline, distance[i])
        else:
            hline = insort(hline, distance[i])
        wmax = 0
        hmax = 0
        for i in range(1,len(wline)):
            if wline[i] - wline[i-1] > wmax:
                wmax = wline[i] - wline[i-1]
        for i in range(1,len(hline)):
            if hline[i] - hline[i-1] > hmax:
                hmax = hline[i] - hline[i-1]

        res.append(hmax * wmax)
    return res


Removing linked list nodes I have this code to remove nodes in a linked list which have odd values. This works well but I want to fine tune it for better reading. Can you please suggest improvements?

public class MyLinkedList
{
    public int data;
    public MyLinkedList next;

    public MyLinkedList(int v)
    {
        this.data = v;
    }

    //public void add(int i)
    //{
    //    this.next = new MyLinkedList(i);
    //    this.next.next = null;
    //}

    public void printLL()
    {
        MyLinkedList mmm = this;
        while (mmm != null)
        {
            Console.Write(mmm.data);
            Console.Write("->");
            mmm = mmm.next;
        }
    }
}

public static void deleteOdd(ref MyLinkedList mll)
{
    MyLinkedList head, previous, curr;
    head = mll;
    while (head.data % 2 != 0)
    {
        head = head.next;
    }
    mll = head;

    previous = head;
    curr = previous.next;

    while (curr != null)
    {
        if (curr.data % 2 != 0)
        {
            previous.next = curr.next;
            curr = previous.next;
        }
        else
        {
            previous = previous.next;
            curr = curr.next;
        }
    }
}

static void Main(string[] args)
    {
        MyLinkedList myll1 = new MyLinkedList(1); MyLinkedList myll2 = new MyLinkedList(3);
        MyLinkedList myll3 = new MyLinkedList(4); MyLinkedList myll4 = new MyLinkedList(4);
        MyLinkedList myll5 = new MyLinkedList(5); MyLinkedList myll6 = new MyLinkedList(6);
        MyLinkedList myll7 = new MyLinkedList(5); MyLinkedList myll8 = new MyLinkedList(6);
        MyLinkedList myll9 = new MyLinkedList(6); MyLinkedList myll10 = new MyLinkedList(9);
        MyLinkedList myll11 = new MyLinkedList(11); MyLinkedList myll12 = new MyLinkedList(12);

        myll1.next = myll2; myll2.next = myll3; myll3.next = myll4; myll4.next = myll5;
        myll5.next = myll6; myll6.next = myll7; myll7.next = myll8; myll8.next = myll9;
        myll9.next = myll10; myll10.next = myll11; myll11.next = myll12; myll12.next = myll12; myll12.next = null;

    myll1.printLL();
    Console.WriteLine();
    deleteOdd(ref myll1);
    Console.WriteLine();
    myll1.printLL();
}

Simple Commentary

The MSDN provides some naming conventions, the relevant bit being that public members should be ProperCamelCase (e.g. Data, not data).
Public Members, and types, should ideally have at least minimal inline documentation (\\\) to make it perfectly clear how to consume the API.
It's usually for the best to strip out old (commented) code. It has a habit of not being maintained and becoming incompatable with the code that is there. It also adds clutter.
For 'export' code, it's rarely useful to have methods which explicitly write to the console: much more useful to have something that writes to an abstract TextWriter (which can take Console.Out or a StreamReader), or that just returns a string. These give the consumer many more options.
deleteOdd(ref MyLinkedList) probably can't cope with empty lists (being passed them or returning them).
printLL isn't a great variable name: what is "LL"? It's a member of MyLinkedList, so the "LL" bit is redundant; I spect a while trying to work out if it was something to do with printing left to right or something.
mmm isn't a great variable name either: I can only assume it was meant to be mll!

Alternative implementation

Below are some methods which just about tally with yours, but using LinkedList<T>, and incorporating some of the suggestions above.

I don't know if you have a particular reason for needing a custom LinkedList implementation (not suggested by the code), but you can implement your methods tidily with .NET's built in LinkedList<T>, which could be argued to provide a nicer API. Crucially, it wraps up a linked list of LinkedListNode<T>s under a LinkedList<T>, which allows some (illusion of) separation of concerns, and spares you having to keep track of the head of the list explicitly. It also allows you to have an 'empty' list, which is good.

Delete Odd

This method is considerably shorter than before, and doesn't require a ref parameter. This works with empty lists. This is a much more understandable implementation, and much harder to 'get wrong', because of the API for LinkedList<T>.

/// <summary>
/// Removes odd elements from a LinkedList of integers
/// </summary>
public static void DeleteOdd(LinkedList<int> ll)
{
    LinkedListNode<int> cur = ll.First; // grab first node

    while (cur != null)
    {
        var next = cur.Next; // make a note of the next node (will be null if cur is the last element)

        if (cur.Value % 2 != 0)
        {
            ll.Remove(cur); // remove the current node if odd
        }

        cur = next; // advance to the next node
    }
}

printLL

Here are two possible alternatives, which don't depend explicitly on Console: one just produces a string, the other prints to a TextWriter (sadly there is no better interface). These are both extension methods for LinkedList<T>, but there is no particular need for this.

/// <summary>
/// Renders a linked list as a string
/// </summary>
public static string Render<T>(this LinkedList<T> ll)
{
    StringBuilder sb = new StringBuilder();

    foreach (var e in ll)
    {
        sb.Append(e.ToString());
        sb.Append("->");
    }

    return sb.ToString();
}

/// <summary>
/// Prints a linked list to a TextWriter, optionally appending a NewLine
/// </summary>
public static void Print<T>(this LinkedList<T> ll, TextWriter textWriter, bool newLine = true)
{
    foreach (var e in ll)
    {
        textWriter.Write(e.ToString());
        textWriter.Write("->");
    }

    if (newLine)
        textWriter.WriteLine();
}

I wouldn't necessarily condone having the newLine parameter being optional, or indeed being there at all: this is just for illustration of some options. It would probably be better to have both Print and PrintLine if you need both.

Example usage

Note the nicer API for building a LinkedList<T>.

private static void ExampleUsage()
{
    LinkedList<int> ll = new LinkedList<int>();
    ll.AddLast(1);
    ll.AddLast(2);
    ll.AddLast(3);
    ll.AddLast(4);
    ll.AddLast(5);

    Console.WriteLine(ll.Render());
    ll.Print(Console.Out);
    DeleteOdd(ll);
    Console.WriteLine(ll.Render());
    ll.Print(Console.Out);
}


Determine if braces in a string are balanced in Clojure I'm currently learning Clojure, and am working through a few exercises. The exercise that I'm working on right now is accept an arbitrary string and determine if the opening and closing parenthesis, curly brackets, and square brackets correctly nest and line up to each other.

For example, "hello { ( world ) } []" would be an example of a balanced string whereas "foo { [ bar ) [}" would be an example of an unbalanced string.

While I have previous experience in other programming languages like Python and Java, I'm very new to Clojure and functional languages in general, so am uncertain if my code is fully idiomatic or if there's a more cleaner or efficient way to complete the problem.

Here's what I have so far. The approach I've taken is to first filter out any characters that are not opening or closing braces. I then loop through the characters, adding the opening brackets to a stack and checking closing brackets against the stack to see if it matches.

Any feedback would be greatly appreciated.

; Given a N different open and close braces in a string ""( { [ } ] )"".
; Write a program to check to see if the braces in the string are balanced.

(ns braces
  (:require [clojure.string :as str]))

(def braces #{\( \) \[ \] \{ \}})

(def pairs { \) \(,
             \] \[,
             \} \{ })

(defn remove-non-braces
  "Takes a string and removes any characters that is not a brace."
  [raw-string]
  (filter #(braces %) raw-string))

(defn is-balanced?
  "Returns true if all the braces in the string are balanced; returns false otherwise."
  ([raw-string] (is-balanced? (remove-non-braces raw-string) []))
  ([string stack]
    (let [[first-char & other] string]
      (cond
        ; if we're done, but the stack is non-empty, we still have opening braces left.
        (empty? string) (empty? stack)
        ; if we encounter a closing brace, check it against the top of the stack.
        (pairs first-char) (if (= (pairs first-char) (peek stack))
                               (is-balanced? other (pop stack))
                               false)
        ; if we encounter an opening brace, add it to the stack.
        :else (is-balanced? other (conj stack first-char))))))

(defn test-strings
  "Tests the given strings and prints the results."
  [& strings]
  (let [format-str (fn [s] (str (is-balanced? s) " : " s))]
    (println (str/join "\n" (map format-str strings)))))

(defn main
  "The main entry point of the program."
  []
  (println "SHOULD PASS")
  (test-strings
    "Hello { ( world ) } [] end"
    ""
    "{[] () {[{{ }}]} ([{}])}")
  (println "")
  (println "SHOULD FAIL")
  (test-strings
    "Hello { world"
    "{{"
    "])"
    "}}{{"
    "{[))"))

(main)

Recursion

Use recur to do tail-call recursion:

  Note that recur is the only non-stack-consuming looping construct in
  Clojure. There is no tail-call optimization and the use of self-calls
  for looping of unknown bounds is discouraged. recur is functional and
  its use in tail-position is verified by the compiler.

Destructuring

You can use :as to avoid the unnecessary let here:

  ([string stack]
    (let [[first-char & other] string]

Instead:

 ([[first-char & other :as string] stack]

Repeated Constants

Here you are entering the same data by hand twice:

(def braces #{\( \) \[ \] \{ \}})

(def pairs { \) \(,
             \] \[,
             \} \{ })

Instead you can do:

(def braces (set (concat (keys pairs) (vals pairs))))

Testing

Use clojure.test to do unit testing. It does have a standard and useful failure reporting. You needn't write a test harness like test-strings yourself. You should only report failures, because test are only useful when they are reporting failures. Most your test should be passing at any time, so any relevant information (a failing test case) should not be lost in noise (dozens of lines of "test-case-xyz passed just as the 937 time before").

Make a habit of putting test code in a separate namespace that does depend on the code tested, but not vice-versa.

After requiring clojure.test:

... (:require [clojure.test :refer :all] ...

You define tests like this:

(deftest non-braces-ignored
  (is (is-balanced? "Hello { ( world ) } [] end")))

(deftest empty-string-is-brace-balanced
  (is (is-balanced? "")))

(deftest some-long-balanced-string
  (is (is-balanced? "{[] () {[{{ }}]} ([{}])}")))

You can group several of similar tests like this until you can find a descriptive name for them, although failure message is not as useful as above tests:

(deftest examples-of-non-balanced-strings
  (are [x] (not (is-balanced? x))
       "Hello { world"
       "{{"
       "])"
       "}}{{"
       "{[))"))

and run these tests like this:

(run-tests)

Namespaces

Don't put function calls inside a namespace. Reloading a namespace should be idempotent.
Also if the function call is something like (main) you don't gain much;
however if you are calling some non-trivial initialization code then it deserves it own function.


PHP Check user and delete if expired Currenty coding a PHP script that will have a CRON to auto run it.
The point is that it checks the user in Mysql and if it has expired, it will delete it from the admin table and set status to expired.
I have used time() and then when it gets INSERT When his trial starts, it starts for example "1347054850" in the "Start" and then i add "900" on it using +, and the result is "1347055750", So the trial is for 15 minutes.
But the point, will this scipt work propery or does it have issues?

<?php
$Sql = new mysqli("localhost", "root", "*******", "serveradmin");
if ($Sql->connect_error) { die("Sorry, Could not connect (".$Sql->connect_errno.") ".$Sql->connect_error);}
$Data = $Sql->query("SELECT * FROM trial");
while ($q = $Data->fetch_assoc()) {
$Start = $q['Start'];
$Stop = $q['Stop'];
$Steam = $q['Steam'];
$Result = $Start - $Stop;
$Time = time();
if ($q['Expired'] == "True") {echo "All expired \n";} else {
if ($Stop <= $Time) {
echo "Deleted ".$Steam." From trial Reason: Expired";
$Sql->Query("DELETE FROM `sm_admins` WHERE `identity` = '".$Steam."'");
$Sql->Query("UPDATE trial SET `Expired` = 'True' WHERE `Steam` = '".$Steam."'");
}
else
{
echo "All ok, 0 deleted";
}
}
}
?>

Proper and consistent indentation would improve the readability a lot. Here is mine:

<?php
    $Sql = new mysqli("localhost", "root", "*******", "serveradmin");
    if ($Sql->connect_error) {
        die("Sorry, Could not connect (" . $Sql->connect_errno . ") " . $Sql->connect_error);
    }
    $Data = $Sql->query("SELECT * FROM trial");
    while ($q = $Data->fetch_assoc()) {
        $Start = $q['Start'];
        $Stop = $q['Stop'];
        $Steam = $q['Steam'];
        $Result = $Start - $Stop;
        $Time = time();
        if ($q['Expired'] == "True") {
            echo "All expired \n";
        } else {
            if ($Stop <= $Time) {
                echo "Deleted " . $Steam . " From trial Reason: Expired";
                $Sql->Query("DELETE FROM `sm_admins` WHERE `identity` = '" . $Steam . "'");
                $Sql->Query("UPDATE trial SET `Expired` = 'True' WHERE `Steam` = '" . $Steam . "'");
            } else {
                echo "All ok, 0 deleted";
            }
        }
    }
?>

These conditions are good candidates for guard clauses:

if ($q['Expired'] == "True") {
    echo "All expired \n";
} else {

and

if ($Stop <= $Time) {

Here is the modified version:

while ($q = $Data->fetch_assoc()) {
    $Start = $q['Start'];
    $Stop = $q['Stop'];
    $Steam = $q['Steam'];
    $Result = $Start - $Stop;
    $Time = time();
    if ($q['Expired'] == "True") {
        echo "All expired \n";
        continue;
    }
    if ($Stop > $Time) {
        echo "All ok, 0 deleted";
        continue;
    }
    echo "Deleted " . $Steam . " From trial Reason: Expired";
    $Sql->Query("DELETE FROM `sm_admins` WHERE `identity` = '" . $Steam . "'");
    $Sql->Query("UPDATE trial SET `Expired` = 'True' WHERE `Steam` = '" . $Steam . "'");
}

References: Replace Nested Conditional with Guard Clauses in Refactoring: Improving the Design of Existing Code; Flattening Arrow Code
Furthermore, if you are not interested in those records whose Expired flag is True and Stop attribute is bigger than the current time you could modify the SQL query to filter those records, for example:

$Data = $Sql->query("SELECT * FROM trial WHERE Expired != `True` AND Stop > " . $Time);

(If you are using string concatenation to create SQL commands be careful about SQL injections attacks. The value of $Time comes from the time() function so it can't contain any malicious data here.)
I'd consider using MySQL's datetime attributes for storing the date informations and using MySQL's date and time functions for the comparison. I'd improve the database structure and make the raw data readable. You might want to prepare for situations when the current time is different between the webserver and the database server.
$mysql->query has a return value. You might want to check whether it's FALSE and print a proper error message.
PHP functions are not case sensitive, but according to this answer you might want to modify ->Query(...) to ->query(...).
This line seems unnecessary, since the $Result variable is not used:

$Result = $Start - $Stop;

The delete and update statement uses the $Steam variable as the part of the query without escaping it. I think it could cause 2nd order SQL attacks.


Simple Python Logger Here is a simple Python logger I would like some general feedback on.
Features:

Dynamically creates log folder and files wherever user runs the program
Abillity to create multiple logs from a single program
Names logs from time stamp to the latest second preventing name conflicts

I understand there may be far better alternatives to this but I just wanted to tinker around with this concept a bit!
Thank you!
logger.py:
#!/usr/bin/python
# -*- coding: utf-8 -*-

import datetime
import time
import os
import sys

# get user's current directory

directory = os.path.dirname(os.path.realpath(__file__)) + '/logs'

def createNewLog():

    # ensure log folder exists

    if not os.path.exists(directory):
        os.mkdir(directory)

    # get timestamp for naming the log

    timeInSeconds = time.time()
    timestamp = \
        datetime.datetime.fromtimestamp(timeInSeconds).strftime('%Y-%m-%d %H:%M:%S'
            )

    logFile = directory + '/' + timestamp + '.txt'
    mostRecentLog = directory + '/mostRecentLog.txt'

    # create log file and file with the log file's name

    with open(logFile, 'w') as log:
        with open(mostRecentLog, 'w') as recentLog:
            recentLog.write(timestamp)

def findMostRecentLog():

    # ensure logger has been intiated

    try:
        with open(directory + '/mostRecentLog.txt', 'r') as logFile:
            logName = logFile.read()
    except FileNotFoundError:

        print("Must initiate logger first!")
        sys.exit(1)

    return directory + '/' + logName + '.txt'

def log(comment):
    comment = str(comment)

    # write to log file retriving most recent log from corresponding file

    with open(findMostRecentLog(), 'a') as log:
        log.write(comment + '\n')

In case you do not know it yet, I suggest you to use pylint to parse your code and have automatic feedbacks. In fact some of my notes come from pylint.

Styling notes:

Constant names like var "directory" require uppercase style PEP8
Function and variable names require snake_case style PEP8
Functions require docstring, as well as the module itself PEP8

Inline notes

#NOTE: 'from datetime import datetime' allows a shorter line below
import datetime
import time
import os
import sys

# get user's current directory

# NOTE: use os.path.join for concat filesystem paths for cross compatibility (also below)
directory = os.path.dirname(os.path.realpath(__file__)) + '/logs'

def createNewLog():

    # ensure log folder exists

    if not os.path.exists(directory):
        # NOTE: what if user has no permission to create files/directory in current directory? (also below)
        # HINT: give a way to choose where to store logs
        os.mkdir(directory)

    # get timestamp for naming the log

    timeInSeconds = time.time()
    # NOTE: fromtimestamp may raise an exception which is not not managed here
    timestamp = \
        datetime.datetime.fromtimestamp(timeInSeconds).strftime('%Y-%m-%d %H:%M:%S'
            )

    logFile = directory + '/' + timestamp + '.txt'
    mostRecentLog = directory + '/mostRecentLog.txt'

    # create log file and file with the log file's name

    # NOTE: redefining name 'log' (it's also a function name)
    # NOTE: this 'log' is not used. What it's that for?
    with open(logFile, 'w') as log:
        with open(mostRecentLog, 'w') as recentLog:
            recentLog.write(timestamp)

def findMostRecentLog():

    # ensure logger has been intiated

    try:
        with open(directory + '/mostRecentLog.txt', 'r') as logFile:
            logName = logFile.read()
    except FileNotFoundError:
        print("Must initiate logger first!")
        # NOTE: kind of rugh, I suggest returning an error and managing at caller level
        sys.exit(1)

    return directory + '/' + logName + '.txt'

def log(comment):
    # NOTE: why forcing the type?
    comment = str(comment)

    # write to log file retriving most recent log from corresponding file

    # NOTE: this dependency will make hard testing this function alone.
    with open(findMostRecentLog(), 'a') as log:
        log.write(comment + '\n')


Mapping Excel rows to model I have an Excel file and each row within the sheet represents an employee's data.

I am trying to find a cleaner way to map each row to a model object:

public IList<Employee> ReadEmployees(byte[] file, string companyId)
{
    var employees = new List<Employee>();
    using (var stream = new MemoryStream(file))
    {
        using (var package = new ExcelPackage(stream))
        {
            var worksheet = package.Workbook.Worksheets[0];
            for (var rowNumber = 3; rowNumber <= worksheet.Dimension.End.Row; rowNumber++)
            {
                //---- Trying to retrieve the row data in an elegant way----
                var employee = GetEmployeeRowSummary(worksheet, rowNumber, companyId);
                employees.Add(employee);
            }
        }
    }

    return employees;
}

I was kind of hoping to use something like auto-mapper but don't know if that would be the best way (maybe create a Custom Resolver).

Is there a better way of mapping excel rows to an model? The way I am doing it currently seems like a maintenance issue.

private static Employee GetEmployeeRowSummary(ExcelWorksheet worksheet, int rowNumber, string companyId)
{
    var employeeId = Guid.NewGuid().ToString();

    return new Employee
           {
               CompanyId = companyId,
               EmployeeId = employeeId,
               EmployeeNumber = worksheet.Cells[rowNumber, 1].GetValue<string>(),
               FirstName = worksheet.Cells[rowNumber, 2].GetValue<string>(),
               LastName = worksheet.Cells[rowNumber, 3].GetValue<string>(),
               BirthDate = worksheet.Cells[rowNumber, 4].GetValue<DateTime>(),
               AppointmentDate = worksheet.Cells[rowNumber, 5].GetValue<DateTime>(),
               CompanyRunFrequencyId = 9,
               JobTitle = worksheet.Cells[rowNumber, 8].GetValue<string>(),
               DateCreated = DateTime.Now,
               EmployeeAddress = new List<EmployeeAddress>
                                 {
                                    new EmployeeAddress
                                    {
                                        EmployeeId = employeeId,
                                        StreetNumber = worksheet.Cells[rowNumber, 9].GetValue<int>(),
                                        Suburb = worksheet.Cells[rowNumber, 11].GetValue<string>(),
                                        City = worksheet.Cells[rowNumber, 12].GetValue<string>(),
                                        PostalCode = worksheet.Cells[rowNumber, 13].GetValue<int>()
                                    }
                                 }
           };
}

The indentation needs to be mentioned. Hopefully it's just an accidental tabs vs spaces issue that caused it to render this way, ...but I'm not seeing the Tab characters, so... the indentation needs to be mentioned, becauase this is not normal:

public IList<Employee> ReadEmployees(byte[] file, string companyId)
        {

Expected:

public IList<Employee> ReadEmployees(byte[] file, string companyId)
{

Just like you have here:

            using (var stream = new MemoryStream(file))
            {

And here:

                    for (var rowNumber = 3; rowNumber <= worksheet.Dimension.End.Row; rowNumber++)
                    {

Now, with that cleared, the GetEmployeeRowSummary method looks much cleaner already:

private static Employee GetEmployeeRowSummary(ExcelWorksheet worksheet, int rowNumber, string companyId)

{
    var employeeId = Guid.NewGuid().ToString();
    return new Employee
    {
        CompanyId = companyId,
        EmployeeId = employeeId,
        EmployeeNumber = worksheet.Cells[rowNumber, 1].GetValue<string>(),
        FirstName = worksheet.Cells[rowNumber, 2].GetValue<string>(),
        LastName = worksheet.Cells[rowNumber, 3].GetValue<string>(),
        BirthDate = worksheet.Cells[rowNumber, 4].GetValue<DateTime>(),
        AppointmentDate = worksheet.Cells[rowNumber, 5].GetValue<DateTime>(),
        CompanyRunFrequencyId = 9,
        JobTitle = worksheet.Cells[rowNumber, 8].GetValue<string>(),
        DateCreated = DateTime.Now,
        EmployeeAddress = new List<EmployeeAddress>
        {
            new EmployeeAddress
            {
                EmployeeId = employeeId,
                StreetNumber = worksheet.Cells[rowNumber, 9].GetValue<int>(),
                Suburb = worksheet.Cells[rowNumber, 11].GetValue<string>(),
                City = worksheet.Cells[rowNumber, 12].GetValue<string>(),
                PostalCode = worksheet.Cells[rowNumber, 13].GetValue<int>()
            }
       }
   };
}

Consistent indentation is fundamental for clean, readable code.

But readability isn't the biggest problem here: the repeated calls to worksheet.Cells and {Range?}.GetValue are.

I'm not sure what ExcelPackage is, but it's definitely not VSTO, so maybe this isn't as much of a problem, but invoking worksheet.Cells[...].GetValue<...>() is spawning a RCW (Runtime Callable Wrapper - a .NET wrapper object to access a COM object) whose reference isn't captured, and thus could be leaking the object reference. If you were using VSTO, there would very likely be a "ghost" EXCEL.EXE process lingering in Task Manager forever, well after your application happily terminated.

In any case, working directly with worksheet cells is THE slowest thing you can do, whether in VBA or C# through COM interop, whether it be through VSTO or any other library.

Again I don't know what ExcelPackage is nor what this API has to offer, but in the native Excel object model Range.Value, when the Range object represents multiple cells, gives you a 2D variant array (object[,] in .NET) - if all you're ever interested in is the values that are in the cells (as seems to be the case), then you don't need to deal with worksheets and cells and .GetValue<T>().

So instead of looping through rows:

for (var rowNumber = 3; rowNumber <= worksheet.Dimension.End.Row; rowNumber++)

Grab the values array (not sure what the syntax would be with this API - if your library doesn't allow you to grab a 2D array of values, drop that library and use VSTO):

var values = worksheet.Range(worksheet.Cells[3, 1], worksheet.Cells[lastRow, 13]).Value;

The idea is to get a Range object that encompasses all the cells you're interested in. Maybe that's everything from row 3 to worksheet.Dimension.End.Row, but even without knowing the slightest thing about this particular API I would fear that worksheet.Dimension.End.Row yields something like 1,048,576 - which is very very very unlikely the actual number of rows you need to care about.

With the Excel object model, you'd do something like sheet.Range("A" & sheet.Rows.Count).End(xlUp).Row to get the last interesting row (aka lastRow), and use that row number to make the "interesting range" from which to read the values.

Now that you have a 2D array that contains all the values, GetEmployeeRowSummary doesn't make much sense anymore: we don't care for a rowNumber, and we don't need a ExcelWorksheet reference anymore.

In fact all we need is a "row slice" of the 2D array, by copying the row to a new array (this code looks like it):

public T[] GetRow(T[,] matrix, int rowNumber)
{
    return Enumerable.Range(0, matrix.GetLength(1))
            .Select(x => matrix[rowNumber, x])
            .ToArray();
}

private static Employee ReadEmployee(object[] row, string companyId)
{
    var id = Guid.NewGuid().ToString();
    return new Employee
    {
        CompanyId = companyId,
        EmployeeId = id,
        EmployeeNumber = (string)row[1],
        FirstName = (string)row[2],
        LastName = (string)row[3],
        BirthDate = (DateTime)row[4],
        //...
    };
}

As for mapping the properties automatically, I'm not a fan. Something, somewhere needs to do the mapping, and I'd much rather it be explicit in the code. Automatic mapping would need more metadata than just column indices - you'd likely need column names and data types, so instead of copying the cells to an array you'd be copying the cells to some DataTable, or have the mapper understand what an Excel Range or ListObject is... all of which sound like quite a lot of effort for an error-prone way to just list the properties at the left of an = operator. If your explicit casts blow up, you'll know why. If the automagic mapping blows up, pray the library throws meaningful exceptions.


Timus Online Judge Problem 1001 - "Reverse Root" The problem statement can be found here.

Description (short):

Input

The input stream contains a set of integer numbers \$A_i\$ (\$0 \le A_i \le 10^{18}\$). The numbers are separated by any number of spaces and line breaks. A size of the input stream does not exceed 256 KB.

Output

For each number \$A_i\$ from the last one until the first one, you should output its square root. Each square root should be printed in a separate line with at least four digits after the decimal point.

#include <iostream>
#include <iomanip>
#include <math.h>
using namespace std;

int main()
{
    double t;
    double v [128 * 1024];
    int idx = 0;
    while (cin >> t) {
        v[idx] = sqrt(t);
        ++idx;
    }

    cout << fixed;
    cout << setprecision(4);

    for (int i = idx - 1; i >= 0; --i)
        cout << v[i] << endl;

    return 0;
}

I am encountering a TLE on test case 9, with an execution time of 2.031s and memory usage of 1 474 KB.  (The allowable limits are 2.0s and 64 MB.)  How can I improve (or change) my method to complete the problem successfully?

The input stream contains a set of integer numbers \$A_i\$ (\$0 \leq A_i \leq 10^{18}\$).

So you should change

double t;

to

unsigned long long t;

This actually speeds up the program a lot.

To speed up the output, instead of

cout << fixed;
cout << setprecision(4);

for (int i = idx - 1; i >= 0; --i)
    cout << v[i] << endl;

Try

for (int i = idx - 1; i >= 0; --i) {
    printf("%.4f\n", v[i]);
}


x / 2 + 100 * (a + b) - 3 / (c + d) + e * e in assembly write an algorithm for: x / 2 + 100 * (a + b) - 3 / (c + d) + e * e
knowing that: a, c - word, b, d - byte, e - doubleword, x - qword
    mov eax, dword [x]
    mov edx, dword [x + 4] ; edx:eax = x
    mov ebx, 2
    idiv ebx ; eax = edx:eax / ebx = x / 2
    mov ebx, eax ; save the result in ebx so we can do the other operations
    mov al, [b]
    cbw ; ax = b
    add ax, [a] ; ax = a + b
    mov dx, 100
    imul dx ; dx:ax = ax * dx = 100 * (a + b)
    push dx
    push ax
    pop eax ; 100 * (a + b)
    add ebx, eax ; ebx = x / 2 + 100 * (a + b)
    mov al, [d] ; al = d
    cbw ; ax = d
    add ax, word [c] ; ax = c + d
    mov cx, ax ; cx = c + d
    mov ax, 3
    cwd
    idiv cx ; ax = dx:ax / cx
    cwd
    push dx
    push ax
    pop eax ; eax = 3 / (c + d)
    sub ebx, eax
    mov eax, ebx
    cdq ; edx:eax = x / 2 + 100 * (a + b) - 3 / (c + d)
    mov ebx, eax
    mov ecx, edx ; ecx:edx = x / 2 + 100 * (a + b) - 3 / (c + d)
    mov eax, [e]
    imul dword [e] ; edx:eax = e * e
    add eax, ebx
    adc edx, ecx
    mov dword [result + 0], eax
    mov dword [result + 4], edx

did I make it unnecessarily complicated?

write an algorithm for: x / 2 + 100 * (a + b) - 3 / (c + d) + e * e

a, c - word,
b, d - byte,
e - doubleword,
x - qword

Because your biggest number is 64 bits (x is a qword), your final result will have to be 64 bits too!
Your first operation was to divide the qword in x by 2. You seem to expect that this result will fit in just a single dword because you've moved the quotient in the EBX register. You cannot make this assumption and worse the division could easily produce a divide exception if the quotient doesn't fit in 32 bits.
For the solution you should be aware that dividing by 2 is actually simply a shift to the right.
mov   ebx, [x]
mov   ebp, [x + 4] ; EBP:EBX is x
sar   ebp, 1
rcr   ebx, 1       ; EBP:EBX is x / 2

This means that you'll have to scale up the other calculations too in order to add them to EBP:EBX using:
add   ebx, ...
adc   ebp, ...

Because addition is associative, you can start by calculating the e * e part. You did not rearrange the expression and had to move around some more the registers in the end. Not a big deal, but nicer my way:
mov   eax, [e]
imul  eax
add   ebx, eax
adc   ebp, edx

Then comes 100 * (a + b):
movsx eax, word [a]
movsx edx, byte [b]
add   eax, edx       ; eax = a + b
mov   edx, 100
imul  edx            ; edx:eax = 100 * (a + b)
add   ebx, eax
adc   ebp, edx

I'll leave 3 / (c + d) to you...
... and finally the end will be:
sub   ebx, eax
sbb   ebp, edx
mov   [result + 0], ebx
mov   [result + 4], ebp

did I make it unnecessarily complicated?

It was a bit hard to read your program because you didn't insert some blank lines between the different operations.

You don't need to write a size tag (byte, word, dword) if the register involved already implies the size. In mov dword [result + 0], eax the dword tag is redundant.

Best have the comments in the program aligned above each other.

Reread carefully to avoid typos like in:
  mov ecx, edx ; ecx:edx = x / 2 + 100 * (a + b) - 3 / (c + d)

Should be ECX:EBX.

To compute a square: once you've loaded the number in the register, you can multiply by that same register and not turn to the memory a second time like you did:
  mov   eax, [e]
  imul  eax        ; Don't write "imul dword [e]"


LINQ generating chart I'm generating a table to use on jQuery visualize plugin and generate a chart. The code is pretty good, but maybe there is a way to improve it. Maybe do more work on the LINQ query and less stuff on the rest of the code.

The idea is get every content and the launch date of them for each region. Separate all dates and the the amount of content launched that day. But maybe there is a better way. Anyone?

Here is the method i use:

    public KeyValuePair<DateTime[], List<KeyValuePair<string, int[]>>> TimeGrowth(int days, params string[] locales)
    {
        DateTime startdate = DateTime.Today.AddDays(days * (-1));

        var qry = from r in GlobalVariables.Regions
                  where locales.Contains(r.ID)
                  select new
                  {
                      Region = r,
                      Launches = (from d in r.RegionalContents
                                 where d.RegionId == r.ID && d.PublishDate != null
                                 group d by d.PublishDate into groupeddates
                                 select new
                                 {
                                     Date = groupeddates.Key,
                                     Count = groupeddates.Count()
                                 }).OrderBy(x => x.Date)
                  };

        var regions = qry.ToArray();

        var dates = new List<DateTime>();
        var datesbeforestart = new List<DateTime>();

        var countries = new List<KeyValuePair<string, int[]>>();

        for (int i = 0; i < regions.Length; i++)
        {
            for (int j = 0; j < regions[i].Launches.Count(); j++)
            {
                var data = regions[i].Launches.ElementAt(j).Date.Value;
                if (data > startdate)
                    dates.Add(data);
                else
                    datesbeforestart.Add(data);
            }
        }

        dates = dates.Distinct().OrderBy(x => x.Date).ToList();

        for (int i = 0; i < regions.Length; i++)
        {
            var values = new int[dates.Count];
            values[0] = CountDateInterval(datesbeforestart.Min(), startdate, regions[i].Region.ID);

            int acum = values[0];
            for (int j = 1; j < dates.Count; j++)
            {
                int valor = 0;

                if (regions[i].Launches.Any(x => x.Date == dates[j]))
                    valor = regions[i].Launches.Single(x => x.Date == dates[j]).Count;

                acum += valor;

                values[j] = acum;
            }

            var country = new KeyValuePair<string, int[]>(regions[i].Region.CountryEnglish, values);

            countries.Add(country);
        }

        countries = countries.OrderByDescending(x => x.Value.Max()).ToList();

        var finalresult = new KeyValuePair<DateTime[], List<KeyValuePair<string, int[]>>>(dates.ToArray(), countries);

        return finalresult;
    }

You can see the actual running code here on the last chart: http://programad.net/xbltoolsv2beta/Stats

A few general points:

(1)  GlobalVariables.Regions -- I don't know how big your application is, but if I'm looking at a code base and I see some static class called "GlobalVariables" that usually makes me leery.  In a pure object-oriented language, all data can (and arguably ought to be) associated with some object.  A GlobalVariables class with a bunch of statics on it is the kind of thing that is likely to grow and rot with as the application grows (and rots).

(2)  As mentioned by Lars-Erik, I think that breaking some of that functionality out into other methods would be beneficial.  Personally, I look at control statements, especially nested ones for opportunities to do that.  Looking at your code, the logic inside for (int i = 0; i < regions.Length; i++) would be a good candidate.  Just by looking at the code casually, it's hard to tell what all is going on inside that loop.  But if you pulled it out into a method and gave it a descriptive name, someone looking at your code could easily understand what the loop was doing.

(3)  You have a lot of code (as Linq tends to) with sequences of statements like Foo.Bar().Baz().Rebar();  Generally speaking, if anything in those sequence chains returns null, you're going to get null reference exceptions.  Perhaps clients of this method handle exceptions that it generates, but I can't see that you're handling anything but the "happy path" in this method.  What happens if things go unexpectedly?


Google Foobar level 3 Over the past few weeks, I've been doing the Google Foobar challenges and I've been progressing quite well. I'm currently 1/3 of the way through the third level. However, there were plenty of times over the past few weeks where I felt that my code could have been a lot better. So today I stumbled upon this community and I think that it will be a good idea to ask you guys to review my code.

So this is what I was instructed to do:

  The fuel control mechanisms have three operations:

  Add one fuel pellet
  Remove one fuel pellet
  Divide the entire group of fuel pellets by 2 (due to the destructive energy released when a quantum antimatter pellet is cut in half, the safety controls will only allow this to happen if there is an even number of pellets)

  Write a function called answer(n) which takes a positive integer as a string and returns the minimum number of operations needed to transform the number of pellets to 1. The fuel intake control panel can only display a number up to 309 digits long, so there won't ever be more pellets than you can express in that many digits.

This is my solution:

    public static int answer(String n) {
        //convert n to BigInteger
        BigInteger x = new BigInteger(n);
        BigInteger two = new BigInteger("2");
        BigInteger three = new BigInteger("3");
        BigInteger y, z;
        int counter = 0;

        //Loop for as long as x is not equal to 1
        while(!x.equals(BigInteger.ONE)){
            //Check if x is divisible by 2
            if(x.mod(two).equals(BigInteger.ZERO)){
                //divide x by 2
                x = x.divide(two);
            } else {
                //subtract x by 1 and then divide by 2 store in variable z
                y = x.subtract(BigInteger.ONE);
                z = y.divide(two);
                //check if the result of that leaves a number that's divisible by 2, or check if x is equal to 3
                if(z.mod(two).equals(BigInteger.ZERO) || x.equals(three)){
                    x = y;
                } else {
                    x = x.add(BigInteger.ONE);
                }
        }
        counter++;
    }
    return counter;
}

For this challenge, I'm a lot more confident and happy with the cleanliness of my code compared to the previous challenges. But seeing that I'm still very much a newbie and this being my first time using BigInteger, I'd very much like a review.

You could have used the bitwise methods in BigInteger.

You could get rid of both .mod(two) and BigInteger two and instead use !__.testBit(0).

  Returns true if and only if the designated bit is set. (Computes ((this & (1<<n)) != 0).)

You could have also used .shiftRight(1) to divide by 2

  Returns a BigInteger whose value is (this >> n). Sign extension is performed. The shift distance, n, may be negative, in which case this method performs a left shift. (Computes floor(this / 2n).)

I haven't put a ton of thought, but I think you could also find a clever way to get rid of 3 with testBit and bitLength, but I think that .compareTo/equals method is alright.

public static int answer(String n) {
    //convert n to BigInteger
    BigInteger x = new BigInteger(n);
    BigInteger three = new BigInteger("3");
    BigInteger y, z;
    int counter = 0;

    //Loop for as long as x is not equal to 1
    while(!x.equals(BigInteger.ONE)){
        //Check if x is divisible by 2
        if(!x.testBit(0)){
            //divide x by 2
            x = x.shiftRight(1);
        } else {
            //subtract x by 1 and then divide by 2 store in variable z
            y = x.subtract(BigInteger.ONE);
            z = y.shiftRight(1);
            //check if the result of that leaves a number that's divisible by 2, or check if x is equal to 3
            if(!z.testBit(0) || x.equals(three)){
                x = y;
            } else {
                x = x.add(BigInteger.ONE);
            }
        }
        counter++;
    }
    return counter;
}


Is there a more optimal way to approach some of these bitwise functions? I completed some bit manipulation exercises out of a textbook recently and have grasped onto some of the core ideas behind manipulating bits firmly. My main concern with making this post is for optimizations to my current code. I get the hunch that there are some functions that I could approach better. Do you have any recommendations for the following code?

#include <stdio.h>
#include "funcs.h"

// basically sizeof(int) using bit manipulation
unsigned int int_size(){
    int size = 0;
    for(unsigned int i = ~00u; i > 0; i >>= 1, size++);
    return size;
}

// get a bit at a specific nth index
// index starts with 0 on the most significant bit
unsigned int bit_get(unsigned int data, unsigned int n){
    return (data >> (int_size() - n - 1)) & 1;
}

// set a bit at a specific nth index
// index starts with 0 on the most significant bit
unsigned int bit_set(unsigned int data, unsigned int n){
    return data | (1 << (int_size() - n - 1));
}

// gets the bit width of the data (<32)
unsigned int bit_width(unsigned int data){
    int width = int_size();
    for(; width > 0; width--)
        if((data & (1 << width)) != 0)
            break;
    return width + 1;
}

// print the data contained in an unsigned int
void print_data(unsigned int data){
    printf("%016X = ",data);
    for(int i = 0; i < int_size(); i++)
        printf("%X",bit_get(data,i));
    putchar('\n');
}

// search for pattern in source (where pattern is n wide)
unsigned int bitpat_search(unsigned int source, unsigned int pattern,
        unsigned int n){
    int right = int_size() - n;
    unsigned int mask = 0;
    for(int i = 0; i < n; i++)
        mask |= 1 << i;
    for(int i = 0; i < right; i++)
        if(((source & (mask << (right - i))) >> (right - i) ^ pattern) == 0)
            return i - bit_width(source);
    return -1;
}

// extract {count} bits from data starting at {start}
unsigned int bitpat_get(unsigned int data, int start, int count){
    if(start < 0 || count < 0 || int_size() <= start || int_size() <= count || bit_width(data) != count)
        return -1;
    unsigned int mask = 1;
    for(int i = 0; i < count; i++)
        mask |= 1 << i;
    mask <<= int_size() - start - count;
    return (data & mask) >> (int_size() - start - count);
}

// set {count} bits (basically width of {replace}) in {*data} starting at {start}
void bitpat_set(unsigned int *data, unsigned int replace, int start, int count){
    if(start < 0 || count < 0 || int_size() <= start || int_size() <= count || bit_width(replace) != count)
        return;
    unsigned int mask = 1;
    for(int i = 0; i < count; i++)
        mask |= 1 << i;
    *data = ((*data | (mask << (int_size() - start - count))) & ~(mask << (int_size() - start - count))) | (replace << (int_size() - start - count));
}

The comment \\ basically sizeof(int) using bit manipulation
is wrong. sizeof(int) gives the number of bytes (technically, "storage units"), but your function gives the number of bits.

If you change the name of the function to something descriptive like bits_in_int(), you might not need the comment.

The C language says that a char must always be a storage unit.  If you know how many bits are in a char, you can just multiply by sizeof(int) to get the number of bits in an int.  And there's a constant, CHAR_BIT, in header <limits.h> which is the number of bits in a character.  So your whole int_size() function could be reduced to the compile-time expression sizeof(int) * CHAR_BIT;

If you really want to figure it out yourself, instead of using language-defined tools, your function could still use better style.  First, a for loop shouldn't try to put everything in the loop header.  The header should concern itself with the iteration.  Everything else should either go before the loop, or in the loop body.  I would replace your loop with:

for (unsigned int i = ~0u; i > 0; i >>= 1)
{
    size++;
}

size has nothing to do with controlling the loop, so I moved it to the body. I changed ~00u to ~0u.  I see no reason to use an octal zero (00) instead of a decimal zero (0).

(Incidentally, I suggest never writing a single-statement loop or if statement without the braces.  The typing you save doesn't make up for the increased risk of introducing an error in the future.)

This loop:

for(int i = 0; i < int_size(); i++)
     printf("%X",bit_get(data,i));

is inefficient.  You're computing int_size() each time through the loop.  This is better:

const unsigned int size = int_size();
for(int i = 0; i < size; i++)
{
    printf("%X",bit_get(data,i));
}

(You're calling int_size() more than once in other functions; I won't keep mentioning it.)

Get in the habit of declaring things const whenever possible.  It makes the code clearer to the reader.  It sometimes helps the compiler do a better job optimizing. And it makes you more aware of stuff that you can move out of loop bodies.

If a function returns an unsigned int, it probably doesn't make sense to return -1 to mean "invalid input" (function bitpat_get) or "not found" (function bitpat_search).  -1 converts to an unsigned int filled with 1 bits (at least on a two's complement machine).  This is probably a valid result for some input, so the calling code can't reliably extract the meaning from the return value.

Does bitpat_search() behave reasonably if the caller lies?  What if the value passed in n is wrong for the given pattern?  If the caller is supposed to do n = bit_width(source), why not do the calculation inside bitpat_search() instead of making the caller pass n?


Finding the upper and lower bound using binary search I am trying to solve this InterviewBit question
https://www.interviewbit.com/problems/search-for-a-range/.

My code for this seems to be working properly and it does give me the correct answers on my IDE and on all the sample test cases online but it gives me a TLE on the online judge. It runs in O(log n) time, just a variation of simple binary search.

I am trying to understand what aspect of my code could be made more efficient and faster.

Here's my code:

int findBound(vector<int> a, int b, bool lower){
int low=0, high=a.size()-1, mid;
while(low<=high){
    mid=low+(high-low)/2;
    if(a[mid]==b){
        if(lower){
            if((mid != (a.size()-1)) && a[mid+1]==b)
                low=mid+1;
            else
                return mid;
        }
        else{
            if(mid != 0 && a[mid-1]==b)
                high=mid-1;
            else
                return mid;
        }
    }
    else if(a[mid]>b)
        high=mid-1;
    else if(a[mid]<b)
        low=mid+1;
}
return -1;
}

vector<int> Solution::searchRange(const vector<int> &A, int B) {
    vector<int> ans(2);
    ans[0]=findBound(A, B, true);
    ans[1]=findBound(A, B, false);
    return ans;
}

I am trying to understand what aspect of my code could be made more efficient and faster.

First, the low hanging fruits...

You unnecessarily copy the sequence of integers when passing it to findBound:

int findBound(vector<int> a, int b, bool lower)

should be

int findBound(const vector<int>& a, int b, bool lower)

You dispatch on the bool lower flag in every iteration of the main while-loop:

if(lower) {
   /* ... */
} else {
   /* ... */
}

Consider implementing two separate functions for the starting and one for the end index of the range.
In one of the if-conditions in the middle of the main loop, you compute a.size() - 1. This could be done once at the top of the function and bound to a const-qualified variable. No need to evaluate this in every iteration.

Now, the crucial step...

You are doing unnecessary work when comparing values. The very first if-condition,

if(a[mid]==b) { // ...

tests for the branch with the least likelihood. Instead, check for a[mid] < b and nothing more. If you're wondering how this can be sufficient, check out this part of Sean Parent's Pacific C++ talk from 2018.


Promises and chained calls I've realized JS callback hell and I started looking into Promises libraries.
After reading some articles I decided to start with BluebirdJS. I didn't realise how to properly use them yet, because documentation written with API examples, not real usage.

I have two files (NodeJS environment):

#monitor.js
Promise = require("bluebird")
class Monitor
  constructor: (@api) ->

  getServerInfo: ->
    return new Promise((resolve, reject) =>
      @api.query("messages.getLongPollServer", { use_ssl: 1}).then(
        (result) =>
          @key = result.response.key
          @server = result.response.server
          @ts = result.response.ts

          resolve(@)

        reject
      )
    )

  printDetails: ->
    console.log(@key, @server, @ts)

module.exports = Monitor

#app.js
Api = require("./api")
Monitor = require("./monitor")

api = new Api("6de61129da290c3edf9c699ad25638156266e7b6bdbcc00407efb630e5c62f2d4c71974c7bdba52a03dbd")

monitor = new Monitor(api)

monitor.getServerInfo().then((monitor) -> monitor.printDetails())

What I wanted to achieve is something like this:

monitor.getServerInfo().then().printDetails()

because getServerInfo runs asynchronously.

Am I using promises correctly?

Don't use explicit returns in CoffeeScript unless you have to (and you don't here)
CoffeeScript may do automatic comma insertion, but I certainly didn't right away (i.e. add a comma yourself, right before reject - anything to make it easier to read)

Anyway: You can't get this code

monitor.getServerInfo().then().printDetails()

because that would be synchronous, not asynchronous. The interpreter would have to wait in the middle of the chain before it could call printDetails. And it doesn't wait - that's the point of asynchronicity.

And that's why then() always returns a promise object (and nothing else), because it can return that right away without any waiting.

So your current code

monitor.getServerInfo().then((monitor) -> monitor.printDetails())

is the way to do it. Of course, it doesn't make much sense to have a printDetails method in your Monitor, if it doesn't always work. Right now, you can just call

(new Monitor(api)).printDetails()

and get

undefined, undefined, undefined

printed. Or, if you just happened to call it after the API query has run, printDetails will suddenly print actual values. The method is almost random at that point because there's nothing guaranteeing that things happen in the right order.

I'd just skip the Monitor class entirely, since it's not really contributing much - at least not right now. But, given the name "monitor", I imagine the point is to get the server info, so you can then set up a polling system of some kind.

In that case, you could make a factory method that returns its product via a promise:

Promise = require "bluebird"

class Monitor
  constructor: (@server, @key, @ts) ->
    # I don't know if a Monitor instance needs an api object
    # for anything. If it does, add it as an argument to
    # this constructor

  startPolling: ->
    # ... or something

  stopPolling: ->
    # ... or something

  printDetails: ->
    console.log @server, @key, @ts

# factory function
Monitor.create = (api) ->
  # internal function that actually runs the query
  createMonitor = (resolveCallback, rejectCallback) ->
    success = (data) ->
      {server, key, ts} = data.response # CoffeeScript destructuring
      resolveCallback new Monitor(server, key, ts) # also pass in the api obj, if needed

    api.query("messages.getLongPollServer", use_ssl: 1).then success, rejectCallback

  # return a promise that'll resolve with
  # a new Monitor instance (or fail,
  # presumably with an error of some sort)
  new Promise createMonitor

module.exports = Monitor

If you already have the server, key and ts values from somewhere, then great; just call new Monitor(server, key, ts) and you're done.

But if you don't, you can do this:

Api     = require("./api")
Monitor = require("./monitor")

api = new Api("6de61....")

success = (monitor) -> monitor.printDetails() # or whatever else you need
failure = (args...) -> console.warn args...

Monitor.create(api).then success, failure

This is basically turning the process inside out: Instead of instantiating a Monitor object, and making it fetch the data it needs, the factory method is doing the fetching, and only creating the instance when it's time to do so.


Downloading data using HttpClient I'm working on a class to download some data from an external server and feed it into my data model.  I'm new to both HttpClient and async methods in C#, so I'm not sure of a few things.

Is my placement of new SearchResultSet() reasonable?  It's a little contrived, true, but I'm trying to get an example of how async code can streamline execution.
Is splitting the GetStringAsync() and await myTask statements going to cause any problems?

The HttpClient class is being passed in via the constructor to aid in unit testing.

class MySearch
{
    private readonly HttpClient searchClient;
    private readonly Uri baseSearchUrl;

    public MySearch(HttpClient client, Uri baseUrl) {
        searchClient = client;
        baseSearchUrl = baseUrl;
    }

    public async Task<SearchResultSet> SearchAsync(string term)
    {
        var query = "?gws_rd=ssl#q=" + term;
        var searchUrl = new Uri(baseSearchUrl, query);

        try {
            var myTask = searchClient.GetStringAsync(searchUrl);
            var result = new SearchResultSet()
            {
                SearchPageUrl = searchUrl
            };

            var searchData = await myTask;

            result.Fill(searchData); //parsing HTML is off-topic for this class!

            return result;
        }
        catch (AggregateException ex)
        {
            throw ex;
        }
        catch (WebException ex)
        {
            throw ex;
        }
        catch (Exception ex)
        {
            throw ex;
        }
    }
}

You are making your class variables readonly which is good because they won't change.

what I don't like is your mixing of the bracing style. Sometimes you use the Allman style, sometimes the K&R and sometimes another style.
Usually in C# one would expect to see Allman style only. If you want to use another style, that is pretty fine as long as you stick to it.

the exception handling like you are doing it is superfluous and is destroying the stacktrace. Please read: is-there-a-difference-between-throw-and-throw-ex
Because you are never handling the exceptions you could just remove the try..catch completely.


Building a madlib generator I'm learning Python and I'm building a basic madlib generator. The code below works as expected. I'm just wondering if it uses best practice / can be made better.
def intro():
    print("Welcome to our madlib generator!")
    print("We'll ask you for a serious of words, you'll give us the words, and "
          "we'll make a dope ass story out of it!")
    print("Let's get some info from you first.")
    print("Select the type of madlib you want. ")
    madlib_type = input("1. Short Story\n2. Poem\n")
    return madlib_type

def user_input():
    name = input("Enter a name: ")
    color = input("Enter a color: ")
    animal = input("Enter an animal: ")
    food = input("Enter a food: ")
    item = input("Enter an item: ")
    adjective = input("Enter an adjective: ")
    noun1 = input("Enter a plural noun: ")
    noun2 = input("Enter any noun: ")
    return [name, color, animal, food, item, adjective, noun1, noun2]

class MadlibGenerator:
    def __init__(self, *args):
        self.name = args[0].title()
        self.color = args[1]
        self.animal = args[2]
        self.food = args[3]
        self.item = args[4]
        self.adjective = args[5]
        self.noun1 = args[6]
        self.noun2 = args[7]

    def short_story_generator(self):
        print(f"There once was a person named {self.name}. {self.name} LOVED "
              f"{self.animal}s. They were {self.name}'s favorite animal.")
        print(f"One day, {self.name} was walking down the street wearing "
              f"{self.color} pants and a {self.color} shirt. In fact, all of "
              f"{self.name}'s ")
        print(f"clothes were {self.color}! In one of {self.name}'s hand, "
              f"there was a bright green {self.item}. In the other, there was "
              f"a big {self.food} that {self.name} was waiting to eat!")

    def poem_generator(self):
        print(f"Roses are {self.adjective}")
        print(f"{self.noun1} are blue")
        print(f"I can't start my day without {self.noun2}")

madlib_type = intro()
madlib_input = user_input()
madlib = MadlibGenerator(madlib_input[0], madlib_input[1], madlib_input[2],
                         madlib_input[3], madlib_input[4], madlib_input[5],
                         madlib_input[6], madlib_input[7])
if madlib_type == "1":
    madlib.short_story_generator()
if madlib_type == "2":
    madlib.poem_generator()

You're asking for a lot of words you don't use. It might be nicer to only ask the user for relevant words. Maybe instead of user_input taking no arguments, it could take a list of descriptions like user_input(["a name", "a color", "a different color"]) and ask based on those?
I don't see why the two story generators live in the same class like that. Passing the same data to multiple generators is probably not very useful very often. The functions can just as easily take the words as arguments themselves instead. This doesn't make the idea of a MadlibsGenerator class inappropriate, but the two functions belong to different generators, and the functions' input doesn't need to be directly tied to a generator at all.
The generators also do two different things each - they both generate the stories and print them. Usually, it would be better to do those separately, in case you maybe want to do something else with the stories as well.
You don't check if the user provides valid input when selecting which generator to use. If I'm asked to pick 1 or 2 and enter 3, I'd probably expect an error message and maybe an opportunity to try again. Instead, the program just keeps going as usual, except I don't get a fun story at the end.
Some day, someone else might want to use these functions (or maybe you'll want to use them for something else). They'll probably try importing your file as a module. When they do that, the entire file is run, which right now means the entire madlibs program. That's probably not what they wanted. You can prevent that by placing the program logic inside an if __name__ == "__main__": block, so that it doesn't execute when the file is imported.
Branching on a string to figure out which generator to use is a bit ugly. Just being able to look it up in a dictionary would be nice, and might also make it easier to add new ones, since we only need to update one place rather than having to worry about keeping intro and the ifs in sync.
But if we don't want to update intro each time we come up with a new generator we still have a problem - it needs to know what to call all the different generators when telling us what our options are. We could put that in the dictionary I mentioned above, or if we're already putting stuff in our generator objects we can add a name field in there as well. The latter is probably tidier.
Taken all together, we might get something like
class MadlibsGenerator:
    def __init__(self, name, descriptions, generator):
        self.name = name
        self.descriptions = descriptions
        self.generator = generator

    def __call__(self, *args, **kwargs):
        return self.generator(*args, **kwargs)

def intro(generators):
    print("Welcome to our madlib generator!")
    print("We'll ask you for a serious of words, you'll give us the words, and "
          "we'll make a dope ass story out of it!")
    print("Let's get some info from you first.")
    print("Select the type of madlib you want. ")

    for key in generators:
        print(f"{key}. {generators[key].name}")

    while True:
        selection = input()
        if selection in generators:
            break
        else:
            print(f"No generator \"{selection}\", please try again.")

    return generators[selection]

def user_input(descriptions):
    words = []
    for description in descriptions:
        words.append(input(f"Enter {description}: "))

    return words

def generate_short_story(name, animal, color, item, food):
    return (f"There once was a person named {name}. {name} LOVED "
        f"{animal}s. They were {name}'s favorite animal.\n"
        f"One day, {name} was walking down the street wearing "
        f"{color} pants and a {color} shirt. In fact, all of "
        f"{name}'s\n"
        f"clothes were {color}! In one of {name}'s hand, "
        f"there was a bright green {item}. In the other, there was "
        f"a big {food} that {name} was waiting to eat!")

def generate_poem(adjective, noun1, noun2):
    return (f"Roses are {adjective}\n"
        f"{noun1} are blue\n"
        f"I can't start my day without {noun2}")

if __name__ == "__main__":
    generators = {
        "1": MadlibsGenerator("Short Story",
            ["a name", "an animal", "a color", "an item", "a food"],
            generate_short_story),
        "2": MadlibsGenerator("Poem",
            ["an adjective", "a plural noun", "any noun"],
            generate_poem)
    }

    generator = intro(generators)
    madlib_input = user_input(generator.descriptions)
    print(generator(*madlib_input))

Update
At this point, you might start to think: "Hang on, each of those generator functions just puts strings into another string - creating a function each time we want to do that seems clumsy". And you have a point. While f-strings are nice, there are other options available that let us specify the template in one place and later use it in another. For example, strings already have a format method. We could take advantage of that and replace the generator functions with strings like
poem_template = """
Roses are {0}
{1} are blue
I can't start my day without {2}
"""

And instead of having our generators call these functions, we can have them call .format on these strings like
class MadlibsGenerator:
    def __init__(self, name, descriptions, template):
        self.name = name
        self.descriptions = descriptions
        self.template = template.strip()

    def __call__(self, *args):
        return self.template.format(*args)

And now we can create generators like greeting_generator = MadlibsGenerator("Greeting", ["a name"], "Hi there, {0}!") if we want to. Neat!


Make a new design of Stack with a new function I am doing some coding exercise, and I am not sure if I have met the requirement. Can someone help me to figure it out?

How would you design a stack which, in addition to push and pop, also has a function min which returns the minimum element? Push, pop and min should all operate in O(1) time.

public class Question {
private Node top;
private Node min;

public void push(int data) {
    Node newNode = new Node(data);
    Node newMin = new Node(data);
    if (top == null) {
        this.top = newNode;
        this.min = newMin;
    } else {
        newNode.next = this.top;
        this.top = newNode;
        if (newMin.data < this.min.data) {
            newMin.next = this.min;
            this.min = newMin;
        }
    }
}

public int pop() {
    if (top == null) {
        return -1;
    }
    int tempInt = this.top.data;
    if (this.top.data == this.min.data) {
        this.min = this.min.next;
    }
    this.top = this.top.next;
    return tempInt;
}

public int min() {
    return this.min.data;
}
public String toString(){
    return "Current top is: " + this.top.data + ", current min is: " + this.min();
}

class Node {
    Node next;
    int data;

    public Node(int data) {
        this.data = data;
    }
}

public static void main(String[] args){
    Question tempQue = new Question();
    for(int i = 100; i > 0; i--){
        tempQue.push(i);
        System.out.println(tempQue);
    }
    for(int i = 0 ; i < 99; i++){
        tempQue.pop();
        System.out.println(tempQue);
    }
}
}

I have tested this code but I am not sure if the Operating time meets the requirement \$O(1)\$.

It's O(1) no loops or recursion that I can see.

You shouldn't have to keep 2 stacks though. You can instead a second minNext field in the Node. It should be null except for when it contains the minimum of all subsequent elements (or it would be the top if the min stack).

public void push(int data) {
    Node newNode = new Node(data);
    if (top == null) {
        this.top = newNode;
        this.min = newMin;
    } else {
        newNode.next = this.top;
        this.top = newNode;
        if (newMin.data < this.min.data) {
            newNode.nextMin = this.min;
            this.min = newNode;
        }
    }
}

public int pop() {
    if (top == null) {
        return -1;
    }
    int tempInt = this.top.data;
    if (this.top == this.min) { // check for equality of the nodes themselves
        this.min = this.top.nextMin;
    }
    this.top = this.top.next;
    return tempInt;
}


Date formatter for javascript I just coded this formatter to format timestamps in javascript (I tied it to underscore for convenience), any remark?

_.toDate = function(epoch, format, locale) {
    var date = new Date(epoch),
        format = format || 'dd/mm/YY',
        locale = locale || 'en'
        dow = {};

    dow.en = [
        'Sunday',
        'Monday',
        'Tuesday',
        'Wednesday',
        'Thursday',
        'Friday',
        'Saturday'
    ];

    var formatted = format
        .replace('D', dow[locale][date.getDay()])
        .replace('dd', ("0" + date.getDate()).slice(-2))
        .replace('mm', ("0" + (date.getMonth() + 1)).slice(-2))
        .replace('yyyy', date.getFullYear())
        .replace('yy', (''+date.getFullYear()).slice(-2))
        .replace('hh', date.getHours())
        .replace('mn', date.getMinutes());

    return formatted;
}

usage

_.toDate($.now(), "dd-mm-yy at hh:mn");
// Will output:
"27-03-13 at 17:20"

I would use a library that is designed for this, such as Moment.js.  It is a lot more flexible, and has internationalization support also.