Minimum number of letters removed, to make a string Peragram Here is the link to the problem Peragrams.

  Peragrams: If a word is an anagram of at least one palindrome, we call it a Peragram.

  Problem: Given a string, find the minimum number of letters you have to remove from it, so that the string becomes a Peragram.

    Input #1: abc

    Output #1: 2

    Input #2: abb

    Output #2: 0

My Python code is as follows:

# Checks whether a string is palindrome
def IsPalindrome(str):
    m = int(len(str) / 2)

    str1 = str[:m]
    str2 = str[m:][::-1]

    if(len(str) % 2 != 0):
        str2 = str[m + 1:][::-1]

    if(str1 != str2):
        return False

    return True

##################################################

str = input()               #Read input from console
toBeRemoved = 0

if(IsPalindrome(str)):      #If the string is already a palindrome
    print(0)
    exit(0)

str = ''.join(sorted(str))  #Sort the string

i = 0
isOdd = True

while i < len(str):
    if(i + 1 < len(str) and str[i] == str[i + 1]):          #If two consecutive chars map, skip matched char
        i += 2
        continue

    toBeRemoved += 1                                        #Increment the number of chars to remove

    if(len(str) % 2 != 0 and isOdd and toBeRemoved > 0):   #If the string is odd length, skip first non-duplicate character & skip to next char
        isOdd = False
        toBeRemoved -= 1
        i += 1
        continue

    str = str[:i] + str[i + 1:] #Remove the char at i

    if(IsPalindrome(str)):
        break

print(toBeRemoved)

How can I improve the code to get a better running time?

The approach you are taking is to remove letters until at most one of them has an odd count. Once you realise that, the solution becomes obvious:

from collections import Counter

s = input() # don't call your own variable 'str'

c = Counter(s)

odds = [None for n in c.values() if n % 2]

if odds:
    print(len(odds) - 1)
else:
    print(0)

collections.Counter is a dict subclass to make counting the letters simpler. You don't actually need to create the substrings or check whether they are peragrams.

You should read and consider following the style guide, PEP-0008. For example:

if(IsPalindrome(str)):

should be:

if is_palindrome(s):


Getting single element or throwing one of two exceptions Sometimes I need to get a single thing from a collection but I'm not very happy with the Single extension. It does not differentiate between empty and more than one element and throws the same InvalidOperationException in both cases. I find it's an important disadvantage and it helps a lot if you know excactly why and exception was thrown.

I want to replace it with my own extension that I currently name Single2 as throwing two different exceptions.

public static T Single2<T>(this IEnumerable<T> source)
{
    var count = 0;
    var single = default(T);
    using (var enumerator = source.GetEnumerator())
    {
        while (enumerator.MoveNext() && count++ < 1)
        {
            single = enumerator.Current;
        }
    }

    switch ((SearchResult)count)
    {
        case SearchResult.NotFound: throw new EmptySequenceException();
        case SearchResult.SingleMatch: return single;
        default: throw new MoreThanOneElementException();
    }
}

public enum SearchResult
{
    NotFound = 0,
    SingleMatch = 1,
    ManyMatches = 2,
}

The exceptions are very simple:

public class EmptySequenceException : Exception { }
public class MoreThanOneElementException : Exception { }

That's all. What do you think of this new helper? Is there still room for improvement?

Example

This is one of the methods that already uses the new Single2. It's from my ResourceReader and it searches for an embedded resource in an assembly.

public static string FindName<T>([NotNull] this IResourceReader resources, [NotNull] Expression<Func<string, bool>> predicateExpression)
{
    if (resources == null) throw new ArgumentNullException(nameof(resources));
    if (predicateExpression == null) throw new ArgumentNullException(nameof(predicateExpression));

    var predicate = predicateExpression.Compile();

    try
    {
        return
            resources
                .GetResourceNames(typeof(T).Assembly)
                .Where(predicate)
                .Single2();
    }
    catch (EmptySequenceException innerException)
    {
        throw DynamicException.Factory.CreateDynamicException(
            $"ResourceNotFound{nameof(Exception)}",
            $"Expression {predicateExpression.ToString().QuoteWith("'")} does not match any resource in the {typeof(T).Assembly.GetName().Name.QuoteWith("'")} assembly.",
            innerException);
    }
    catch (MoreThanOneElementException innerException)
    {
        throw DynamicException.Factory.CreateDynamicException(
            $"MoreThanOneResourceFound{nameof(Exception)}",
            $"Expression {predicateExpression.ToString().QuoteWith("'")} matches more than one resource in the {typeof(T).Assembly.GetName().Name.QuoteWith("'")} assembly.",
            innerException);
    }
}

I don't really like the second while condition count++ < 1 because IMO it is to hard to grasp at first glance when this condition really comes true.

I would do it without a loop but with simple if statements like so

public static T Single2<T>(this IEnumerable<T> source)
{
    using (var enumerator = source.GetEnumerator())
    {
        if (enumerator.MoveNext() == false)
        {
            throw new EmptySequenceException();
        }
        var single = enumerator.Current;
        if (enumerator.MoveNext())
        {
            throw new MoreThanOneElementException();
        }
        return single;
    }
}

Now you don't have a count anymore hence no switch either. I have not used the ! operator on purpose, because having two calls to MoveNext() in two different if condition one checking for false and the other for true harms the readability a little by using that operator.


Max Contiguous Subarray: Divide and Conquer I am conversant with Kadane's Algorithm. This is just an exercise in understanding divide and conquer as a technique.

  Find the maximum sum over all subarrays of a given array of positive/negative integers.

Here is what I have worked on but accumulating sum in solve_partition() looks pretty similar to solve_crossing_partition(), left and right sections. Am I duplicating computation?

I would also appreciate some guidance on the intuition behind moving from mid to low when calculating left_sum: for i in range(m, lo - 1, -1): ...

import math

def max_subarray_sum(A):
    def solve_crossing_partition(m, lo, hi):

        left_sum = -math.inf
        _sum = 0
        for i in range(m, lo - 1, -1):
            _sum += A[i]
            left_sum = max(left_sum, _sum)

        right_sum = -math.inf
        _sum = 0
        for j in range(m + 1, hi):
            _sum += A[j]
            right_sum = max(right_sum, _sum)

        return left_sum + right_sum

    def solve_partition(lo, hi):
        if lo == hi:
            return A[lo]

        max_sum = -math.inf
        _sum = 0

        for i in range(lo, hi):
            _sum += A[i]
            max_sum = max(max_sum, _sum)

        return max_sum

    if not A:
        return 0

    m = len(A) // 2
    L = solve_partition(0, m + 1)
    R = solve_partition(m + 1, len(A))
    X = solve_crossing_partition(m, 0, len(A))

    return max(max(L, R), X)

if __name__ == "__main__":
    for A in (
        [],
        [-2, 1, -3, 4, -1, 2, 1, -5, 4],
        [904, 40, 523, 12, -335, -385, -124, 481, -31],
    ):
        print(max_subarray_sum(A))

Output:

0
6
1479

I followed this ref.

Nested max
max(max(L, R), X)

can be
max((L, R, X))

Actual tests
Assert what you're expecting:
assert 0 == max_subarray_sum([])
assert 6 == max_subarray_sum([-2, 1, -3, 4, -1, 2, 1, -5, 4])
assert 1479 == max_subarray_sum([904, 40, 523, 12, -335, -385, -124, 481, -31])


Ruby getter recursively builds tree This is a getter in a class.  It recursively generates a tree/hash to a certain level of depth.

def children(depth: 4, article_children: self.root.child_links)
  get_children = lambda do |depth, article_children|
    article_children.map do |uri|
      if (depth == 0)
        Article.new(uri: uri)
      else
        article = Article.new(uri: uri)
        { article => get_children.call(depth - 1, article.child_links) }
      end
    end
  end

  @children ||= get_children.call(depth, self.root.child_links)
  return @children
end

I would like feedback on these specific areas:

Readability/maintainability concerns
Is this an appropriate use of lambda?
Style concerns
General feedback

Obviously people are going to have different opinions. I think that adding a lambda just makes it more difficult to read, I would put that in a private emethod (which also has the advantage of making it testable)

There is a bug in your code:

def children(depth: 4, article_children: self.root.child_links)
  @children ||= get_children.call(depth, self.root.child_links)
  return @children
end

You pass self.root_child_links to get_children. You should be passing article_children.

Some style notes:

def children(depth: 4, article_children: self.root.child_links)
  ...
  @children ||= get_children.call(depth, self.root.child_links)
  return @children
end

The return is generally considered bad style. It is enough to specify just the return value on the last line. i.e. just @children. In this case even that is unnecessary as the assignment above returns the value of @children. iow I would just write:

def children(depth: 4, article_children: self.root.child_links)
  ...
  @children ||= get_children.call(depth, self.root.child_links)
end

Also (simplifying this piece of code to show my point)

article_children.map do |uri|
  article = Article.new(uri: uri)
  { article => get_children.call(depth - 1, article.child_links) }
end

This returns an array of hashes which is an odd way to structure data, wouldn't it make sense to return a single hash.

Lastly instead of passing article_links into the method I would pass the article.

My take on this code would be something like:

def children(depth: 4, root: self.root)
  @children ||= recurse_children(depth, root)
end

private

def recurse_children(depth, article)
  article.child_links.reduce do |hash, uri|
    article = Article.new(uri: uri)
    hash[article] = depth > 0 ? recurse_children(article) : nil
    hash
  end
end


Middleware for a client I'm developing a middleware for my client and it's found that there are some delay when executing the process. We are currently investigating what's the exact causes of the issue, from architecture design to coding.

Here is part of the script we are calling, which is working, and I'm just wondering if it's optimized and properly written.

public string getResponse(System.Type type, Object input, string taskName, string method)
{
    string response = string.Empty;

    try
    {
        DataContractJsonSerializer ser = new DataContractJsonSerializer(type);
        MemoryStream mem = new MemoryStream();
        ser.WriteObject(mem, input);
        string data = Encoding.UTF8.GetString(mem.ToArray(), 0, (int)mem.Length);

        using (WebClient webClient = new WebClient())
        {
            webClient.Headers["Content-type"] = "application/json";
            webClient.Encoding = Encoding.UTF8;

            logger.Info("Sending [" + method + "] request to API[" + string.Format(Configuration.getTargetURI(), taskName) + "]");

            response = webClient.UploadString(string.Format(Configuration.getTargetURI(), taskName), method, data);

            logger.Info("Response of API[" + string.Format(Configuration.getTargetURI(), taskName) + "] successfully retrieved");
            logger.Debug("Response: " + response);
        }
    }
    catch (Exception ex)
    {
        logger.Info("Failed to invoke [" + method + "] request to API[" + string.Format(Configuration.getTargetURI(), taskName) + "], Error: " + ex.Message);
        logger.Error(ex.Message + ex.StackTrace);

        throw ex;
    }

    return response;
}

public string getResponseNoInput(string taskName, string method)
{
    string response = string.Empty;

    try
    {
        WebRequest req = WebRequest.Create(String.Format(Configuration.getTargetURI(), taskName));
        req.Method = method;
        req.ContentType = "application/json; charset=utf-8";

        logger.Info("Sending [" + method + "] request to API[" + string.Format(Configuration.getTargetURI(), taskName) + "]");

        HttpWebResponse resp = req.GetResponse() as HttpWebResponse;
        using (StreamReader sr = new StreamReader(resp.GetResponseStream()))
        {
            response = sr.ReadToEnd();
        }

        logger.Info("Response of API[" + string.Format(Configuration.getTargetURI(), taskName) + "] successfully retrieved");
        logger.Debug("Response: " + response);
    }
    catch(Exception ex)
    {
        logger.Info("Failed to invoke [" + method + "] request to API[" + string.Format(Configuration.getTargetURI(), taskName) + "], Error: " + ex.Message);
        logger.Error(ex.Message + ex.StackTrace);

        throw ex;
    }

    return response;
}

Looking at your code I don't see any glaring performance issues. What you really need to do is measure the performance of any code that accesses an outside resource, such as a file or web service. That said, I would benchmark the execution time for the following items:

All calls to logger since I assume these hit the file system or database. Also be wary of logging to a shared drive, as this is really a network service call disguised as a direct call to the hard disk.
The call to the WebClient.UploadString method. If that service gets bogged down, your middleware will run slow as well.
The last item that might be slow is deserializing the input from JSON.

The more likely culprits are calls utilizing external resources, such as web services or the file system.


RLE-like string compression in clojure There have been already many similar questions to this one (e.g. this and this, just to point two), but I did not find any for clojure.

The goal is to compress a string in a RLE-like way: i.e. each character "run" is replaced by the character and the count of the run. Some examples:

aaaabbccc       -->      a4b2c3
b               -->      b1
aaaaaaaaaaaa    -->      a9a3

Some remarks:

For now, I do not care about the possible optimisation that runs with the length of 1 can omit the count. (E.g. the output could be b instead of b1.)
If there are more than n > 9 characters in a run, then it is split up into two or more runs, like 9 + 9 + 9 + ... + k = n, where k <= 9.
For now, I do not care if the "compressed" string will be longer than the original one (this is the case e.g. for an input containing only single characters, like: abcde --> a1b1c1d1e1).

That said, please see my implementation below:

Source

(defn countrepetitions [s]
      (if (empty? s) 0
          (loop [acts (vec s) ch (first acts) cnt 1]
                (if (= ch (first(rest acts)))
                   (recur (rest acts) ch (inc cnt))
                    cnt))))

(defn mycompress [s]
      (loop [acts s encoded "" cnt1 (countrepetitions acts)]
            (if (= 0 cnt1)
                encoded
                (let [cnt2 (if (< cnt1 10) cnt1 9)]
                     (recur (subs acts cnt2) (str encoded (first acts) cnt2) (countrepetitions (subs acts cnt2)))))))

Tests

(deftest a-test
  (testing "compressor"
    (is (= "" (mycompress "")))
    (is (= "a2" (mycompress "aa")))
    (is (= "a1b8a1c7a3" (mycompress "abbbbbbbbacccccccaaa")))
    (is (= "b9b3" (mycompress "bbbbbbbbbbbb")))))

Questions

In particular, I would be interested in the following, but any other remark/suggestion is also welcome:

Is there a more efficient way (either in terms of readability or speed) to implement countrepetitions, or maybe leave it out completely?
Are there any other tests worth adding? Or can you find a "counter-example" (i.e. a test case, for which the implementation does not work correctly)?

Whenever you're splitting a list up into a number of chunks, it's worth looking into the partition function (and related functions).

A string is just a list of characters, and you just want to split it when the character changes, so (partition-by identity characters) should be much simpler than what you have above.  You can pair that with another partition to make sure you have no chunks of > 9 characters:

(defn rle-chunks [string]
  (->> string
       (partition-by identity)
       (mapcat #(partition-all 9 %))))

Actually performing the compression is then just a case of calling first & count on each of these chunks, then converting back to a string:

(defn rle [input]
  (->> input
       rle-chunks
       (map (juxt first count))
       (apply concat)
       (apply str)))


Best Practices concerning Includes and SQL Connect Strings I'm just starting to learn PHP. I have three random questions about PHP and one question about Code Review. None of them are technical questions ("will this work?"); they're best practices questions ("is this the best way to do this?"). Can you offer advice?

I put commonly used data in separate files and use include to insert them into other pages. This avoids typing the exact same code into dozens of files. Is that considered a best practice? Is there a better way to organize common page elements? An example:
A file named footer.php:

<?php echo "
Copyright Michael

Go home
" ?>

And a snippet that's in all my other files:

 <?php include "footer.php ?>

What's the best way to store a MySQL connect string? I saved it in a separate "util.php" file. It reads something like:

<?php $con=mysqli_connect("localhost","admin","password","devDatabase");
if (mysqli_connect_errno())
{
    echo "Failed to connect to MySQL: " . mysqli_connect_error();
}
?>

All files that connect to the db have:

<?php include "util.php" ?>

Is than an efficient and/or secure way to store the connection string?
What PHP frameworks do you recommend? I'm trying to get a solid understanding of PHP/Javascript/MySQL for basic web design. After that, should I select a particular PHP framework? Or should I skip PHP and move to a newer language like Ruby on Rails?

Finally, can I get feedback by posting my URL here? If I list www.mynewsite.com and post the code for the main PHP files, could I get general feedback on the site?

1.) What you do here is to extract code used in several locations into a shared file. Instead of describing what do to you refer to a building block to use here (the footer building block). This is a technique commonly used in other engineering areas as well (e.g. construction). This approach greatly boosts re-usability. In my opinion it helps to see software as a set of building blocks to combine (of course those are composed of other smaller building blocks as well). Many software architecture diagrams and techniques use this view.

Other principles this approach relates to are:

Modularity: Decompose your system into modules. Each module represents one functional aspect of your application. Related is the separation of concerns: each building block has one dedicated responsibility., i.e. the rendering of the footer.
Information hiding: Hides the detail how the footer is actually rendered and how the information is retrieved.
Don't repeat yourself: Probably the most generic one applicable for all of real-life as well: Don't repeat the footer, refer to it.

These principles usually are applied at another level of architecture. Yet it really helps to practice them in a smaller scale. These principles (and others not mentioned) are the foundation of software development. They are very generic though and in my opinion you really can't learn them but do experience them by applying them gradually while refining your projects.

2.) For smaller projects it is probably a good idea to keep all configuration in one location. How to actually store passwords and other sensitive information is really up to your use-case: either store it in a file, in environment variables and others. Especially for web-sites it is very common to store all configuration in one file (including passwords). Of course this file should not be accessible by visitors ;).

3.) I won't give a recommendation on any particular framework or language. With experience it mostly doesn't matter. The common principles and patterns are the same for most languages and frameworks. Yet I highly recommend to have a look at other projects and try to understand them. Learn by observing how others solve problems. Start with some smaller projects (can be as simple as a single function or class) and gradually approach more complex frameworks. Among the complex frameworks written in PHP are the big MVC-Frameworks (e.g.. Symphony, Zend Framework... ) or Doctrine. The important thing really is to understand and work with those frameworks, yet abstract their approach and not just learn how this is done in this particular framework. Of course this requires a lot of research and experimentation :).

A final remark: the stackexchange sites works best for one new question per (unrelated) question (e.g. one for each, 1 & 2).


Bash script to clear files not matching a given extension Using LaTeX creates a bunch of auxiliary files like foo.aux, foo.log, etc. Sometimes, I want to create a clean run of my document. I usually manually execute this in the terminal as

$ ls foo*
foo.tex
foo.aux
foo.log
foo.bbl
foo-blx.bib
$ rm foo{.aux,.log,.bbl,-blx.bib}

This works fine. However, it is error-prone and I don't want to accidentally erase my .tex file. So I added this function to my ~/.bashrc:

# Delete all non-TeX files
# invoke as: cleantex foo
# where: foo.tex, foo.aux, foo.log, etc. are files in directory
cleantex () {
    if [ -n "$1"]; then
        name = $(basename -s ".tex" "$1")
        rm -f $name{.!(*tex),-blx.bib}
    fi
}

My question is about the key line

rm -f $name{.!(*tex),-blx.bib}

that actually executes the script.

Is this line well-written? What might I improve here?

My real concerns with the script are:

cleantex path/to/file.tex will not delete files in that path, but in the current directory.
you should check the actual .tex file exists before you delete all the things around it.
the brace-expansion is unnecessarily complicated...... especially when combined with the extended glob !(*tex). I would manually resolve the brace-expansion so that there is only one complicated operation on that line.
I would use actual glob-expansion and only delete existing files.... and not use the -f option on rm (which does more than just suppress the error message if files do not exist....)


Building a chain of responsibility in Ruby to apply transformators on an object I try to make a middleware stack system in the rack way (but not for HTTP request).

Here is the main class:

class MiddlewareStack

  def self.stack middleware
    @middlewares ||= []
    @middlewares << middleware
  end

  def self.middlewares
    @middlewares
  end

  def self.next payload
    next_middleware = @middlewares.shift
    next_middleware.apply(payload, self)
  end

  def self.apply payload
    self.next payload
  end

end

Usage

We can setup a middleware stack this way:

class PlusOneMiddleware
  def self.apply payload, stack
    stack.next(payload) + 1
  end
end

class DummyMiddleware
  def self.apply payload, stack
    return payload
  end
end

class PlusOneStack < MiddlewareStack
  stack ::PlusOneMiddleware
  stack ::DummyMiddleware
end

Now we can call the stack like this:

PlusOneStack.apply(1) #return 2

This use case is pretty useless, but I plan to use it for filtering and caching purposes.

Questions

What do you think about the naming/the code?
Do have a better implementation in mind?
What do you think about the test suite? (above)

Test suite

describe MiddlewareStack do

  context "when we stack middlewares" do
    it "contains all the middlewares in the right order" do
      expect(DummyMiddlewareStack.middlewares).to eq [PlusOneMiddleware, DummyMiddleware]
    end
  end

  context "when we apply the middleware stack" do
    it "call all the middlewares" do
      expect(PlusOneMiddleware).to receive(:apply).ordered.and_call_original
      expect(DummyMiddleware).to receive(:apply).ordered.and_call_original
      PlusOneStack.apply(1)
    end
  end

end

I'm concerned by the interplay between the two middlewares. Why is PlusOneMiddleware.apply responsible for calling the next middleware?

It breaks the sequence implicit in the stack being a stack. In effect, the payload passes through DummyMiddleware before PlusOneMiddleware although they've been stacked in the opposite order.

It also requires PlusOneMiddleware to know that there is a "next" middleware. But why should it know or care?

I'd expect the stack to be responsible to for calling the middlewares in sequence, and the middlewares being ignorant of each other. E.g.

class MiddlewareStack
  def self.apply payload
    middlewares.reduce(payload) do |payload, middleware|
      middleware.apply(payload, self)
    end
  end
end

Your tests are misleading too. You have a spec that says

  MiddlewareStack: When we apply the middleware stack it call [sic] all the middlewares

But "it" - the middleware stack - doesn't actually do that. It only happens because the PlusOneMiddleware calls .next on the stack. Which, again, shouldn't be its responsibility. If you reverse the order of middlewares, only one gets called. If you only add the PlusOneMiddleware you get NoMethodError: undefined method ‘apply’ for nil:NilClass because @middlewares.shift returns nil.

In other words, you specs will break if you do this:

class PlusOneStack < MiddlewareStack
  stack ::DummyMiddleware
  stack ::PlusOneMiddleware # doesn't get called
end

or this:

class PlusOneStack < MiddlewareStack
  stack ::PlusOneMiddleware # raises NoMethodError on nil
end

which sort of negates the entire idea of a flexible stack of arbitrary middlewares.

Similarly, I should be able to do this:

class PlusOneStack < MiddlewareStack
  stack ::PlusOneMiddleware
  stack ::DummyMiddleware
  stack ::AnotherMiddleware
end

But AnotherMiddleware will never be called, which doesn't really make sense: It's in the stack, right?

There's also the use of shift in MiddleWareStack. Especially as the stack's not an instance, it's strange to see the @middlewares array being manipulated when the stack is "run". Doubly so since .next can be called from the middlewares themselves.

Other small stuff:

You set a default value for @middlewares in .stack, but you should probably be doing that in your .middlewares accessor method. And you should then use said accessor in your other methods. For instance, if you never add any middleware at all, .apply will fail, since @middlewares is nil.
However, you may not want to expose a @middlewares accessor to the world, as anyone can then mess with it. So beware.
Take a cue from Rack, and name the main middleware method call. That way, you can use plain procs and lambdas as middleware in addition to custom classes and objects. And you may want to check for that with respond_to? when stacking middlewares.

A brutally simple implementation might simply look like:

class MiddlewareStack
  class << self
    def use middleware # same method name as Rack
      raise ArgumentError unless middleware.respond_to?(:call)
      stack << middleware
    end

    def apply payload
      stack.reduce(payload) do |payload, middleware|
        middleware.call payload
      end
    end

    private

    def stack
      @stack ||= []
    end
  end
end

Edit: After the reading the comments, something like this might be a more fitting solution. Where a stack/middleware construction would require an agreed-upon API between stack and middlewares, this is more of a linked-list construction, where there no central "stack", and the agreed-upon API is instead between the members of the linked-list. Note that this is just a quickly-written example

class ExampleLinkedHandler
  def initialize(name)
    @name = name
  end

  def append(handler)
    if @next
      @next.append(handler)
    else
      @next = handler
    end
  end

  def call(level = 0)
    if @next
      "#{@name} (level #{level}) <- #{@next.call(level + 1)}"
    else
      "#{@name} (level #{level})"
    end
  end
end

chain = ExampleLinkedHandler.new("head")

chain.append(ExampleLinkedHandler.new("1st"))
chain.append(ExampleLinkedHandler.new("2nd"))
chain.append(ExampleLinkedHandler.new("tail"))

puts chain.call(0) # prints "head (level 0) <- 1st (level 1) <- 2nd (level 2) <- tail (level 3)"

You can of course have any kind of derived or duck-typed classes/objects as longs as they respond to append and call in a similar manner.


Vertical-align in Nested Column Requirements:

There are two columns. The left column is fixed width.
The height of two Column should be the same.
Left column contain a title (can be multi-line) and an Image (limit max width to left column's width, but can be any height)
Right column contains rich text content.
If the height of left column is higher than right column, content of right should remain vertical center.
If the height of right column is higher than left column, the Title should stay top and the Image should stay vertical center in the rest of the space.

Edit:
Oh, sorry. I found that I can't do 5 & 6 in the same time...  Is it possible make it work on IE 7 in pure CSS?

.row-Item{
    border: 3px solid #F9BE00;
    position: relative;
}

.left-column{
    position: absolute;
    width:200px;
    background : #F9BE00;
    top: 0;
    left: 0;
    bottom: 0;
}

.right-column{
    margin-left:200px;
    vertical-align: middle;
    background: #FEF2CC;
}

.warpper{
    display:table;
    min-height: 100%;
}

.left-column .text-container{
    display:table-row;
    height:1px;
    font-size:20px;
    font-weight:bold;
}

.left-column .img-container{
    display:table-row;
    height:100%;
    text-align:center;
}

    <meta charset="utf-8">
    <title>JS Bin</title>

                                Some Title Here

                                May be multi-line

            Content text go heres. Content text go heres. Content text go heres.    Content text go heres. Content text go heres. Content text go heres.

some list item..

some list item..

some list item..

some list item..

             Content text go heres. Content text go heres. Content text go heres.Content text go heres. Content text go heres. Content text go heres.Content text go heres. Content text go heres. Content text go heres.Content text go heres. Content text go heres. Content text go heres.   Content text go heres. Content text go heres. Content text go heres.

Live

As Kid Diamond stated in a comment, your HTML has errors - 5 of them.  You can validate it by running it through the W3C validator.

First off, you need a DOCTYPE so browsers know which version of HTML you are using - the latest is simply <!DOCTYPE html>

Second, img elements need alt attributes so screen readers, search engines, and other computer programs can make sense of them:

<img src= "http://static.jsbin.com/images/jsbin_static.png" alt="image description">

Finally, you forgot to close one of your divs - the div around the img element.

Your CSS has no errors at the W3C CSS validator.


Rolling dice simulator with probability I'm a beginner at Python, and to start off my learning progress I created a simple program which will roll a dice and decides the probability of it. It would be great to have criticism on my code, like how I can improve it, make it shorter, etc.

import random

rolled = []
rolledtimes = 0;
biggest = []

freq = int(input('How many times would you like to roll the dice? '))

def roll():
    rand = random.randrange(1,7)
    return rand
def probability():
    for i in range(0,6):
        print('Calculation of probability:')
        percentage = "{:.2f}".format((count[i] / freq)*100)
        percent = str(percentage) + '%'
        print(' ', i + 1, ':', percent)
def theoretical():
    result = "{:.2f}".format((1/6)*freq)
    denominator = "{:.2f}".format(((1/6)*freq)*6)
    print('\nIn theory, each dice should roll {} out of {} times'.format(result,denominator))
def findBiggest():
    for i in range(1,7):
        biggest.append(rolled.count(i))
    print('\n', 'The most times a dice is rolled is', max(biggest), 'times')
def findSmallest():
    for i in range(1,7):
        biggest.append(rolled.count(i))
    print('\n', 'The least times a dice is rolled is', min(biggest), 'times')

for i in range(1,freq + 1):
    number = roll()
    rolled.append(number)
    rolledtimes+=1
count = [rolled.count(1),rolled.count(2),rolled.count(3),rolled.count(4),rolled.count(5),rolled.count(6)]
print('After being rolled {} times:\n\n1 is rolled {} times\n2 is rolled {} times\n3 is rolled {} times\n4 is rolled {} times\n5 is rolled {} times\n6 is rolled {} times\n'.format(rolledtimes,count[0],count[1],count[2],count[3],count[4],count[5]))

probability()
findBiggest()
findSmallest()
theoretical()

When used, it will print out something like this in the console:

How many times would you like to roll the dice? 1000
After being rolled 1000 times:

1 is rolled 180 times
2 is rolled 161 times
3 is rolled 190 times
4 is rolled 145 times
5 is rolled 162 times
6 is rolled 162 times

Calculation of probability:
  1 : 18.00%
Calculation of probability:
  2 : 16.10%
Calculation of probability:
  3 : 19.00%
Calculation of probability:
  4 : 14.50%
Calculation of probability:
  5 : 16.20%
Calculation of probability:
  6 : 16.20%

 The most times a dice is rolled is 190 times

 The least times a dice is rolled is 145 times

In theory, each dice should roll 166.67 out of 1000.00 times

For styling, variable naming etc. conventions, please take a look at PEP-8. Your globals should be UPPERCASED and function names should be snake_cased.

Since you are not at all interested in the rolled list, why store a thousand values in memory?

You should always seed when using a RNG function. For more information on why this practice is favoured, check this SO response.

Instead of using random.randrange, prefer using random.randint.

With those in place, you have:

import random
import operator

random.seed()

ROLLED = {i: 0 for i in range(1, 7)}
ITERATIONS = int(input('How many times would you like to roll the dice? '))

def probability():
    print("Calculation of probability: ")
    for key, count in ROLLED.items():
        print("\t{}: {:.2f}".format(key, count*100./ITERATIONS*1.))

for _ in range(ITERATIONS):
    ROLLED[random.randint(1, 6)] += 1

probability()

To find the most rolled, and least rolled face of the die, you can use a custom operator on ROLLED dictionary:

max(ROLLED.iteritems(), key=operator.itemgetter(1))
min(ROLLED.iteritems(), key=operator.itemgetter(1))


Haskell program to turn off computer This a program I wrote to execute various power-related function on my computer. It accepts one command-line argument that describes what action to take and then performs that action. Usage looks like this:

C:\Users\Noah\My Code\GHCI> power sleep
execute sleep? (y/n)
y

//computer goes to sleep

Here's the code. Any feedback whatsoever is appreciated, regarding either my program's design or the style, formatting and good/bad practices of my code.

--power.hs

import System.Cmd(system)
import System.Environment(getArgs)

type Arg = String --A command-line argument
type Cmd = String --A command to execute

--paths to programs that will carry out the actions
shutdown_path = "C:\\Windows\\System32\\shutdown.exe"
rundll32_path = "C:\\Windows\\System32\\rundll32.exe"

--assoc list mapping arguments to the action that should be taken when that argument is received
cmds :: [(Arg, Cmd)]
cmds = [ ("shutdown",  shutdown_path ++ " /s /t 0"                           ),
         ("sleep",     rundll32_path ++ " powrprof.dll,SetSuspendState 0,1,0"),
         ("hibernate", shutdown_path ++ " /h"                                ), -- does not work
         ("lock",      rundll32_path ++ " user32.dll,LockWorkStation"        ),
         ("restart",   shutdown_path ++ " -t 0 -r -f"                        ) ]

--gets the argument passed to the program. Calls error if there is not exactly one argument.
extractCmdOrFail :: IO Arg
extractCmdOrFail = getArgs >>= check
    where check [s] = return s
          check _   = error "Wrong Number of arguments"

--finds the command to execute from the given command-line arg. Calls error if no command is found.
findCmdOrFail :: Arg -> Cmd
findCmdOrFail s = check $ lookup s cmds
    where check (Just s') = s'
          check Nothing   = error "Unknown command: " ++ s

--prompts the user to confirm whether or not they want to execute the command.
prompt :: Cmd -> Arg -> IO ()
prompt cmd name = putStr ("Execute " ++ name ++ "? (y/n)\n") >> getChar >>= branch cmd

--conditional "loop" that either executes the command, cancels, or awaits valid input.
branch :: Cmd -> Char -> IO ()
branch cmd 'y' = system cmd >> return ()
branch cmd 'n' = putStrLn "Command Cancelled" >> getChar >> return ()
branch cmd _   = getChar >>= branch cmd

--main function
main = extractCmdOrFail >>= (\arg -> prompt (findCmdOrFail arg) arg)

The code is short and nicely done, so I don't have much comment

In branch, you could use void instead of return ()

branch cmd 'y' = system cmd >> void
branch cmd 'n' = putStrLn "Command Cancelled" >> getChar >> void

In your 'n' match, you use "getChar". I think it's to prevent the console from closing, right? Be aware that there is other ways, but it really depend on how you want to use your program.

Haskell use camelCase, so shutdown_path and rundll32_path should be shutdownPath and rundll32Path

Check

Your actual check functions are fine. But if you have more case like this, you should really consider this:

You could make the check function a top level one like this

check :: String -> Maybe a -> a
check _ (Just s) = s
check errorMsg Nothing   = error errorMsg

The check call from findCmdOrFail will remain practically unchanged, you just have to add your error message

The interesting part is the one from extractCmdOrFail, you have to use an helper function that I didn't find on hackage

whenMaybe :: (a -> Bool) -> a -> Maybe a
whenMaybe predicate x = if predicate x then Just x else Nothing

then your extractCmdOrFail will looks like this

extractCmdOrFail = getArgs >>= check (whenMaybe  $ {-your predicate-}) "Wrong Number of arguments"


Cloning Python standard library's function math.frexp() that extracts exponent and mantissa from a floating point number This is Python 3.5 code.

I have attempted to develop my own frexp() function that mimics the functionality of Python standard library's math.frexp().

I'm interested in suggestions for how improve my code style to become more Pythonic and how to achieve better performance and precision for this function.

My code:

import ctypes
def frexp(x):
    *binstr, = bin(ctypes.c_int.from_buffer(ctypes.c_float(x)).value)
    if binstr[0] == '-' : binstr.pop(0)
    exponent = int ( (''.join(binstr))[2:10], 2 ) - 127
    mantissa = 1
    for index, item in enumerate(''.join(binstr)[10:]):
        if item == '1':
            mantissa += 2 ** ( -(index + 1) )
    return mantissa, exponent

Precision: not accurate

>>> import random
>>> testednumber = random.random() + random.random() * ( 10 ** 10 )
>>> testednumber
5478324268.848498
>>> # This is my code's output
>>> import mymath
>>> mymath.frexp(testednumber)
(1.2755217552185059, 32)
>>> # This is Python's frexp() output
>>> import math
>>> math.frexp(testednumber)
(0.6377608828303053, 33)
>>> # my result:
>>> (1.2755217552185059) * ( 2 ** 32 )
5478324224.0
>>> # Python's frexp() result:
>>> (0.6377608828303053) * ( 2 ** 33 )
5478324268.848498

Speed: 55 times slower than math.frexp()

>>> # This is my minimum speed
>>> print(min(timeit.Timer("mymath.frexp(testednumber)", setup="import mymath; testednumber = 5478324268.848498").repeat(7,1000)))
0.01864783600103692
>>> print(min(timeit.Timer("mymath.frexp(testednumber)", setup="import mymath; testednumber = 5478324268.848498").repeat(7,1000)))
0.01870606800002861
>>> print(min(timeit.Timer("mymath.frexp(testednumber)", setup="import mymath; testednumber = 5478324268.848498").repeat(7,1000)))
0.018558342000687844
>>> # This is Python's frexp() minimum speed
>>> print(min(timeit.Timer("math.frexp(testednumber)", setup="import math; testednumber = 5478324268.848498").repeat(7,1000)))
0.0003347589990880806
>>> print(min(timeit.Timer("math.frexp(testednumber)", setup="import math; testednumber = 5478324268.848498").repeat(7,1000)))
0.0003345459990669042
>>> print(min(timeit.Timer("math.frexp(testednumber)", setup="import math; testednumber = 5478324268.848498").repeat(7,1000)))
0.0003347729998495197

1. Review

The ctypes module is dangerous (it's possible to crash Python if you get it wrong) and so it should be used as a last resort, when no other facilities will do the job. In this case we could use the struct module instead:

>>> # Reinterpret double-precision float as a 64-bit unsigned integer
>>> struct.unpack('Q', (struct.pack('d', 5478324268.848498)))
(4752538446597034867,)

The code in the post gets at the bits in the representation by converting to a string of 0s and 1s. It would be better to use Python's bitwise operators instead, to avoid converting to a string and back again. Let's start by declaring named constants for the shifts and masks:

# IEEE-754 double-precision floating-point format
SIGN_SHIFT = 63
EXPONENT_SHIFT = 52
EXPONENT_MASK = (1 << 11) - 1
MANTISSA_MASK = (1 << 52) - 1

and then

Q, = struct.unpack('Q', (struct.pack('d', 5478324268.848498)))
sign = Q >> SIGN_SHIFT
biased_exponent = (Q >> EXPONENT_SHIFT) & EXPONENT_MASK
mantissa = Q & MANTISSA_MASK

To assemble the result, normalize the exponent and reverse the disassembly process:

NORMAL_EXPONENT = 1023
R = (sign << SIGN_SHIFT) + (NORMAL_EXPONENT << EXPONENT_SHIFT) + mantissa
normalized, = struct.unpack('d', struct.pack('Q', R))
return normalized, biased_exponent - NORMAL_EXPONENT

(Use 1022 instead of 1023 if you want to match math.frexp exactly.)

2. Revised code

from math import isinf, isnan

# IEEE-754 double-precision floating-point format
SIGN_SHIFT = 63
EXPONENT_SHIFT = 52
EXPONENT_MASK = (1 << 11) - 1
MANTISSA_MASK = (1 << 52) - 1
NORMAL_EXPONENT = 1023

def frexp(x):
    """Return the mantissa and exponent of x, as pair (m, e).
    m is a float and e is an int, such that x = m * 2.**e.
    If x is 0, m and e are both 0.  Else 1.0 <= abs(m) < 2.0.

    """
    x = float(x)
    if x == 0.0 or isinf(x) or isnan(x):
        return x, 0
    Q, = struct.unpack('Q', struct.pack('d', x))
    sign = Q >> SIGN_SHIFT
    biased_exponent = (Q >> EXPONENT_SHIFT) & EXPONENT_MASK
    mantissa = Q & MANTISSA_MASK
    R = (sign << SIGN_SHIFT) + (NORMAL_EXPONENT << EXPONENT_SHIFT) + mantissa
    normalized, = struct.unpack('d', struct.pack('Q', R))
    return normalized, biased_exponent - NORMAL_EXPONENT

(Note that this doesn't get denormals right. I could fix it with some more work, but I think I'd rather just use math.frexp.)

3. Use cases for frexp

frexp is useful when computing mathematical functions, where we need to get the argument into a known range in order to avoid overflow, or to use a particular solution technique. Two examples:

To compute \$\log a\$ we can use frexp to find \$a = m2^e\$ with \${1\over2} \le m < 1\$ and then $$\log a = \log m2^e = \log m + e\log 2$$ and since \$m\$ is close to 1 we can use polynomial approximation (and \$\log 2\$ can be hard-coded).
The geometric mean of \$a_1, a_2, \dots, a_n\$ is \$\sqrt[n]{a_1a_2\dotsb a_n}\$ but the multiplication might overflow the floating-point range. Using frexp we can replace \$a_i\$ by \$m_i2^{e_i}\$ which gives us $$ \eqalign{ \sqrt[n]{a_1a_2\dotsb a_n} &= \sqrt[n]{m_12^{e_1}m_22^{e_2}\dotsb m_n2^{e_n}} \\ &= 2^{(e_1 + e_2 + \dotsb + e_n)/n}\sqrt[n]{m_1m_2\dotsb m_n}} $$ where \${1\over2} \le m_i < 1\$. We can now divide the multiplication into chunks of length 1023 or less, knowing that each chunk can be multiplied and remain in range. We then use frexp on the results of the chunks to extract some more powers of 2, and repeat.

(An alternative approach to computing the geometric mean while keeping the intermediate results in range would be to use logarithms and exponentiation: $$ \sqrt[n]{a_1a_2\dotsb a_n} = \exp\left({\log a_1 + \log a_2 + \dotsb + \log a_n} \over n\right) $$ but computing a logarithm is much more expensive than multiplying.)


OBBs intersection improvement for AABBs-OBBs I'm trying to code some intersection codes for Oriented Bounding Boxes (OBBs) and axis-aligned bounding boxes (AABBs). I have the OBB-OBB intersection using the Separating Axis Theorem (SAT) and I'm trying to figure out how can I improve this code to the specific case where I know I'm comparing an OBB with an AABB.

This is the code of my function:

bool Collider::isColliding(OBB * one, OBB * other)
{
    TestManager::collisionTests++;
    glm::vec3 oneCenter = one->m_gameObject->getWorldPosition(); // object's pos = collider center
    glm::mat4 oneTransform = glm::scale(one->m_gameObject->getTransform(), one->m_halfSize); // scaling for halfsize
    glm::vec3 otherCenter = other->m_gameObject->getWorldPosition();
    glm::mat4 otherTransform = glm::scale(other->m_gameObject->getTransform(), other->m_halfSize);

    for (int a = 0; a < 3; a++) {
        glm::vec3 l = glm::vec3(oneTransform[a]); // one axis to project on
        float tl = std::abs(glm::dot(l, otherCenter) - glm::dot(l, oneCenter)); // center distance
        float ra = std::abs(glm::dot(l, glm::vec3(oneTransform[0]))) + std::abs(glm::dot(l, glm::vec3(oneTransform[1]))) + std::abs(glm::dot(l, glm::vec3(oneTransform[2])));
        float rb = std::abs(glm::dot(l, glm::vec3(otherTransform[0]))) + std::abs(glm::dot(l, glm::vec3(otherTransform[1]))) + std::abs(glm::dot(l, glm::vec3(otherTransform[2])));
        float penetration = (ra + rb) - tl;
        if (penetration <= 0) { // no overlap
            return false;
        }
    }
    for (int b = 0; b < 3; b++) {
        glm::vec3 l = glm::vec3(otherTransform[b]); // other axis to project on
        float tl = std::abs(glm::dot(l, otherCenter) - glm::dot(l, oneCenter)); // center distance
        float ra = std::abs(glm::dot(l, glm::vec3(oneTransform[0]))) + std::abs(glm::dot(l, glm::vec3(oneTransform[1]))) + std::abs(glm::dot(l, glm::vec3(oneTransform[2])));
        float rb = std::abs(glm::dot(l, glm::vec3(otherTransform[0]))) + std::abs(glm::dot(l, glm::vec3(otherTransform[1]))) + std::abs(glm::dot(l, glm::vec3(otherTransform[2])));
        float penetration = (ra + rb) - tl;
        if (penetration <= 0) { // no overlap
            return false;
        }
    }
    for (int a = 0; a < 3; a++) {
        glm::vec3 aAxis = glm::vec3(oneTransform[a]);
        for (int b = 0; b < 3; b++) {
            glm::vec3 bAxis = glm::vec3(otherTransform[b]);
            if (aAxis != bAxis) {
                glm::vec3 l = glm::cross(aAxis, bAxis); // has flaw when axis are same, result in (0,0,0), solved by if
                float tl = std::abs(glm::dot(l, otherCenter) - glm::dot(l, oneCenter)); // center distance
                float ra = std::abs(glm::dot(l, glm::vec3(oneTransform[0]))) + std::abs(glm::dot(l, glm::vec3(oneTransform[1]))) + std::abs(glm::dot(l, glm::vec3(oneTransform[2])));
                float rb = std::abs(glm::dot(l, glm::vec3(otherTransform[0]))) + std::abs(glm::dot(l, glm::vec3(otherTransform[1]))) + std::abs(glm::dot(l, glm::vec3(otherTransform[2])));
                float penetration = (ra + rb) - tl;
                if (penetration <= 0) { // no overlap
                    return false;
                }
            }
        }
    }
    return true;
}

Hope you can help me.

Broad Phase collision detection

It looks like you are using this for what is called broad phase collision detection. This is the step in collision detection that is used to determine candidate pairs for the narrow phase that does the actual collision detection (Quick Summary)

As this is a bounding box text (especially if you are involving AABB) it looks like you're just trying to see whether to go through the actual intersection routing of the two shapes or not. If you can just stick with AABB in general, the math gets a lot easier and therefore faster. Also moving forward you can employ AABB-trees (they are space partinioning trees) to find all intersection candidates for one object with a tree query rather than a pairwise check.


Optimizing mathematical index calculation function I have made a function which calculates the index of a point to which an arbitrary point on a polygon made of XY points belong.

This is a visual representation:

And this is the function I made:

stock GetNodeIndexFromPolygonIndex(polygonid,Polygon_Size)
{
    new polid = (polygonid - (polygonid % 2));
    new mid = Polygon_Size/2;
    if(polid > mid)
    {
        polid /= 2;
        return (mid - (++polid));
    }
    else
    {
        polid /= 2;
        if(polid == 0)
        {
            return 0;
          }
        return --polid;
    }
}

Is there anything I can optimize here?  This function is going to be called ~2000 times in the worst case in one run on a single threaded application. I would like this to be as optimal as possible.  Is it already?

First, I do not know anything about PAWN.

Does it have a profiler? Can any profiler be used? If yes, probably use it.
If no, write at least some test units to measure execution time.

I will assume typical language elements. I will assume polygonid > 0 and Polygon_Size > 0, because I do not know (and did not find in short time) the behavior of / and % for negative numbers.

As far as I understood, there is a virtual machine between compiler and execution. This makes things a little bit more complex, but well, we will assume that nothing special happens there.

Some of the optimizations could change nothing, because the compiler already does it. I can not predict or test it.

That said, I will choose line after line. I do not know if you can change the algorithm, because like Cygal I do not understand the exact purpose.

new polid = (polygonid - (polygonid % 2));

This makes polid the next smallest even number? % is rather costly, you could either do a polygonid & 1 instead of polygonid % 2 or new polid = polygonid & (max_value - 1) which will set all bits, but the last one to zero. max_value - 1 should be precalculated.

new mid = Polygon_Size/2;

Change to: new mid = Polygon_Size >> 1 This will be done most probably by the compiler anyway, but we do not know, so lets try it.

polid /= 2;

This is calculated in both branches. It could help to do it before the branching:

new polid_half = polid >> 1;
...

return (mid - (++polid));

This depends on the language details. It could be that ++polid as preincrement operator will load polid, update it by one, and store polid back. We do not plan to use the modified polid, so we could try return (mid - polid + 1) assuming that ++polid is not some magic fast special instruction compared to the normal +.

    if(polid == 0)
    {
        return 0;
    }

This does only happen if polygonid is 0 or 1. If this happens a lot, you should return immediately at the beginning, saving all the rest:

if (polygonid < 2 )  //assuming polygonid > 0
    return 0;

And remove the check inside the else branch.

    return --polid;

Same as before, it could be better to return polid - 1

Depending on the branch prediction handling, it could be better to have only one return point, not one in every branch. You could introduce a return value and set it according to the current logic.

I would try one change after the next and profile every step. All together, it could be:

stock GetNodeIndexFromPolygonIndex(polygonid,Polygon_Size)
{
    if (polygonid < 2 )  //assuming polygonid > 0
        return 0;
    new result = 0;
    new polid = polygonid & (max_value - 1); //polid is a bad name
    new polid_half = polid >> 1;
    new mid = Polygon_Size >> 1;
    if(polid > mid)
        result = mid - polid_half + 1;
    else
        result = polid_half - 1;
    return result;
}


T-SQL Constant Time Comparison Explanation is in the function. Please point out any egregious errors:

create function api.ConstantTimeCompare (
    @x varbinary(8000) = 0x00
  , @y varbinary(8000) = 0x00
)
/*
    What? Compares two variable length binary strings byte by byte in constant time.

    When? Cryptography.

    Why? Given the strings 0x1234 and 0x1213 a simple equality comparison will stop processing after
         it hits the first mismatch at byte position 3. An opponent who only knows @x can exploit this
         feature to guess @y by trying thousands of comparisons and gathering statistics on how long
         each one took.

         In other words, assuming that it takes 100 milliseconds to compare at 16 byte string an
         opponent can rely on the fact that string that only takes 15 milliseconds to process is a
         worse guess than ones that take more than 15 milliseconds. A 50 millisecond comparison in
         this scenario means that @x is a 50% match of @y (from byte 1-8).

    How? Set up a loop based on the length of @x and perform a byte by byte XOR. If @result is still 0
         after all iterations then @x is equal to @y. If the comparison is meant to be secure then the
         untrusted string should be @x and the trusted string should be @y.
 */
returns bit
with schemabinding
as
begin;
    declare @i int = 0;
    declare @l int = DataLength(@x);
    declare @result int = @l - DataLength(@y);

    while (@i < @l)
    begin;
        set @result = ((@result ^ Substring(@x, @i, 1)) ^ Substring(@y, @i, 1));
        set @i = @i + 1;
    end;

    set @result = case @result when 0 then 1 else 0 end;

    return @result;
end;

The general approach is sound, but the xor-ing is unnecessarily complex and actually causes it to give an incorrect answer in some cases.  If I've worked through it correctly, it will report that the empty string is equal to the string with the single byte 1.  The simple approach would be to just increment @result when the strings differ at position @i.

There's another concern that I'm not qualified to comment on for T-SQL, but that I know applies to C# or C++: the algorithm that you've specified should be constant-time, but that is before it gets run through any compilers or optimizers.  If you switch to an algorithm that increments a counter, it's conceivable that an optimizer would realize that once @result is not zero the return value is 1, defeating your efforts.


Converting minutes and seconds in seconds I have 2 input fields to input the duration of a CD song. The first input is for minutes and the second is for seconds

When submitting the form, I must insert in the db the duration either in seconds (minutes + seconds) or NULL.

Here's my validation formula:

$warning = array();

function val_duration($min, $sec, $required = false) {
    global $warning;

    if (!empty($min) || !empty($sec)) {

        if (!empty($min) && (!is_numeric($min) || !ctype_digit($min) || $min > 60)) {
            $warning['min'] = 'The minutes must contain a positive numeric value between 1 and 60';
        } else {
            $min_in_sec = $min * 60;
        }

        if (!empty($sec) && (!is_numeric($sec) || !ctype_digit($sec) || $sec > 59)) {
            $warning['sec'] = 'The seconds must contain a positive numeric value between 1 and 59';
        } else {
            $tmpsec = $sec;
        }

        $value = $min_in_sec + $tmpsec;

    } else {

        if ($required) {
            $warning['duration'] = "The duration is a required field";
        }

        $value = NULL;
    }

    return $value;
}

// HTML
<input id="min" name="min" type="text" value="...">
<input id="sec" name="sec" type="text" value="...">

// validate
$min = ltrim(trim($_POST['min']), '0');
$sec = ltrim(trim($_POST['sec']), '0');

$duration_in_secs = val_duration($min, $sec, true); // duration

// mySQL
enter $duration_in_secs in db (either seconds or NULL)

I'm not familiar with PHP too much but globals are usually considered harmful. I don't know what's the best practice in PHP but passing the $warning argument by reference seems a good idea here.
When a field is not posted you get various PHP notices. I guess it's better to avoid these notices:

PHP Notice:  Undefined index: min in /tmp/x.php on line ...
PHP Notice:  Undefined index: sec in /tmp/x.php on line ...

A robust code should handle these cases.
tmpsec and min_in_sec are not initialized in every code paths, so you can get notices about these too when there is no min key in the $_POST array:

PHP Notice:  Undefined variable: tmpsec in /tmp/x.php on line 24
PHP Notice:  Undefined variable: min_in_sec in /tmp/x.php on line 2

I think it's cleaner to initialize all variables. It also could help debugging since you don't have to ignore lots of PHP notice messages when you are looking for an uninitialized global variable.
tmpsec and min_in_sec could be omitted if you reorganize the code to not count anything until it is sure that there isn't any input error. (See below.)
A small note: The function is a little bit inconsistent: it allows 60 as minute but doesn't as seconds. It would be cleaner and more common to set one hour as hours = 1, minutes = 0 and seconds = 0.

$warning = array();

function val_duration(&$warning, $min, $sec, $required = false) {
    if (empty($min) && empty($sec)) {
        if ($required) {
            $warning['duration'] = "The duration is a required field";
        }
        return NULL;
    }

    if (!ctype_digit($min) || $min > 61) {
        $warning['min'] = 'The minutes must contain a positive numeric value between 1 and 60';
    }
    if (!ctype_digit($sec) || $sec > 59) {
        $warning['sec'] = 'The seconds must contain a positive numeric value between 1 and 59';
    }
    if (!empty($warning)) {
        return NULL;
    }

    return $min * 60 + $sec;
}


Bash script for referencing git status output files Whenever I do git status, I want to reference the files that were shown easily... whether it be so I can open it in vim, git add it, delete it, git checkout (to revert), etc. So after a bit of looking around, I couldn't find any existing script for this so I made my own. It works, but the code is not pretty, and being a beginner bash scripter (if that's even a noun), I feel like I can do so much better with this.

Here is the script:

#usage gits: sets env variables $m<n> $d<n> $u<n> $a (modified, deleted, untracked, added)
function gits {
    git status
    gitsall=$(git status -s)
    m=$(echo "$gitsall" | grep "^ M")
    d=$(echo "$gitsall" | grep "^ D")
    u=$(echo "$gitsall" | grep "^??")
    a=$(echo "$gitsall" | grep "^A ")
    s=$(echo "$gitsall" | grep "^M ")
    #i=0
    #while [[ -n "$(echo {m,d,u}$(($i)))" ]]; do
#   i=$(($i+1))
#   unset {m,d,u}$(($i))
#    done
    count=1
    while read -r tmpfilename; do
    tmpfilename=${tmpfilename:2}
    set m$(($count))="$(pwd)/$tmpfilename"
    count=$(($count+1))
    done <<< "$m"
    count=1
    while read -r tmpfilename; do
    tmpfilename=${tmpfilename:2}
    set d$(($count))="$(pwd)/$tmpfilename"
    count=$(($count+1))
    done <<< "$d"
    count=1
    while read -r tmpfilename; do
    tmpfilename=${tmpfilename:3}
    set u$(($count))="$(pwd)/$tmpfilename"
    count=$(($count+1))
    done <<< "$u"
    count=1
    while read -r tmpfilename; do
    tmpfilename=${tmpfilename:3}
    set a$(($count))="$(pwd)/$tmpfilename"
    count=$(($count+1))
    done <<< "$a"
    count=1
    while read -r tmpfilename; do
    tmpfilename=${tmpfilename:3}
    set s$(($count))="$(pwd)/$tmpfilename"
    count=$(($count+1))
    done <<< "$s"

    unset m d u a s tmpfilename gitsall
}

Basically, I call git status -s, save the output as a variable, then grep it to get lists of filenames of modified, deleted, untracked, and added.

I then parse each line individually and set $ to the respective file in absolute paths.

Now, much of the parsing is the same. Things that vary are env var prefix, substring location (2 or 3 so far), and file list var name. I feel like this can somehow be cleaned up nicely.

Note: I do know this is not going to cover everything. Namely, I only so far implemented it for " M" " D" "??" "A ", but I know others exist like "M ", "MM", etc. which I eventually plan to address. This script is still a WIP for me but I wanted to clean it up before I add all the change types.

I did look around google thinking someone has made something like this because I thought it'll be something people would find convenient. For those who have different workflows that work around my situation, it'll be nice if I can hear them too.

Example usage:

gits

Output of git status and set the appropriate vars

vim $m1

Opens first modified, untracked file in the list from git status

Nice try

That's a fantastic idea.
It's very annoying to run git status and then very often type some next command and copy paste selected files from the output.
I long wanted to have something like this but never made time to actually do it.
You gave me the final push, so thanks.

Setting variables dynamically

I don't think your script actually works :-)
At least on my computer, in Bash 3 or 4,
it's not possible to create variables dynamically with set,
for example:

m1=
c=1
set m$c=x
echo $m1

This outputs nothing, because set m$c=x does not actually perform m1=x.

After some research, I found that this works with declare:

m1=
c=1
declare m$c=x
echo $m1

This outputs x alright. Unfortunately, according to help declare:

[...] When
used in a function, makes NAMEs local, as with the `local' command.

That is, the variables created dynamically this way inside the function will not be visible outside, so this is not usable for this purpose.

One obvious option is eval, but not a good one, because eval is evil.
A well-crafted filename in the working tree could wreak havoc.
Even if this is unlikely, I would rather not resort to such option.

Finally, there's a trick with printf -v that will work well:

m1=
c=1
printf -v m$c x
echo $m1

This outputs x, and even when used in a function, m1 will be visible outside.
The only downside is that printf is not POSIX compliant,
but that's probably not a big problem in practice.

Unnecessary repeated processing of git status -s

This is really quite wasteful:

m=$(echo "$gitsall" | grep "^ M")
d=$(echo "$gitsall" | grep "^ D")
u=$(echo "$gitsall" | grep "^??")
a=$(echo "$gitsall" | grep "^A ")
s=$(echo "$gitsall" | grep "^M ")

It would be better to run git status -s just once,
loop over the output,
use a case statement to decide the type of the change,
and set the m, d, u, a, s variables accordingly.

When you go this way, you can also directly create the variables with a count, so that you don't need all those repetitive while loops for each status type.

Arithmetic context

When within $((...)), you don't need to write $ to access the values of variables, for example instead of:

count=$(($count+1))

You can write:

count=$((count+1))

An even shorter equivalent:

((count++))

Current working directory

Instead of $(pwd) it's better to use the $PWD variable.

Local variables

Instead of using the unset command at the end of the function to clear the variables used, it's much better to use local variables, for example:

local m d u a s

Not only this will make sure you don't clear the variables,
it also makes your use correct.
The current use is not correct,
because if some of these variables had value before calling the function,
the values get reset.
When using local, the original values of those variables remain intact in the calling environment.

Alternative implementation

Putting the above together,
the function could be written simpler as:

gits() {
    git status
    mc=1 dc=1 uc=1 ac=1 sc=1
    local line status path name
    while read -r line; do
        [ "$line" ] || continue
        status=${line:0:2}
        path=${line:3}
        case "$status" in
            " M") name=m$((mc++)) ;;
            " D") name=d$((dc++)) ;;
            "??") name=u$((uc++)) ;;
            "A ") name=a$((ac++)) ;;
            "M ") name=s$((sc++)) ;;
            *) echo unsupported status on line: $line
        esac
        printf -v $name "$path"
    done <<< "$(git status -s)"
}


Find minimum and maximum numbers Write the find_boundaries function, which will take a list of numbers. The function should return the tuple with the smallest and largest number in the set. If there is an item in the list that is not a number, it should be ignored. If an empty list is entered, the function should return None.

My code:

def find_boundaries(lista):

    if not lista:
        return None

    elif lista:
        max_ = lista[0]
        min_ = lista[0]
        new_lista = []

        for i in lista:
            if int == type(i):
                new_lista.append(i)

        for i in range(len(new_lista)):
            if new_lista[i] > max_:
                max_ = new_lista[i]

            elif new_lista[i] < min_:
                min_ = new_lista[i]

    return max_, min_

I am looking for any opinion/ suggestion on that code. It does what is expected, but would you say that this is the most efficient way to solve that task in Python?

welcome to Code Review!

I would write your code like this:

def find_boundaries(lista):
    if not lista:
        return None

    int_list = [x for x in lista if type(x) == int]
    return (max(int_list), min(int_list))

Notice a few things:

If you test your parameters for validity at the beginning of the function, it's perfectly fine not to use an else afterwards because the return statement exits the function (short-circuiting). This saves one level of nesting and makes the function more simple.
Python is designed to let you avoid writing loops as much as possible. Loops are a nuisance to write, take longer to read and increase cognitive load. You should get familiar with Python's list and dict comprehensions. Notice how much clearer the code for filtering out the values that aren't integers is.
Again, the max and min functions are built-in in Python and take any iterable (like a list) to return the largest and smallest element respectively. You don't need to deal with loop indices, the length of the list, etc. You just express what it is you want to return: a tuple that contains the largest and the smallest elements.


Displaying member information from a CSV file The code is about displaying membership information of all clients from database.  I was just wondering what I could do to improve/change it.

def optionA():
    global DoAnother
    while DoAnother == True:
        print("")
        member = input("Please enter the member number: ")
        if len(member) !=4:
            print ("")
            print("The member number is not valid")
            DoAnother = True
        else:
            global memberfile
            memberfile = open("members.txt","r+")
            searchMem = True
            for line in memberfile.readlines():
                listed = line.split(",")
                if member in listed:
                    print("")
                    print("Team Code: {0}".format(listed[0]))
                    print("Member number: {0}".format(listed[2]))
                    print("Date of joining: {0}".format(listed[1]))
                    print("Membership type: {0}".format(listed[3]))
                    print("Amount paid: {0}".format(listed[5]))
                    searchMem = False
                    nextchoice()
            if searchMem == True:
                optionA()

def optionB():
    print("")
    print("Team Code  Member No           Fee      Amount Paid      Amount Outstanding")
    print("----------------------------------------------------------------------------------------")
    global memberfile
    memberfile=open("members.txt","r+")
    amountOut = 0
    for line in memberfile.readlines():
        listed = line.split(",")

        if listed[3] =="F" and listed[5].strip('\n')<listed[4]:
            difference = (int(listed[4])-int(listed[5]))
            amountOut = (amountOut + difference)
            print("{0}          {1}             £{2}            £{3}          £{4}".format(listed[0], listed[2], listed[4], listed[5].strip('\n'), difference))
    print("")
    print("                                                         Total Outstanding: £{0}".format(amountOut))
    nextchoice()

There are a number of issues with this code; the first two items below are seriously bad habits.

Inappropriate use of functions.  Functions are not goto labels.  Having optionA call itself or nextchoice() makes no sense.  This must be corrected — it is a fundamental misunderstanding of how to write programs.
Use of global variables.  If you design your functions properly, there should be no need for any global variables.
Lack of abstraction.  There is nothing in common between these two functions, other than the fact that they both read from members.txt.  It would be a good idea to have a function to read, interpret, search the file.  Furthermore, since the data are in a csv format, you should use the Python csv module.
Using string comparison for numbers. This is a bug:

listed[5].strip('\n')<listed[4]

… because you are comparing numbers ASCIIbetically.
Lack of documentation. optionA and optionB are both mysterious names for functions.  It's not immediately clear what the members.txt file looks like, or why optionB ignores rows whose membership type column is not F.
File descriptor leak.  If you call open() without a corresponding call to close(), you might leak file descriptors.  In Python, you almost always want to call open() using a with block.
Member search by all attributes. You ask the user to enter a member number.  However, when you do

listed = line.split(",")
if member in listed:
    …

… you are actually searching all fields, not just the third column.
Python style guide.  PEP 8, the official Python style guide, says that variable names should be lower_case_with_underscores unless you have a good reason to deviate from the norm.

Suggested solution

import csv
from decimal import Decimal

MEMBERS_TXT_FIELDS = [
    ('Team Code', str),
    ('Date of joining', str),
    ('Member number', str),
    ('Membership type', str),
    ('Fee', Decimal),
    ('Amount paid', Decimal),
]

def members(member_number=None):
    with open('members.txt') as member_file:
        csv_file = csv.reader(member_file)
        for row in csv_file:
            member = {
                field_name: conv(val)
                for (field_name, conv), val in zip(MEMBERS_TXT_FIELDS, row)
            }
            if member_number is None or member_number == member['Member number']:
                yield member

def show_single_member(member_number):
    for member in members(member_number):
        print("""
Team Code: {Team Code}
Member number: {Member number}
Date of joining: {Date of joining}
Membership type: {Membership type}
Amount paid: {Amount paid}""".format(**member))
        return True
    return False

def show_owing_members():
    def value_fmt(field_def, member):
        field_name, width = field_def
        value = member[field_name]
        if isinstance(value, Decimal):
            return "£{0}".format(value.quantize(Decimal('0.01'))).rjust(width - 1) + ' '
        else:
            return str(member[field_name]).ljust(width)

    TABLE_FIELDS = [
        # Field name, width
        ('Team Code', 11),
        ('Member number', 20),
        ('Fee', 9),
        ('Amount paid', 17),
        ('Amount outstanding', 31),
    ]

    # Print headings
    print(''.join(field.ljust(width) for field, width in TABLE_FIELDS))
    print(''.join('-' * width for field, width in TABLE_FIELDS))

    total_outstanding = Decimal('0.00')
    for member in members():
        if member['Membership type'] == 'F' and member['Amount paid'] < member['Fee']:
            member['Amount outstanding'] = member['Fee'] - member['Amount paid']
            print(''.join(value_fmt(field_def, member) for field_def in TABLE_FIELDS))
            total_outstanding += member['Amount outstanding']
    print()
    w = sum(width for field, width in TABLE_FIELDS if field != 'Amount outstanding')
    print("Total outstanding: ".rjust(w) + "£{0}".format(total_outstanding))

def menu():
    while True:
        print("A: Member lookup")
        print("B: Outstanding fees")
        print("Q: Quit")
        choice = input("Choice: ").upper()
        if choice == 'A':
            ok = show_single_member(input("Please enter the member number: "))
            if not ok:
                print("No such member")
        elif choice == 'B':
            show_owing_members()
        elif choice == 'Q':
            break
        print()


Converting decimal to octal This is a simple program converting user input decimal numbers into octal ones.

#include <iostream>
using namespace std;
main()
{
    int de,oc,y,i=1,octal;
    float decimal,deci,x;
    cout<<"Enter decimal no :: ";
    cin>>decimal;
    de=decimal;
    deci=decimal-de;
    cout<<"("<<decimal<<")10 = (";
    while(de>0)
    {
        oc=de%8;
        de=de/8;
        octal=octal+(oc*i);
        i=i*10;
    }cout<<octal<<".";
    while(deci>0)
    {
        x=deci*8;
        y=x;
        deci=x-y;
        cout<<y;
    }
    cout<<")8";
}

Just, i don't understand why?

What's wrong with (for integer):

#include <iostream>
int main() {
    int input;
    std::cout << "Enter decimal number: ";
    std::cin >> std::dec >> input;
    std::cout << '\n' << std::oct << input << '\n';
}

And about your code:

Avoid using namespace std, it's a bad practice
Just main() isn't a valid prototype for the entry point, at least use `int main()
Define variables in the closest scope possible.
Try naming variable consistently
de=de/8; can be simplified to de /= 8;
floating point conversion is a bit more tricky than just doing what you do

Edit: After testing it (and correcting main() and) : Your code just doesn't work, even for an integer value

Enter decimal no :: (0)10 = (32767.)8
Enter decimal no :: (8)10 = (32776.)8
Enter decimal no :: (10)10 = (32779.)8

Test yourself


Select a location on the map depending on which button is clicked I have code like below. By default the location is myloc. When user clicks btn-a it selects myloc and when btn-b is clicked it select myloc1. In my code I repeat the code L.circle three times, which is bad practice. So can anybody help me? How can I optimize this code, DRY it up and improve the quality as well?

var myloc = new L.LatLng(13.7433242, 100.5421583);
var myloc1 = new L.LatLng(14.979900, 102.097771);
$(function () {
    var circle;
    var slider = document.getElementById('myRange');
    var output = document.getElementById('demo');
    output.innerHTML = slider.value + scale;

    slider.oninput = function (val) {
        output.innerHTML = this.value + scale;
        circle.setRadius(this.value);
    }

    circle = L.circle(myloc, {
        color: '#7a7777',
        weight: 0.1,
        fillColor: '#7a7777',
        fillOpacity: 0.2,
        radius: 0
    }).addTo(map);

    $('.btn-a').on('click', function(e){
        if ($(this).val() == 'First') {
            circle = L.circle(myloc, {
                color: '#7a7777',
                weight: 0.1,
                fillColor: '#7a7777',
                fillOpacity: 0.2,
                radius: 0
            }).addTo(map);

        } else if($(this).val() == 'Second') {
            circle = L.circle(myloc1, {
                color: '#7a7777',
                weight: 0.1,
                fillColor: '#7a7777',
                fillOpacity: 0.2,
                radius: 0
            }).addTo(map);
        }
    });
});

To DRY up your code, you need to use a variable to call the same code with different input.s It seems like if you had something like this:

const circles = {
   First: myloc,
   Second: myloc1
}

Then you'd be able to dry up the click code:

L.circle(circles[$(this).val()], {
            color: '#7a7777',
            weight: 0.1,
            fillColor: '#7a7777',
            fillOpacity: 0.2,
            radius: 0
        }).addTo(map)


CHIP-8 delay timer in C++ I'm writing a CHIP-8 emulator in C++, which has two timers. If these timers are non-zero, they are decremented by 1 at 60Hz. I'm still getting the hang of C++, so I wanted to see if I'm doing this the Right Way.

Here's the header file:

#include <chrono>

struct TimedRegister
{
  unsigned char value;
  std::chrono::time_point<std::chrono::high_resolution_clock> last_write;

  TimedRegister();
  void set(const unsigned char new_value);
  void decrement();
};

And this is the the implementation:

#include "chip8.hpp"

#include <chrono>

TimedRegister::TimedRegister()
  : value(0)
{}

void
TimedRegister::set(const unsigned char new_value)
{
  value = new_value;
  last_write = std::chrono::high_resolution_clock::now();
}

void
TimedRegister::decrement()
{
  if (value == 0)
    return;

  const auto now = std::chrono::high_resolution_clock::now();
  if (std::chrono::duration_cast<std::chrono::milliseconds>(now - last_write)
        .count() >= 1000 / 60) {
    --value;
    last_write = now;
  }
}

The decrement function is called from the main emulator loop like this:

while (!chip8.halted) {
  chip8.delay_timer.decrement();
}

They seem to work fine, but I'm not sure if I'm getting away with using a heavy-handed, brute-force approach.

When doing emulation you should disconnect from the real world and only provide a abstract view to a world.

In other words don't use a real time clock instead simulate the timer by linking it to the emulated clockspeed. For example if you pick 8 MHz you would have 133 333 instructions executed per decrement of each timer.

This will allow you to pause and fast forward when debugging programs.


How to make below code more readable and efficient? Code below reads static variables from ProductConfiguration class. Functions setAccess and setMarketAccess in two classes ProductInitialization and ProductMarketInitialization reads variables from ProductConfiguration. Can this reading be done in a better way? I still want to have file ProductConfiguration to hold public variables. Was just wondering if there is a better way of reading this variables in the other two classes, may be there in future there will be additional classes that will read variables from ProductConfiguration

public class ProductConfiguration{

      public static final PRODUCT_ACCESS=//this value is read from properties file

      public static final MARKET_PRODUCT_ACCESS=//this value is read from properties file

}

public class ProductInitialization{
    public Product setAccess(Product p){
         if(p.brand == "LOCAL") {
             p.access=ProductConfiguration.PRODUCT_ACCESS;
             return p;
          }
         p.access = "DEFAULT";
        return p;
    }
}

public class ProductMarketInitialization{
       public void setMarketAccess(Product p){
            if(p.type == "MARKET"){
                p.access=
              ProductConfiguration.MARKET_PRODUCT_ACCESS;
              return p;
            }
           p.access ="MARKET_DEFAULT";
          return p;
      }
}

I cannot imagine the code being that reduced; you probably removed extraneous code.
The code shown should be clearer as:

public class ProductInitialization {
    public void setAccess(Product p){
        p.access = p.brand.equals("LOCAL") ? ProductConfiguration.PRODUCT_ACCESS
             : "DEFAULT";
    }
}

public class ProductMarketInitialization {
    public void setMarketAccess(Product p){
        p.access = p.type.equals("MARKET") ? ProductConfiguration.MARKET_PRODUCT_ACCESS
                   : "MARKET_DEFAULT";
     }
}

The ProductConfiguration is taken from a properties file. This PropertiesResourceBundle is principally partly lazy. A ListResourceBundle would be totally immediate, as this is a java class containing arrays. If on the other hand you would like the startup as smoothly as possible, a totally lazy loading would need getters for the ProductConfiguration.

This seems still not very important, so be it.

The special cases of a brand LOCAL and type MARKET, and finally a special result DEFAULT and MARKET_DEFAULT hinting at yet another case handling are more cumbersome.

Old      New

LOCAL    ${productConf.productAccess}
other    Optional.empty()

MARKET   ${productConf.marketProductAccess}
other    Optional.empty()

So making the access field an Optional<...>, maybe even an optional getter.

Having two initialisation classes seems overdesigned, especially as they create two distinct categories of the same Product.

In general it seems too early to pay too much attention to adding extra code for nicety's sake.

    public void configAccessForDefault(Product p){
        p.access = p.brand.equals("LOCAL") ? ProductConfiguration.PRODUCT_ACCESS
             : "DEFAULT";
    }

    public void configAccessForMarket(Product p){
           p.access = p.type.equals("MARKET") ? ProductConfiguration.MARKET_PRODUCT_ACCESS
                   : "MARKET_DEFAULT";
     }


C program for whitespace normalization I have made a beginner program using C that uses file handling and formats the given file. Formatting includes removing trailing blank spaces, replacing one or more blanks by a single blank and also wrapping lines.
It would be extremely helpful if you could point out the mistakes I have made and also if  I add any other features to file formatting.

MAIN.C

#include <stdio.h>
#include <stdlib.h>
#include "whiteSpacing.c"
#include "lineFolding.c"

FILE *p;
FILE *q;
char c;

int main(int argc, char *argv[])
{
    if(argc != 2)
    {
        printf("Error! Check number of arguements.\n");
        exit(-1);
    }

    p = fopen(argv[1], "r");                //original file containing input
    q = fopen("temporary.txt", "w");

    if(p == NULL || q == NULL)          //error checking, also asks user if file is to be created.
    {
        printf("ERROR! Unable to open file\n");
        printf("Do you want to create '%s' file ? (y/n)\n", argv[1]);
        c = getchar();

        if(c == 'y' || c == 'Y')
        {
            p = fopen(argv[1], "w+");
            printf("Type here to write to the file\n");

            while((c = getchar()) != EOF)
            {
                putc(c, p);
            }
            fclose(p);

            p = fopen(argv[1], "r");
        }

        else
            exit(1);
    }

    while((c = getc(p)) != EOF)   // for line wrapping
    {
        folding();
    }

    fclose(p);
    fclose(q);

    p = fopen("temporary.txt", "r");  // original file will be overwritten
    q = fopen(argv[1], "w");

    if(p == NULL || q == NULL)
    {
        printf("ERROR! Unable to open file\n");
        exit(1);
    }

    while((c = getc(p)) != EOF)         //removes trailing spaces/tabs, also replaces two or more white space by a single space.
    {
        whiteSpacing();
    }

    remove("temporary.txt");

    printf("Text in \"%s\" was formatted and overwritten.\n", argv[1]);
    fclose(p);
    fclose(q);

    return 0;
}

linefolding.c

#define MAXLEN 65       //length for wrapping
#define MAXLINE 1000    // maximum length for one line

extern FILE *p;
extern FILE *q;
extern char c;

int getLine(char *);
void trim(char *);

void folding(void)
{
    char line[MAXLINE];
    int len;

    while((len = getLine(line)) > 1) // proceed only if length of line is greater than 1
    {
        if(len > MAXLEN)    // fold the length only if length of line is greater than the MAXLEN.
        {
            trim(line);
        }
        fprintf(q, "%s", line);
    }

}

int getLine(char line[])
{
    int nc = 0;
    while(((c != EOF)) && c != '\n')  // store each character of a line in an array.
    {
        line[nc] = c;
        nc++;
        c = getc(p);
    }
    if(c == '\n')
        line[nc] = '\n';

    line[nc + 1] = '\0';

    return nc;
}

void trim(char line[])
{
    int i = 0;
    int lastBlank = 0;
    int j = 1;

    for(i = 0; line[i] != '\0'; i++)
    {
        if(line[i] == ' ' || line[i] == '\t' || line[i] == '\n' || line[i] == ',' || line[i] == '.')   // kees track of the last white Spacing or non alphabetical character .
        {
            lastBlank = i;
        }

        if(i == MAXLEN * j)    //prints a new line at every MAXLEN interval
        {
            line[lastBlank] = '\n';
            j++;
        }
    }

    line[i] = '\n';
    line[i + 1] = '\0';
}

whiteSpacing.c

#include <stdio.h>

extern FILE *p;
extern FILE *q;
extern char c;

void whiteSpacing()
{
    while(c == ' ')   //replaces two or more spaces by a single space
    {
        c = getc(p);
        if (c != ' ' && c != EOF)
            putc(' ', q);

    }
    putc(c, q);
}

I see a number of things that could help you improve your code.

Don't #include .c files

The code has these two lines

#include "whiteSpacing.c"
#include "lineFolding.c"

Generally speaking, we divide code into .h files which define an interface and .c files which contain an implementation.  For example, a lineFolding.h file might look like this:

#ifndef LINEFOLDING_H
#define LINEFOLDING_H
#include <stdio.h>
void folding(FILE *p, FILE *q, char c);
#endif // LINEFOLDING_H

Although there are other functions within the lineFolding.c file, they are only used internally to that module and so they aren't listed in the interface.  Also note that the include of <stdio.h> is required because the interface includes two FILE * parameters.  It also incorporates the next suggestion.

Eliminate global variables where practical

Having routines dependent on global variables makes it that much more difficult to understand the logic and introduces many opportunities for error.  Eliminating global variables where practical is always a good idea, whether programming for desktop machines or for embedded systems.  In this particular case p, q and c should be passed to the other functions (if needed) and local to main.

Use better naming

Function names like getLine are good because the names suggest what they're doing.  Names like folding are not as good because the name is very generic and doesn't describe very well what the function is doing.  Names like p and q are not good at all because they don't tell the reader anything at all about what the variable means in the context of the program.  I'd rename them infile and outfile and then the program is much easier to read and understand.

Fix the bug

In the getLine function, what happens if line doesn't happen to contain a '\n' character?  The answer is that the program continues to march through memory well beyond the allocated size of the buffer.  This is an error and it must be fixed.  Fortunately, it's not that hard to do.  One way to do that is to add it as a condition for the while loop:

while(((c != EOF)) && c != '\n' && nc < MAXLINE-2)

Consider the user

If the user of this program does not supply an input filename, the program prints this unhelpful message:

  Error! Check number of arguements.

I'd suggest that something a little more instructive (and without the spelling error) might be better.  Consider instead using something like this:

printf("Usage: %s filename\n", argv[0]);

Also, instead of having a hardcoded filename temporary.txt and doing the formatting in several steps, it might be nice to allow the user to control the name and location of a sepaate output file.  For this, it would make sense to use another command line argument.  Also, while having the user type the file if the file doesn't exist might be handy, I'd suggest that doing just one thing well is a better idea.  In particular, I'd write this using only stdin and stdout so that the user could call it with redirected input and output if they care to deal with files, or without if they'd like to type directly.

Reconsider the function interfaces

Consider the whiteSpacing function.  Its basic function is to replace double spaces with a single space.  However, it's called after the file has already been adjusted to introduce line wrapping.  For this reason, I'd suggest that a better way to do this would be to pass a buffered line to the function instead.  This would allow the program to do a single pass through the file instead of requiring two.

Be careful with function sequencing

The last lines of the code include this:

remove("temporary.txt");
printf("Text in \"%s\" was formatted and overwritten.\n", argv[1]);
fclose(p);

It will probably work, but it contains a flaw in that the file p is removed and then the file is closed.  I'd suggest closing it first.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments:  "I didn't know that." or "That's bad advice!"  My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard.  For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

  [...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

  If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea.  We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function.  Reasons against omitting seem to boil down to "it looks weird".  If, like me, you're curious about the rationale for the change to the C standard read this question.  Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!)  In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.


Is there a more efficient way to do Base 32 encoding/decoding than this? This journey started with a quest to create the shortest possible case-insensitive verification number. Based on the answers there, I worked out a Classic ASP/VBScript implementation of Crockford's Base32 Encoding and it has served me well so far.

Out of semi-idle curiosity, I was wondering if there was a way to further improve the performance. To be clear: it works, and it runs more than fast enough. I am planning to use it more extensively and since I never heard back on a better way, I was hoping for a second set of eyes to say "yes, it's as good as it's going to get" or "here's how to make it more awesome" or whatever.

So you don't have to go back to the latter Stack Overflow question to see the code, here it is:

'Base32 encoding functions for shorter, less confusing verification numbers
Const kBase32Digits = "0123456789abcdefghjkmnpqrstvwxyz"

Function ToBase32(ByVal lInput)
    Dim lModulo, sTemp
    Do Until lInput = 0
        lModulo = lInput Mod 32
        sTemp = Mid(kBase32Digits, lModulo + 1, 1) & sTemp
        lInput = lInput \ 32
    Loop
    ToBase32 = sTemp
End Function

Function FromBase32(ByVal sInput)
    Dim sTemp, sR, i,iY,lLen, zMultiplier
    sTemp = LCase(sInput)
    sTemp = Replace(sTemp,"o","0")
    sTemp = Replace(sTemp,"i","1")
    sTemp = Replace(sTemp,"l","1")
    sTemp = Replace(sTemp,"u","v")
    zMultiplier = 1
    lLen = Len(sTemp)
    For i = lLen To 1 Step -1
        sR = Mid(sTemp, i, 1)
        iY = InStr(1, kBase32Digits, sR, vbTextCompare) - 1
        FromBase32 = FromBase32 + iY * zMultiplier
        zMultiplier = zMultiplier * 32
    Next
End Function

If you really want to chase those milliseconds then I would try following:

1) I consider division operations as slow so I would avoid using two divisions if it can be replaced with one division and some easier operation:

Function ToBase32(ByVal lInput)
    Dim lModulo, sTemp, lNextInput
    Do Until lInput = 0
        lNextInput = lInput \ 32
        lModulo = lInput - 32 * lNextInput
        sTemp = Mid(kBase32Digits, lModulo + 1, 1) & sTemp
        lInput = lNextInput
    Loop
    ToBase32 = sTemp
End Function

2) In some other language I would use bit shift of course instead of diviosion/multiply. But as far as I got vbscript doesn't have bit shifting operators.

3) Your FromBase32 method doesn't look optimal to me at all. I would replace those Replace and InStr calls with array of integers for letter chars. This array will contains corresponding base 32 numbers for chars. And you will be able to translate letter into number using array indexing accessor which is much faster operation than looking for a character in a string. Some pseudocode showing how to convert character sR into number:

Dim sR;

// defining an array:
Dim charArray = [  10, // 'a' char
                   11, // 'b' char
                   ...
                   17, // 'h'
                   1 , // 'i' !!
                   ...
                   0 , // 'o'
                   ...];

Dim lNumber;
if (sR >= 'a' && sR <= 'z')
    lNumber = charArray[sR - 'a'];
else // it is digit
    lNumber = sR - '0'  // in most languages I know this will convert '1' char into 1 integer

P.S. of course I haven't measured anything, I do not even know vbscript, just wanted to give you an idea on what can be changed.

P.P.S.: My second point doesn't take into account that there is a chance that VBScript interpreter will do such an optimization for you.