Display HTML based on user input (via checkbox selection) I currently have this, which works. However, I would prefer to not have the HTML that displays being stored inline in array like this, but in a more flexible / optimal way so I can have more text / HTML without being restricted to having it inline within the quotes.

texts = {
      item1: 'Item Box 1 Content html right here! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.',
      item2: 'Now its Item Box 2 html content here ! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.',
      item3: 'This is the example html of Item box 4! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.',
      item4: 'Item box number 5 html content is here! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.',
    }
    $("#container").css('background', '#fff')

     $('.download-pdf').click(function() {

      notChecked = $("input[type=checkbox]:not(:checked)").parent();
      notChecked.hide();
      yesChecked = $("input[type=checkbox]:checked").parent();

      $.each(yesChecked, function( index, el ) {
        $(el).show().html(texts[$(el).attr('id')]);
      });

full js code for better clarity.

texts = {
      item1: 'Item Box 1 Content html right here! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.',
      item2: 'Now its Item Box 2 html content here ! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.',
      item3: 'This is the example html of Item box 4! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.',
      item4: 'Item box number 5 html content is here! Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.',
    }
    $("#container").css('background', '#fff')

     $('.download-pdf').click(function() {

      notChecked = $("input[type=checkbox]:not(:checked)").parent();
      notChecked.hide();
      yesChecked = $("input[type=checkbox]:checked").parent();

      $.each(yesChecked, function( index, el ) {
        $(el).show().html(texts[$(el).attr('id')]);
      });

     var pdf = new jsPDF('p', 'pt', 'a4');

      pdf.addHTML(document.getElementById('records'), function() {

          setTimeout(function(){
           location.reload();
           },3000);

      });

      var file = 'test';
      if (typeof doc !== 'undefined') {
          doc.save(file + '.pdf');
      } else if (typeof pdf !== 'undefined') {
          setTimeout(function() {
              pdf.save(file + '.pdf');
              // $("#item4").hide();

          }, 2000);
      } else {
          alert('Error 0xE001BADF');
      }

  });

You should execute most if not all your code inside the callback for the download click handler. Your code gives you the illusion that it's working because you're setting timeouts. If the user sits idle for longer than the timeouts your script will break.

Consider adding listeners to your checkboxes that bind to a model in your application. This will reduce the amount of dom parsing you have to do manually.

You also have some global variables defined in your script which might interfere with other scripts you've included in your page.

I...didn't test any of this though.

var checkboxPDF = (function($model, $config, $err, $data, $view) {

    //keeping the pdf config options here will let you determine how you want the pdf to look case by case everytime you use this module. It's how jquery plugins work anyway.
    return function(options) {
        $view.setBg();
        //uses a closure to keep track of the model outside the callback context
        $view.bindToModel($model);
        $view.readyDownload(function() {
            //setting up the pdf is the meat of your business logic
            var pdf = new jsPDF(options.orientation, options.pt, options.size);
            $view.update($model, $data);
            //I'm not entirely sure what is in your records div
            $view.addHtml(pdf);
            //In your code I don't know where the doc variable comes from
            if (doc)
                doc.save(pdf + $config.filename);
            else if (pdf)
                pdf.save(pdf + $config.filename);
            else
                alert($err['not defined']);
        });
    };

})(

//keep your model separate from your controller
{},

//pdf configuration, it can be here because it has nothing to do with your application logic
{ filename: 'test' extension: '.pdf' },

//you can keep your error configuration separate as well
{ 'not defined': 'Error 0xE001BADF' },

//static data here, or you could put this directly into the html and toggle classes between visible/hidden in your view logic.
{
    '#idA': 'item 1 box content: ...',
    '#idB': 'item 2 box content...',
    '#idC': 'item 3 box content...'
    '#idD': 'etc...'
},

//organized methods that are concerned with the view/model
{
    setBg: function() {
        $("#container").css('background', '#fff')
    },
    addHtml: function(pdf, callback) {
        pdf.addHTML(document.getElementById('records'), callback || null);
    },
    bindToModel: function(model) {
        $("input[type=checkbox]").on("click", function(e) {
            model[this.id] = e.target.value;
        });
    },
    readyDownload: function(callback) {
        $('.download-pdf').click(function() {
            callback();
        });
    },
    update: function(model, data) {
        for (var p in model) {
            if (model[p] === true) {
                $(p).show().html(data[p]);
            }
        }
    }
);

checkboxPDF({orientation: 'p', pt: 'pt', size: 'a4'}); //appends your listeners, waits for user to check boxes and downloads pdf once they click on a download handler


Ensure multiple steps work/saving multiple models via a service I just thought of something new re: saving multiple objects in Rails (though Rails is not necessary).

Objectives:

Create an author and a tomato, but both of those must be valid.
If invalid creation, then return an error object.
Not raise an ActiveRecord if I can (slow). (Is this something that I am even supposed to think about or am I overkilling this?)
Make it easy to add steps/add steps in front or in the back.

That is just something I thought of. I just made creating authors and tomatoes methods that are sequentially called and checked if they are valid? or not.

I'm not sure what design pattern this is because the alternative is multiple if statements. If there is a formal gem or better way then I'd appreciate suggestions:

class AuthorAndTomatoSave
  def call
    steps.map do |step|
      result = step.call

      # If error, return
      return result unless result.valid?
    end

    # hypothetical case where we want to return the author.
    # (this is also why create_author is memoized)
    create_author
  end

  private

  def steps
    [ proc { create_author }, proc { create_tomato } ]
  end

  def create_author
    @created_author ||= Author.create(author_params)
  end

  def create_tomato
    Tomoto.create(tomato_params)
  end
end

A couple of issues I see here:

What happens if an Author is saved, and then Tomato creation fails? Should the author record remain in the database? (In most cases the answer would be no)
How does a caller differentiate between a successful call and a failure to save an Author? In both cases, call returns an Author object. Would they have to always call author.valid? to check?

  Not raise an ActiveRecord if I can (slow). (Is this something that I am even supposed to think about or am I overkilling this?)

This seems like a micro-optimization to me, I'd use whatever makes code easier to read.

The generic pattern to handle such cases is to use a transaction. Transactions aren't specific to Rails, they are a feature that most databases offer. ActiveRecord supports transactions out of the box.

In your case, this would be reduced to

def create_author_and_tomato!
  ActiveRecord::Transaction.do
    Tomato.create!(tomato_params)
    Author.create!(author_params)
  end
end

If any of those steps fail, the transaction will be rolled back (i.e. all databases changes reversed) before an exception is raised. Also, the Author object is returned if everything succeeds.

If you need to perform a different action if saving one record fails, catch the ActiveRecord::RecordInvalid exception.

begin
  author = create_author_and_tomato!

  # Use `author` for whatever
rescue ActiveRecord::RecordInvalid => e
  # `e.record` can be used to access the invalid object
end


Range class to represent limited values, like HP, Stamina and so on I made a small script for Unity3D, meant to be use as GameComponent for GameObjects. It is supposed to represent values that have a max value, and an actual value. In games that would be the case, for instance, for the Health Points, the Stamina, Magic Points and so on. I would love to hear your feedback.

/////////////
//
// This is meant for things like Stamina, or HP. You have a max
// value, and an actual value.
//
// It inherits from MonoBehaviour because it is meant to be used
// as a Game Object Component
//
/////////////

using UnityEngine;
using UnityEngine.UI;

public class Range : MonoBehaviour {

    private int val, max;

    public Slider slider;

    // Is/Has/Can

    public bool IsDepleted() { return val == 0;    }
    public bool IsFull()     { return val == max; }

    public bool HasAtLeast(int _val) { return this.val >= _val; }

    // Actions

    public void Fill()    { val = max; }
    public void Deplete() { val = 0;   }

    public void Increase(int _val) { this.SetValue(this.val + _val); }
    public void Decrease(int _val) { this.SetValue(this.val - _val); }

    // Getters

    public int GetValue() { return this.val; }
    public int GetMax()   { return this.max; }

    // Setters

    public void SetValue(int amount) {
        if (amount > this.max) this.Fill();
        else if (amount < 0) this.Deplete();
        else this.val = amount;

        UpdateSlider();
    }

    public void SetMax(int amount) {
        this.max = amount;
        if (this.val > this.max) this.Fill();

        UpdateSlider();
    }

    private void UpdateSlider() {
        if (slider) {
            slider.maxValue = this.max;
            slider.value = val;
        }
    }
}

Problematic
Fill and Deplete are public, but don't call UpdateSlider, whereas Increment/Decrement do.

SetMax should reject values < 0 (0 could be allowed, for value forced to 0)

Design Style
There is a function in the Unity Mathf static class that does bounds checking Mathf.Clamp

C# has properties, while you can have the pair GetX/SetX they are preferred.

public int Value {
    get { return val; }
    set { val = Mathf.Clamp( value, 0, Max );
        UpdateSlider(); } }
public int Max {
    get { return max; }
    set { if ( value < 0 ) throw new ArgumentOutOfRangeException ("Max");
        max = value; } }

Consider
Increment and Decrement could be replaced by overriding + and -, as += and -= will use such overloads.


Fake utcnow for the pytest I want to create a pytest with a fake utcnow, but also I need to preserve the functionality of all other datetime methods. Simple example here:

import datetime as dt

class FakeTime(dt.datetime):
    fake_time = None

    @classmethod
    def utcnow(cls):
        return cls.fake_time

def str_2_time(str_dt: str) -> dt.datetime:
    """Shortcut to do convert the string to datetime"""
    return dt.datetime.strptime(str_dt, '%Y-%m-%d %H:%M')

def test_patch_datetime():
    for utc_time in ['2019-01-01 10:00', '2019-02-01 13:00', '2019-03-01 16:00']:
        FakeTime.fake_time = str_2_time(utc_time)
        dt.datetime = FakeTime
        assert dt.datetime.utcnow() == str_2_time(utc_time)

Is this the right way?

The method str_2_time just need to show that all other methods of the datetime works fine.

According to this, subclassing datetime.datetime seems the way to go.

There is no use for the str_2_time method though. You can easily inline this, or even simpler, just use the datetime.datetime constructor:

def test_patch_datetime():
    for utc_time in [
        dt.datetime(2019, 1, 1, 10),
        dt.datetime(2019, 2, 1, 13),
        dt.datetime(2019, 3, 1, 16),
    ]:
        FakeTime.fake_time = utc_time
        dt.datetime = FakeTime
        assert dt.datetime.utcnow() == utc_time

You should be aware that this can have side effects in other parts of your code, so it might be needed to replace it back with the original class after  the test method:

def test_patch_datetime():
    datetime_orig = dt.datetime

    utc_times = [
        dt.datetime(2019, 1, 1, 10),
        dt.datetime(2019, 2, 1, 13),
        dt.datetime(2019, 3, 1, 16),
    ]
    for utc_time in utc_times:
        FakeTime.fake_time = utc_time
        dt.datetime = FakeTime
        assert dt.datetime.utcnow() == utc_time
    dt.datetime = datetime_orig
    # print(dt.datetime.utcnow())
    assert dt.datetime.utcnow() > max(utc_times)


node.js Syslog server recording to MS-SQL database This 43 line program is designed to listen for Syslog messages on port 514 and record the messages to an MS SQL Server database table.

I would appreciate feedback from experienced node.js developers.

I am just starting to learn node.js.  I'm average at JavaScript, still getting my head around closures.  I am really struggling to get my head around the node.js event driven programming paradigm.

If I try to access the SQL server connection outside of the sql.open callback function, I get the error that the database connection is closed.

So for this reason I had to place the receive message and save it in database logic inside the sql.open callback function, which seems messy and counter-intuitive, but that is probably due to my not quite fully understanding the event driven model?

Anyway the code works (amazingly!) and seems to be quite efficient, saving many dozens of syslog messages per second to the local MS SQL database with negligible CPU impact...

Are there any fundamental flaws in this code, and any changes that should be made to improve it?

I guess I am amazed that this can be done in just one page of code.  node.js is very cool...

var dgram = require("dgram");
var server = dgram.createSocket("udp4");
var sql = require('msnodesql');

var conn_str = "Driver={SQL Server Native Client 11.0}; Server=.; Database=SYSLOG; UID=xxxxxxx; PWD=xxxxxxxx;";

// open the database connection
sql.open(conn_str, function (err, conn) {
    if (err) {
        console.log("error", err);
        return;
    }
    //database connection succeeded, log a mesage into the syslog.incoming table
    conn.queryRaw("insert into Incoming(Source,Message) values('LOCAL','node.js syslog server started')");

    //create an event listener for when a syslog message is recieved
    server.on("message", function (msg, rinfo) {
      //sanitise the data by replacing single quotes with two single-quotes
      var message = msg.toString().replace(/'/g, "''")
      var src = rinfo.address.toString().replace(/'/g, "''")
      var s = "insert into Incoming(Source,Message) values('"+src+"','"+message+"')";

      //send the SQL to the database
      conn.queryRaw(s, function (err, results) {
        if (err) {
            console.log(s);
            console.log(err);
            return;
        }
      });
    }); // end of server.on("message") listener

    //create an event listener to tell us that the has successfully opened the syslog port and is listening for messages
    server.on("listening", function () {
      var address = server.address();
      console.log("server listening " + address.address + ":" + address.port);

    });

    //bind the server to port 514 (syslog)
    server.bind(514);

});// end of sql.open()

From a once over:

Comments should be worth the space they take up, my favourite offender is this one:

// open the database connection
sql.open(conn_str, function (err, conn) {

I want to point to is msnodesql maintained?
I also want to point out that storing uid/pwd in your script can be dangerous, I hope this is not a production database, which does not have sensitive data
As Nelson Menezes mentioned, you should look into using prepared statements
In this block, you should either use 1 comma separated var or semicolons:

var message = msg.toString().replace(/'/g, "''")
var src = rinfo.address.toString().replace(/'/g, "''")
var s = "insert into Incoming(Source,Message) values('"+src+"','"+message+"')";

In a script of this size I would consider merging some statements, this

console.log(s);
console.log(err);
return;

might as well be

return console.log(s, '\n', err);

});// end of sql.open() <- Overkill comments

Still, all in all a solid script.


Emacs module that generates templates for my blog's static site generator I wrote a simple emacs module that generates standard templates that I use for my blog's static site generator.

(defun hakyll-site-location ()
  "Return the location of the Hakyll files."
  "~/Sites/hblog/")

(defun hakyll-new-post (title tags)
  "Create a new Hakyll post for today with TITLE and TAGS."
  (interactive "sTitle: \nsTags: ")
  (let ((file-name (hakyll-post-title title)))
    (set-buffer (get-buffer-create file-name))
    (markdown-mode)
    (insert
     (format "---\ntitle: %s\ntags: %s\ndescription: \n---\n\n" title tags))
    (write-file
     (expand-file-name file-name (concat (hakyll-site-location) "posts")))
    (switch-to-buffer file-name)))

(defun hakyll-new-note (title)
  "Create a new Note with TITLE."
  (interactive "sTitle: ")
  (let ((file-name (hakyll-note-title title)))
    (set-buffer (get-buffer-create file-name))
    (markdown-mode)
    (insert (format "---\ntitle: %s\ndescription: \n---\n\n" title))
    (write-file
     (expand-file-name file-name (concat (hakyll-site-location) "notes")))
    (switch-to-buffer file-name)))

(defun hakyll-post-title (title)
  "Return a file name based on TITLE for the post."
  (concat
   (format-time-string "%Y-%m-%d")
   "-"
   (replace-regexp-in-string " " "-" (downcase title))
   ".markdown"))

(defun hakyll-note-title (title)
  "Return a file name based on TITLE for the note."
  (concat
   (replace-regexp-in-string " " "-" (downcase title))
   ".markdown"))

Now, this works, but it could do with DRYing up a little bit, but I don't know enough elisp to do it myself.

hakyll-new-post and hakyll-new-note are very similar and could do with DRYing up, but I'm not sure how to pass the correct parameter to any refactored function
I'm hard coding hakyll-site-location. Is there any way that I can request for and store a configuration in my emacs dotfiles?

Any help or pointers to documentation are welcome.

DRY-ing it up:

It looks like variation is in:

how the filename is obtained
the contents of the inserted text
the name of the file to write to.

Also note that if format is given extra arguments not needed by its format-spec that they are ignored.

So you could define a function which takes 3 arguments: the function to call to get the filename to read from, the file-name to write to, and a the format-spec string.

In order to pass a named function to a method you need to quote it as in the example:

(mapcar '1+ '(1 2 3))

If you want to pass an anonymous function to a function you defined it with #'(lambda ...). For example:

(mapcar #'(lambda (x) (1+ x)) '(1 2 3))

Making a configuration setting

I'd suggest changing your hakyll-site-location function into a variable. Use defvar to define it and then you can simply setq it in your emacs dotfiles.


Simple jQuery search on input element Just wanted a way of attaching a jQuery search plugin to an input element (or collection) and be able to pass specific options at invoke time:

This is the plugin code

(function($){

$.fn.extend({

  enableSearch:function(opts,callback){
    return this
    .bind('focus',function(){
      $(this.form.elements).val(''); // clears all search inputs
      $(this).trigger('keyup'); // triggers the search on an empty input element
    }) // END focus bind
    .bind('keyup',function(){
       var searchData = $.extend({},'source':this.form.id,'column':$(this).attr('id'),'value':$(this).val()},opts);
       $.ajax({
         url:opts.URL,
         type:'post',
         data:searchData,
         dataType:'json'
       })
       .promise().then(function(returndata){
         callback(returndata);
       }); // END ajax call
       return false;
    }); // END bind keyup
  } //END fn enableSearch

})

})(jQuery);

This is the call on the collection:

$('input') // collection on which to enable searching
  .enableSearch({
   URL:'<server side script url>',
   extra_param:extra_param_value, // extra param which gets passed in the data to the server side fn
  },function(data){
    //actions performed on the returned data, e.g. update a list
  });

I couldn't get my idea to work until I came across the jQuery promise implmentation. Opens many doors.

Works for me - any thoughts?

I find $.ajax().promise().then(fn) to be a little more confusing when looking at the code than it would have been if you instead used $.ajax().complete(fn) (or success if that is what you meant).
I would throttle the ajax post so that it doesn't happen on every keyup
if you are using the latest syntax already for $.ajax, I think it makes sense to use the .on method instead of .bind
some of the $() calls can be cached at various levels

So far the resulting code looks something like this (completely untested):

(function($, window){
    $.fn.extend({
        enableSearch:function(opts,callback){
            var timeout;
            var $t = this;
            return $t
                .on({
                    focus: function(){
                        $(this.form.elements).val(''); // clears all search inputs
                        $t.trigger('keyup'); // triggers the search on an empty input element
                    }, // END focus bind
                    keyup: function(){
                        var t = this;
                        if (timeout) { window.clearTimeout(timeout); }
                        timeout = window.setTimeout(function() {
                            var searchData = $.extend({}, {
                                source:t.form.id,
                                column:$t.attr('id'),
                                value:$t.val()
                            }, opts);

                            $.ajax({
                                url:opts.URL,
                                type:'post',
                                data:searchData,
                                dataType:'json'
                            }).complete(function(returndata){
                                callback(returndata);
                            }); // END ajax call
                        }, $.fn.enableSearch.throttle); //END setTimeout
                    } //END keyup
                }); //END on
        } //END fn enableSearch
    });
    $.fn.enableSearch.throttle = 150; //$.ajax will not happen until 150 ms delay happens between keystrokes
}(jQuery, window));

This code is still very specific to your particular case and isn't very reusable elsewhere. The only reasons I would even be making it into a plugin are testability and reuse. I suppose you may be able to reuse this on your application so that isn't much of a deal, but I feel like it requires too much knowledge of your specific implementation to be testable.


Tableview Subclass implementing delegate datasource I have two view controllers which have same tableview. One is for Post and one is for Profile, which also contains a post by a user. I have decided to reuse the code, so I have created the class PostTableView.

class PostTableView: UITableView {

    var arrayPosts:[PostTableViewCompitible] = [] {
        didSet {
            self.reloadData()
        }
    }
    var btnCommentTapped:((ArrayInspireListCell) -> ())?
    var heightOfRows = Dictionary<IndexPath, CGFloat>()
    var indexOfCellBeforeDragging = 0

    override func awakeFromNib() {
        super.awakeFromNib()
        setupTableView()
    }

    private func setupTableView() {
        self.register(UINib.init(nibName: StoryBoard.Cells.ArrayInspireListCell, bundle: nil), forCellReuseIdentifier: StoryBoard.Cells.ArrayInspireListCell)
        self.dataSource = self
        self.delegate = self
        self.estimatedRowHeight = 470
        //    self.tableView.decelerationRate = UIScrollViewDecelerationRateFast
        self.rowHeight = UITableViewAutomaticDimension
        self.separatorStyle = .none

    }

}

With Delegate / Data source:

extension PostTableView:UITableViewDataSource,UITableViewDelegate {

    func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
        return self.arrayPosts.count
    }

    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        let post = self.arrayPosts[indexPath.row]
        let cell = post.cellForTableView(tableview: tableView, atIndexPath: indexPath)
        cell.setCollectionViewDataSourceDelegate(dataSourceDelegate: self, forRow: indexPath.row)
        cell.layer.shouldRasterize = true
        cell.layer.rasterizationScale = UIScreen.main.scale

        cell.btnCommentTapped = self.btnCommentTapped

        cell.layoutIfNeeded()

        return cell
    }

    func tableView(_ tableView: UITableView, estimatedHeightForRowAt indexPath: IndexPath) -> CGFloat {
        if let height = self.heightOfRows[indexPath] {
            return height
        }
        return 470
    }

    func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) {
        let height = cell.frame.size.height
        self.heightOfRows[indexPath] = height + 30
    }

}

Is it good practice to do so?

Personally I don't like the way UITableViews work with a delegate and dataSource yet there's no real way to say "Here's the data". You are kind of forced to either send around copies of your data or put everything into one class. There's tons of mixing of the M, V and C layers in one class going on.

However, it should be possible to use a separate implementation of the dataSource and `delegate. While in this example there's little code, in some cases you might cross (my personal) soft limit of 200 lines of code or even the hard limit of 300 in one file.

In my Objective-C / Storyboards days I actually had implementations where I would use several objects all linked together via @IBOutlets. So a delegate, dataSource and tableView. The delegate could have a weak ref to the dataSource and the dataSource had the actual data. Not using the selected mechanism for button-like actions could allow you to sever the ties between the delegate and the data.

So I don't disapprove of what you're doing because you're trying to do your best with the bad hand of cards that Apple dealt you. But you could try to have the data only exist on the dataSource implementation. Also I see you doing a lot of stuff to the cell in dequeue, that should definitely move to the implementation of the cell because it's doing V code in an M class. I use a configure(with: *someData*) function that sets it up further.

If you try to separate the M from the V and the C you will see pretty clearly where you should cut it up.


Swing UI for a test Websocket client I am new to Java's Swing and built a test client that connects to a server. I was wondering if my use of a Controller class as a link between the classes ClientWindow and WebsocketService made sense and would be thankful for comments. (I can take it, so the more my code is criticized the happier I am.)

For the sake of brevity, I have omitted most methods and fields.

ClientWindow

public class ClientWindow implements ActionListener {
    private static final String USER_ID = "19";
    private final IController controller;

    @Override
    public void actionPerformed(ActionEvent e) {
        controller.connectAs(USER_ID);
    }

    public void addMessage(String line) {
        responseArea.setText( responseArea.getText() + line );
    }
}

DefaultController (implements IController)

public class DefaultController implements IController {

    protected WebsocketService wsService;
    protected ClientWindow display;

    public DefaultController() {
        wsService = new WebsocketService(this);
    }

    public void setDisplay(IClientDisplay display) {
        this.display = display;
    }

    // ------------------
    // IController methods
    @Override
    public void updateMessage(String message) {
        if (display != null) {
            display.addMessage(message);
        }
    }

    @Override
    public void connectAs(String userId) {
        wsService.connectToServer(userId);
    }
}

WebsocketService

@ClientEndpoint
public class WebsocketService {
    private final IController controller;
    private String userId;

    public WebsocketService(IController controller) {
        this.controller = controller;
    }

    /**
     * On open: register the session and signal
     * the user ID and to the Node.js server.
     * @param session The WebSocket session
     */
    @OnOpen
    public void onOpen(Session session) {
        this.session = session;
        controller.updateMessage("Connected to server");
        send( "c:" + userId);
    }

    /**
     * On message: handle incoming message by
     * invoking methods of the controller.
     * @param message
     */
    @OnMessage
    public void onMessage(String message) {
        controller.updateMessage(message);
    }

    /**
     * Open a WebSocket to connect to the Node.js server.
     */
    public void connectToServer(String userId) {
        this.userId = userId;
        WebSocketContainer container = ContainerProvider.getWebSocketContainer();
        try {
            container.connectToServer(this, new URI(Commons.SERVER_ADDR));
        }
        // catch blocks
    }
}

Finally, Client

public class Client {
    public static void main(String[] args) {
        DefaultController controller = new DefaultController();
        controller.setDisplay(new ClientWindow(controller));
    }
}

Client definitely looks bad, especially that second line. I might as well skip the DefaultController and just have a field for ClientWindow in WebsocketService and vice versa but that's obviously a big no-no. I have looked into MVC but I don't really have much to model (or am I wrong?).

After all, I need to be able to give commands to my Service but also to display output from it at any given time (onMessage) and so this was the most elegant solution I could think of.

I'm not too familiar with GUIs nor MVC, but I guess your model contains at least a string (which is displayed in the responseArea). You might put that into a separate model class.

Another thing is a websocket service which I try to hide behind the model and separate it from the controller class. Wikipedia says the following about MVC's model:

  A model notifies its associated views and controllers when there has been a change in its state.

So, the websocket service could notify the model, then the model would notify the view about the change.
I'd rename IController to Controller.

Clean Code by Robert C. Martin, Chapter 2: Meaningful Names, p24:

  Interfaces and Implementations

  These are sometimes a special case for encodings. For example, say you are building an
  ABSTRACT FACTORY for the creation of shapes. This factory will be an interface and will
  be implemented by a concrete class. What should you name them? IShapeFactory and
  ShapeFactory? I prefer to leave interfaces unadorned. The preceding I, so common in
  today’s legacy wads, is a distraction at best and too much information at worst. I don’t
  want my users knowing that I’m handing them an interface. I just want them to know that
  it’s a ShapeFactory. So if I must encode either the interface or the implementation, I choose
  the implementation. Calling it ShapeFactoryImp, or even the hideous CShapeFactory, is
  preferable to encoding the interface.

I think WebsocketService should not know anything about Controller.connectAs. Currently it violates the interface-segregation principle. You could create a WebsocketCallback interface which your controller implementation class could implement:

public interface WebsocketCallback {
    void updateMessage(String message);
}

public class DefaultController implements Controller, WebsocketCallback {
    ...
}

I guess implementing it by the model class could be even better (see #1).
I would rename the parameter here to message to express its purpose and make the code easier to understand:

public void updateMessage(String string) {

I would throw here an exception instead of hiding the error:

@Override
public void updateMessage(String message) {
    if (display != null) {
        display.addMessage(message);
    }
}

Crash early. It is rather a bug in the code when it is called without setting the display before which you shouldn't hide. It would help debugging: having a stacktrace usually better than just an one-line bugreport: "the button does not do anything" or something similar. See also: The Pragmatic Programmer: From Journeyman to Master by Andrew Hunt and David Thomas: Dead Programs Tell No Lies.
If responseArea if a JTextArea instance, you could use append instead of setText(getText() ...) here:

responseArea.setText( responseArea.getText() + line );


How can I make only one if-else statement among all of these? I have these if-else statements that I am not able to refactor.  I have used them for doing validations on server-side using asp.net.

Would anyone please suggest ways to reduce these statements?  Here, validation fields and validation types are enum lists.

else if (CheckNextItem(ddlBr1Type.SelectedValue.ToString(), ddlBr2Type.SelectedValue.ToString(), ValidationFields.FO.ToString(), ValidationTypes.P.ToString()))
{
   BrkTypeValidator2.ErrorMessage = "'TOP0618 -Invalid combination of Bracket IDs'";
   args.IsValid = false;
}
else if (CheckNextItem(ddlBr1Type.SelectedValue.ToString(), ddlBr2Type.SelectedValue.ToString(), ValidationFields.FW.ToString(), ValidationTypes.P.ToString()))
{
   BrkTypeValidator2.ErrorMessage = "'TOP0618 -Invalid combination of Bracket IDs'";
   args.IsValid = false;
}
else if (CheckNextItem(ddlBr1Type.SelectedValue.ToString(), ddlBr2Type.SelectedValue.ToString(), ValidationFields.UF.ToString(), ValidationTypes.P.ToString()))
{
     BrkTypeValidator2.ErrorMessage = "'TOP0618 -Invalid combination of Bracket IDs'";
     args.IsValid = false;
}
else if (CheckNextItem(ddlBr1Type.SelectedValue.ToString(), ddlBr2Type.SelectedValue.ToString(), ValidationTypes.O.ToString(), ValidationTypes.P.ToString()))
{
      BrkTypeValidator2.ErrorMessage = "'TOP0618 -Invalid combination of Bracket IDs'";
      args.IsValid = false;
}
else if (CheckNextItem(ddlBr1Type.SelectedValue.ToString(), ddlBr2Type.SelectedValue.ToString(), ValidationTypes.W.ToString(), ValidationTypes.P.ToString()))
{
      BrkTypeValidator2.ErrorMessage = "'TOP0618 -Invalid combination of Bracket IDs'";
      args.IsValid = false;

}
else if (CheckNextItem(ddlBr1Type.SelectedValue.ToString(), ddlBr2Type.SelectedValue.ToString(), ValidationTypes.P.ToString(), ValidationTypes.C.ToString()))
{
      BrkTypeValidator2.ErrorMessage = "'TOP0618 -Invalid combination of Bracket IDs'";
      args.IsValid = false;

}
else if (CheckNextItem(ddlBr1Type.SelectedValue.ToString(), ddlBr2Type.SelectedValue.ToString(), ValidationTypes.C.ToString(), ValidationTypes.U.ToString()))
{
      BrkTypeValidator2.ErrorMessage = "'TOP0618 -Invalid combination of Bracket IDs'";
      args.IsValid = false;

}

This is method for Checknextitem:

 public static bool CheckNextItem(string Compareitem1, string comnpareitem2, string items1, string items2)
  {

        var listContains = Compareitem1 == items1 && comnpareitem2 != items2;
        return listContains;

  }

If each ID1 can only have one valid corresponding ID2, then I would recommend using a  dictionary object to store the valid combinations, like this:

Dictionary<string, string> validIds = new Dictionary<string,string>();
validIds[ValidationFields.FO.ToString()] = ValidationTypes.P.ToString();
validIds[ValidationFields.FW.ToString()] = ValidationTypes.P.ToString();
validIds[ValidationFields.UF.ToString()] = ValidationTypes.P.ToString();
// ...

Then, you can check to see if the currently selected values are valid by simply comparing them to the valid combinations in the dictionary, like this:

if (validIds[ddlBr1Type.SelectedValue.ToString()] != ddlBr2Type.SelectedValue.ToString())
{
    BrkTypeValidator2.ErrorMessage = "'TOP0618 -Invalid combination of Bracket IDs'";
    args.IsValid = false;
}

By doing it that way, you only need the single if statement.  Another side benefit is that you can now store the list of valid combinations somewhere else, outside of your code, such as in a configuration file or a database.  Then you could load the data from that data source into the dictionary at run-time.


HackerRank Algorithm Challenge 1: Insertion sort This is my solution for HackerRank's first "Algorithm Challenge, Insertion Sort Part 1". The challenge is to sort the array from least to greatest, the input being an array in sorted order, except for the last entity, i.e. {2, 3, 4, 5, 6, 7, 8, 9, 1}

It works fine, but it's definitely not an optimal solution. Could anybody provide any insights on how I should look at it so as to optimize it? This is for learning purposes, so I can apply the same concepts to future code.

   static void insertionSort(int[] ar) {
          int i, j;
          int inserted = 0;
          i = ar[ar.length-1];

          for (j = ar.length-2; j > -1; j--){
              if (ar[j] > i){
                  ar[j+1] = ar[j];
                  printArray(ar);
              } else if (ar[j] <= i){
                  ar[j+1] = i;
                  inserted = 1;
                  break;
              }
          }

          if(inserted == 0){
              ar[0] = i;
          }

          printArray(ar);
   }

This is the rest of the code, but it's preset and is already available:

 static void printArray(int[] ar) {
     for(int n: ar){
        System.out.print(n+" ");
     }
       System.out.println("");
  }

  public static void main(String[] args) {
       Scanner in = new Scanner(System.in);
       int n = in.nextInt();
       int[] ar = new int[n];
       for(int i=0;i<n;i++){
          ar[i]=in.nextInt();
       }
       insertionSort(ar);
   }

Two components in particular are unnecessary. This test:

        } else if (ar[j] <= i){

is redundant. else means if (ar[j] <= i), since the latter is the negation of the earlier if (ar[j] > i).

And then, the inserted variable is used to track whether i was inserted within the loop. To simplify, I'd only do it outside the loop -- we can do this because j is still useful. (If you're not using j outside the loop, as in your original code, it's better -- safer, more readable -- style to declare it in the for statement.)

This is the heart of what you're doing:

static void insertionSort(int[] ar) {
    int i, j;
    i = ar[ar.length-1];

    for (j = ar.length-2; j > -1; j--){
        if (ar[j] > i){
            ar[j+1] = ar[j];
            printArray(ar);
        } else {
            break;
        }
    }
    ar[j+1] = i;
    printArray(ar);
}

Or:

static void insertionSort(int[] ar) {
    int i = ar[ar.length-1];
    int j;
    for (j = ar.length-2; (j >= 0) && (ar[j] > i); j--) {
        ar[j+1] = ar[j];
        printArray(ar);
    }
    ar[j+1] = i;
    printArray(ar);
}

There are some style quibbles, but that's the big stuff.  I'd take the printing out of it, except that the purpose of this function doesn't seem to be "perform an insertion sort", but rather "show how an insertion sort inserts an element".  I don't see any advantage to using a binary search, since that only helps you find the insertion point, and you still need to iterate over all elements > ar[i] to move their values.


Copy a stream to a file using Universal .NET I have answered to a question on Stackoverflow where Andy struggled to cancel a Stream copy to file operation using the Universal .NET Framework:

Apparently, cancelling the cts CancelOperation token had not the expected behavior.

await httpResponseContent.Content.WriteToStreamAsync(streamToWrite.AsOutputStream()).AsTask(cts, progressDownload);

So I suggested to do it another way using a chunks-based stream reading and writing. So I provided this code which has been tested and accepted:

async Task Download(Uri uriToWork, CancellationToken cts, IProgress<int> progress) {
    using(HttpClient httpClient = new HttpClient()) {

        var chunkSize = 1024;
        var buffer = new byte[chunkSize];
        int count = 0;
        string fileToWrite = Path.GetTempFileName();

        using(var inputStream = await httpClient.GetInputStreamAsync(uriToWork)) {
            using(var streamToRead = inputStream.AsStreamForRead()) {
                using(Stream streamToWrite = File.OpenWrite(fileToWrite)) {
                    int size;
                    while((size = await streamToRead.ReadAsync(buffer, 0, chunkSize, cts).ConfigureAwait(false)) > 0) {
                        count += size;
                        await Dispatcher.RunAsync(CoreDispatcherPriority.Normal, () => progress.Report(count));
                        // progress.Report(count);
                        await streamToWrite.WriteAsync(buffer, 0, size, cts).ConfigureAwait(false);
                    }
                }
            }
        }
    }
}

Although it works, I am not that familiar with the Universal .NET and I am not sure this code is optimal. A casual .NET solution would be shorter.

I am very sensitive about code quality and canonicity and I wonder whether some parts of this code are redundant and can be removed, and I would like it to match the most "Universal .NET" oriented as possible.

I will first focus on your nested using statements, as that can be very problematic. As you can read on the help page of CA2202 (CA2202: Do not dispose objects multiple times) the suggested way to resolve this is the following:

  To resolve this issue, use a try/finally block instead of the outer using statement. In the finally block, make sure that the stream resource is not null.

So let's map this to your code. You nest four different using statements, which is extremely problematic. Not only can this result in failures, it makes the code unnecessarily hard to read and comprehend. Refactor it to make use of the try-finally pattern. At most use one using statement
Additionaly I am unsure about your use of Dispatcher.RunAsync. Is there a specific reason for it? You are inside of an async method, meaning you don't have to wrap every method call awkwardly like that. Instead just call the commented-out line of code (and you should remove commented-out code in general).
The use of the var keyword seems strange to me. Why do you use it in this statement:

using(var streamToRead = inputStream.AsStreamForRead())

but not in this:

using(Stream streamToWrite = File.OpenWrite(fileToWrite))

I would generally discourage use of the var keyword if you don't explicitly state the type you define in a declaration. That means this line is okay:

var buffer = new byte[chunkSize];

But you should refactor the rest of your code to explicitly define the types of the variables.
Is there a specific reason to define some variables towards the top of the method and others at the place of use? Also why are they not defined at the very beginning of your method? You should decide on one approach. I prefer defining them right before they are used. This would e.g. mean moving int count = 0 down:

int count = 0;
int size;
while((size = await streamToRead.ReadAsync(buffer, 0, chunkSize, cts).ConfigureAwait(false)) > 0)


Produces a string based on rules I was recently playing around with some code, and someone mentioned that my code uses too many classes. I didn't realize I've been doing ES6 incorrectly for so long.

I'm having a hard time finding a way to organize my code in a way that reads nicely. I say this coming from a C# background.

Thanks for your time, everyone.

class Producer {
    constructor(starterString) {
        this.rules = [];
        this.createRules();
        this.producedString = starterString;
    }
    createRules() {
        this.rules.add(new RuleAddE());
        this.rules.add(new RuleAddNumberOne());
    }
    produce() {
        for(var i = 0, len = this.rules.length; i < len; i++) {
            this.producedString = this.rules[i].run(this.producedString);
        }

        return this.producedString;
    }
}

class Rule {
    constructor() {
        this.applied = false;
    }
    run() {
    }
}

class RuleAddE extends Rule {
    constructor() {
        super();
    }

    run(stringToChange) {
        this.applied = true;
        return stringToChange + "e";
    }
}

class RuleAddNumberOne extends Rule {
    constructor() {
        super();
    }

    run(stringToChange) {
        this.applied = true;
        return stringToChange + "1";
    }
}

var producer = new Producer("aa 1hdheb");

console.log(producer.produce());

How could I improve this code with more proper structure?

1. You have classes but you do not effectively need them because they do not keep any state (even the applied field is actually unused). In C# you do not have much options (unless they're local) but in JavaScript you can use functions anywhere:

const ruleAddE = (text) => text + "E";
const ruleAddNumberOne = (text) => text + "1";

And so on. Now let's go to the core of your algorithm: the Producer class. Do you need e class here? From the code you posted it seems you don't and again a function is enough. Let me assume that rules is accessed from outside the class then it makes sense to keep it in-place.

constructor. I'm not sure if you want to discard starterString, repeately calling produce() will always work on latest produced string. If this is the intended behavior then you can let it as-is.

createRules. If this is a private method I like to prefix it with _. It's not a rule, not everyone agrees with this and - unlike Python - it does not change method visibility/accessibility. I like it because it's self-documenting and I won't, by chance, use externally a private method (which is an implementation detail). You do not even really need this function because code is reduced to:

this.rules = [ ruleAddE, ruleAddNumberOne ];

Note that your original code is broken because there isn't a method Array.add() but Array.push().

produce. This may be simplified, the same way you'd do in C# using some LINQ:

produce() {
    this.producedString = this
        .rules.reduce((text, rule) => rule(text), this.producedString);

    return this.producedString;
}

2. In short, your code might be:

const ruleAddE = (text) => text + "E";
const ruleAddNumberOne = (text) => text + "1";

class Producer {
    constructor(starterString) {
        this.producedString = starterString;
        this.rules = [ ruleAddE, ruleAddNumberOne ];
    }

    produce() {
        this.producedString = this.rules
            .reduce((text, rule) => rule(text), this.producedString);

        return this.producedString;
    }
}

Note that for undefined and null inputs the result is pretty odd (undefined is converted to a literal string) while you may expect an empty string in this case. If that's your case then change declaration to:

this.producedString = starterString || "";

We're not using Rule class hierarchy but, in general, remember that in JavaScript there isn't a (easy) syntax to enforce an object with a specific base class then, unless, it adds something, you can simply drop it.

3. To answer the question about "do you need a class for Producer?": if you don't need to call produce() multiple times on the same object then you don't even need a class and everything may be reduced to:

const ruleAddE = (text) => text + "E";
const ruleAddNumberOne = (text) => text + "1";

function produce(starterString) {
    const rules = [ ruleAddE, ruleAddNumberOne ];
    return rules.reduce((text, rule) => rule(text), starterString);
}

In general in JavaScript you can use classes when you want to carry state and behavior on the same object but for everything else you may well use plain functions (and if you use modules you do not even need to use classes as namespaces).

4. If you're using a class only to give a chance to the caller to specify a different set of rules:

function produce(starterString, rules = [ ruleAddE, ruleAddNumberOne ]) {
    return rules.reduce((text, rule) => rule(text), starterString || "");
}


Generating a random alphanumeric string succinctly and efficiently I want to generate a random alphanumeric string in ruby, as succinctly and efficiently as possible. The following solution works, but is obviously not very efficient.

Please review the following code, bearing in mind that I am trying to keep it as succinct and readable as possible while improving efficiency. Be sure to include a detailed explanation in your answer.

([nil]*8).map { ((48..57).to_a+(65..90).to_a+(97..122).to_a).sample.chr }.join

You could use
Array.new(8){[*'0'..'9', *'a'..'z', *'A'..'Z'].sample}.join

From the documentation for Array:

new(size=0, obj=nil) new(array) new(size) {|index| block }
Returns a new array. In the first form, the new array is empty. In the second it is created with size copies of obj (that is, size references to the same obj). The third form creates a copy of the array passed as a parameter (the array is generated by calling to_ary on the parameter). In the last form, an array of the given size is created. Each element in this array is calculated by passing the element’s index to the given block and storing the return value.

Perhaps it is better to create the array with the valid characters once in advance:
range = [*'0'..'9', *'a'..'z', *'A'..'Z']
Array.new(8){range.sample}.join

I made a Benchmark for the solutions.
require 'benchmark'
N = 10_000 #Number of Test loops

Benchmark.bmbm(10) {|b|

  b.report('Nat'      ) { N.times { ([nil]*8).map { ((48..57).to_a+(65..90).to_a+(97..122).to_a).sample.chr }.join } }
  b.report('tokland') { N.times { 8.times.map { [*'0'..'9', *'a'..'z', *'A'..'Z'].sample }.join } }
  b.report('knut'   ) { N.times { Array.new(8){[*'0'..'9', *'a'..'z', *'A'..'Z'].sample}.join } }
  b.report('Natinit') {
    range = ((48..57).to_a+(65..90).to_a+(97..122).to_a)
    N.times { ([nil]*8).map { range.sample.chr }.join }
  }
  b.report('knutinit') {
      range = ((48..57).to_a+(65..90).to_a+(97..122).to_a)
      N.times { Array.new(8){range.sample}.join }
  }

} #Benchmark

The result:
Rehearsal ---------------------------------------------
Nat         0.765000   0.000000   0.765000 (  0.765625)
tokland     2.172000   0.000000   2.172000 (  2.171875)
knut        1.953000   0.000000   1.953000 (  1.984375)
Natinit     0.063000   0.000000   0.063000 (  0.062500)
knutinit    0.078000   0.000000   0.078000 (  0.078125)
------------------------------------ total: 5.031000sec

                user     system      total        real
Nat         0.781000   0.000000   0.781000 (  0.781250)
tokland     1.953000   0.000000   1.953000 (  1.968750)
knut        1.922000   0.000000   1.922000 (  1.921875)
Natinit     0.063000   0.000000   0.063000 (  0.062500)
knutinit    0.078000   0.000000   0.078000 (  0.078125)

@Nat: Gratulation, your version is the fastest ;)
To analyze the fastest way to build the range I used the following benchmark:
Benchmark.bmbm(10) {|b|

  b.report('Natinit') {
    N.times { ((48..57).to_a+(65..90).to_a+(97..122).to_a) }
  }
  b.report('knutinit') {
      N.times { [*'0'..'9', *'a'..'z', *'A'..'Z'] }
  }
} #Benchmark

Result:
Rehearsal ---------------------------------------------
Natinit     0.093000   0.000000   0.093000 (  0.093750)
knutinit    0.250000   0.000000   0.250000 (  0.250000)
------------------------------------ total: 0.343000sec

    user     system      total        real
Natinit     0.094000   0.000000   0.094000 (  0.093750)
knutinit    0.219000   0.000000   0.219000 (  0.218750)


Arithmetic operation from string I have a string like str = " 4+ 6 * 30";. I have to perform an arithmetic operation on this using C#; My solution to this problem is as follows.

     string temp = " 4 + 6 * 5";
     int firstNaum = 0;
     int secondNum = 0;
     int ThirdNum = 0;
     int finalResults = 0;
    //Spliting strings
    string[] withoutOperator = temp.Split('\t',' ','*' , '+');
    //Iterating strings
    int counter = 0;
    foreach (var res in withoutOperator)
    {
        if (!string.IsNullOrEmpty(res) && counter ==1)
        {
            firstNaum = Convert.ToInt32(res);
        }
        if (!string.IsNullOrEmpty(res) && counter== 4)
        {
            secondNum = Convert.ToInt32(res);
        }
        if (!string.IsNullOrEmpty(res) && counter == 7)
        {
            ThirdNum = Convert.ToInt32(res);
        }
        counter += 1;
    }
    finalResults = firstNaum + secondNum * ThirdNum;

One of the simplest and easiest ways to evaluate math expressions is to use the Compute method of the DataTable class:

DataTable dt = new DataTable();
int answer = (int)dt.Compute("2+(4*3)*4", "");

answer is 50.

One caveat you'll have to trap exceptions to catch invalid expressions.


A primitive folder listing implementation on Windows I am a recent graduate in computer programming and failed so far to get an internship or a job in the business so I am trying my best to learn from online source.
I am trying to implement a dir-like command on Windows, going through a folder and list all files and folder.

What I've learned is to avoid using namespace std; and making the includes in alphabetical orders.

In this code, I am obviously mixing c and c++, good or bad?
I am also using Digital Mars as a compiler, I'm trying to avoid installing heavy IDEs and compilers like VS.

All thoughts and critiques are welcome.

#include <iostream>
#include <io.h>
#include <string>
#include <sys/stat.h>
#include <sys/types.h>
#include <windows.h>

bool DirectoryExists( const char* absolutePath ){

if( _access( absolutePath, 0 ) == 0 ){

    struct stat status;
    stat( absolutePath, &status );

    return (status.st_mode & S_IFDIR) != 0;
}
return false;
}

char* replace(const char *s){

char* p = new char[strlen(s)+1];
int i=0;

for (i=0;s[i];i++)
if (s[i]=='\\')
    p[i] ='/';
else
    p[i]=s[i];

if (p[i-1] == '/')
    p[i]='\0';
else
{
    p[i]='/';
    p[i+1]='\0';
}

return p;
}

void dirc (const char* destpath){

HANDLE hFind;
WIN32_FIND_DATA FindFileData;
char filename[256];
size_t i=1;

 if((hFind = FindFirstFile(destpath, &FindFileData)) != INVALID_HANDLE_VALUE)
 {
  do    {
        sprintf (filename, "echo %d-%s", i, FindFileData.cFileName);
        system (filename);
        i++;
        }
while(FindNextFile(hFind, &FindFileData));

FindClose(hFind);
}
}

int main(int argc, char**argv) {

const char* path = argv[1];
char* fspath;

if (argv[1] == NULL)
    {
    std::cout <<"No path provided"<<std::endl;
    return 0;
    }

else
    if ( DirectoryExists(path) )
        std::cout <<"Provided path is "<<path<<std::endl;
    else
        {
        std::cout <<"Path doesn't exist"<<std::endl;
        return 0;
        }
 fspath = replace(path);
 char* destpath = (char *) malloc (strlen(fspath)+6);
 destpath = strcat (fspath,"*.*");
 dirc (destpath);
 free (destpath);

return 0;
}

Thous shalt not mix C/C++ unless you have a good reason to do so. Jokes aside, using pure C++ will lead more bug-free code (in terms of memory management etc.) especially since your statement

  What I've learned is to avoid using namespace std; and making the includes in alphabetical orders.

implies (to me) you are a newcomer. You're welcome, by the way. That being said, learning C way of doing things is really helpful, especially if you are not familiar with pointers etc. You will experience first hand, say why C++ smart pointers and containers are a thing.

My advice would be to study exactly why

  You should not mix C/C++ unless you have a good reason to do so.

What you are doing with this code is nothing but plain C (except iostream methods). I did not check your code thoroughly though. So why do you want to use C++? If you want to use C++, why do you write C code and compile as C++ (again, except iostream, which can be as well stdio.h)?

Since you are using Digital Mars, for which I can't find any standard compliance information, I am not sure if you can use C++17 STL. If you can, there is a filesystem library which can be used to do what you are trying to accomplish with a few lines of code. If you can't, you can try to find a more up-to-date compiler.


is_dir function for FTP/FTPS connections I'm implementing an is_dir() function to determine if a given item is a directory or a file for FTP/FTPS connections. Currently, I have two methods: one using PHP's FTP wrapper for the is_dir() function and another recommended in the help page for the ftp_chdir() function.

Here's the first one (protocol wrapper):

if ($items = ftp_nlist($this->attributes["link"], $path)) {
    $output = null;

    foreach ($items as $item) {
        if (is_dir("ftp://{$this->attributes["user"]}:{$this->attributes["password"]}@{$this->attributes["host"]}/{$item}")) {
            $output[] = $item;
        }
    }

    $output[] = sprintf("%5.4fs", (microtime(true) - $_SERVER["REQUEST_TIME_FLOAT"]));
    return $output;
}

Here's the second one (chdir implementation):

if ($items = ftp_nlist($this->attributes["link"], $path)) {
    $output = null;
    $current = ftp_pwd($this->attributes["link"]);

    foreach ($items as $item) {
        if (@ftp_chdir($this->attributes["link"], $item)) {
            ftp_chdir($this->attributes["link"], $current);
            $output[] = $item;
        }
    }

    $output[] = sprintf("%5.4fs", (microtime(true) - $_SERVER["REQUEST_TIME_FLOAT"]));
    return $output;
}

The problem I find with these two implementations is they're quite slow (or, at least, I think they should be faster to be used within AJAX calls). Running these two functions and measuring their execution time with the last element in the returned array, these are their timed values (ran them 50 times to get an accurate enough time reading): 7.2312s for method A and 2.4534s for method B.

Which shows the ftp_chdir() implementation is almost 3x faster than using the FTP wrapper. Still... I have the feeling there may be room for improvement.

My development machine is Windows 7 x64 with Apache 2.4.4 (x86), PHP 5.4.13 (x86, thread-safe version) and MySQL 5.6.10 x64 (irrelevant in this case), always keeping up-to-date packages. The FTP server I ran these tests against is another Windows machine but a full-fledged production server (dedicated) on a data center.

Is there any other method to do the same but in a better (and/or faster) fashion?

In case someone is interested, here's a follow up to MarcDefiant's comment about using ftp_rawlist(). It's fast if you're already using it for getting the file list as well. Here's a snippet from the filebrowser.php in my GitHub filebrowser project. It's certainly not on the same level as Flysystem but I enjoyed working on it :).

public function isDir($path,$contents=NULL){
    $name = getPathName($path);
    if($contents === NULL){
        $path = getItemPath($path);
        $contents = $this->scandir($path);
    }
    foreach($contents as $content => $data){
        if($content == $name){
            if($data['type'] == 'directory'){
                return TRUE;
            }
            return FALSE;
        }
    }
    return FALSE;
}

public function scandir($path=NULL){
    if($path === NULL){
        $path = $this->Path;
    }
    if(!$path){
        $path = ".";
    }
    if(is_array($children = @ftp_rawlist($this->conn_id, $path))){
        $items = array();

        foreach($children as $name => $child){
            if(!$name){
                continue;
            }
            $chunks = preg_split("/\s+/", $child);
            list($item['rights'], $item['number'], $item['user'], $item['group'], $item['size'], $item['month'], $item['day'], $item['time']) = $chunks;
            $item['type'] = $chunks[0]{0} === 'd' ? 'directory' : 'file';
            array_splice($chunks, 0, 8);
            $items[implode(" ", $chunks)] = $item;
        }

        return $items;
    }
    return FALSE;
}

function getPathName($path){
    return substr($path,strrpos("/".$path,"/"));
}


Unit testing async functions in node/express I am looking for the cleanest pattern to test middleware that contains async code using Mocha, Node's assert library, and Sinon. Here's a paired-down example of the function I'm testing.

function some_middleware(req,res, next){
    var input = req.body;
    if (some_test(input)) {
        lib.async_function(input)
            .then((returned_object) => {
                res.locals.data = returned_object;
                res.render('stop-list');
            })
            .catch((err) => {
                res.render('message', {message: err})
            })
        }
    else{ next() }
}

None of these functions return the promise returned by lib.async_function(). Returning the promise would make testing in Mocha much easier. We could just change:

...
return lib.async_function(input)
...

we could test like this use:

it('Should responsd set locals.data to fetched object', function(){
    // Stubs -- most of this goes in a beforeEach block but here for the example
    var res = {render: sinon.stub(), locals: {}}
    var req = {body: "returned_value" }
    var next = sinon.stub()
    var async_stub = sinon.stub('lib', async_function)
    async_stub.resolves(fakedata.stoptimes)

    next.throws("Next Shouldn't be called")
    return some_middleware(req, res, next)
    .then((ret) => assert.equals(res.locals.data, "returned_value" ))

})

Question one: Is it a bad practice to insist functions return a value, which isn't required, simply to make testing easier? Are there any side effects of this?

If making all the async middleware functions return a promise is a bad idea, we can still design tests, but they start to get a little wordy. For example, we could use the stubbed out res.render() and test like this:

it('Should responsd set locals.data to fetched object', function(done){
    // Stubs
    var res = {locals: {}}
    var req = {body: "returned_value" }
    var next = sinon.stub()
    var async_stub = sinon.stub('lib', async_function)
    async_stub.resolves(fakedata.stoptimes)

    next.throws("Next Shouldn't be called")
    res.render = result => {
        try {
            assert.equals(res.locals.data, "returned_value" )
            done()
        } catch (e) {done(e)}
    }
    some_middleware(req, res, next)

})

We need to wrap the assert in a try/catch block otherwise done() never gets called when the test fails and the test times-out. This seems like a lot of code for such a simple test. Which leads to:

Question two: is there a better way or an accepted best-practice that gracefully handles test failures?

Is it a bad practice to insist functions return a value, which isn't required, simply to make testing easier? Are there any side effects of this?

I wouldn't say it's bad practise, having to provide anchors for test purposes isn't uncommon. Side affects would really boil down to how & where it's being used, in this case it looks very much like Express middleware and nowhere in the docs does it mention any rules about not returning values. Furthermore, if you dig into the source you'll find that middleware return values are ignored.

I think in your case returning a Promise from the middleware is the ideal solution.


Singleton with readonly parameters The goal is to create a Singleton and pass it a parameter that is required for the construction and initialization of the class, then preventing any changes to be made to the passed parameter (just like a readonly field being set by an argument passed to a constructor).

For instance:

Sockets
Hosts
Databases
Repositories
(Any instance that requires at least one argument in order to construct)

  I am having a tough time coming to terms with this design, and I am quite certain that there is a pitfall or a loose-end to this implementation of a Singleton combined with a Builder Pattern, to mimic readonly fields set by constructor arguments.

Example Implementation

In this example, I am trying to get a Singleton of Host, where I would like the enum EnvironmentTypes to be treated like a readonly field usually found in classes that have parameters passed into the constructor.

EnvironmentTypes Enum

public enum EnvironmentTypes
{
    Production,
    Staging,
    Development
}

IHost Interface

public interface IHost
{
    string Name { get; set; }
}

Host Class

public sealed class Host : IHost
{
    #region Singleton
    private static readonly Lazy<Host> _instance = new Lazy<Host>(() => new Host());
    public static Host Instance { get { return _instance.Value; } }
    #endregion

    private static bool _isInstantiated;
    private static EnvironmentTypes _environment;
    private string _name;

    internal static EnvironmentTypes Environment
    {
        get { return _environment; }
        internal set
        {
            if (_isInstantiated) throw new InvalidOperationException(nameof(_environment) +" cannot be set once an instance is created.");

            _environment = value;
        }
    }

    public string Name
    {
        get { return _name; }
        set { _name = value; }
    }

    static Host()
    {
        _isInstantiated = false;
        _environment = EnvironmentTypes.Production;
    }

    private Host()
    {
        _isInstantiated = true;
        _name = "My Server";
    }
}

HostBuilder Class

public sealed class HostBuilder
{
    private readonly EnvironmentTypes _environment;
    private string _name;

    public HostBuilder(EnvironmentTypes environment)
    {
        _environment = environment;
    }

    public HostBuilder SetName(string name)
    {
        _name = name;
        return this;
    }

    public IHost Build()
    {
        Host.Environment = _environment;
        Host host = Host.Instance;
        host.Name = _name;

        return host;
    }
}

Implementation

class Foo
{
    void UsingTheBuilder()
    {
        // probably over-kill
        HostBuilder builder = new HostBuilder(EnvironmentTypes.Development)
            .SetName("Bingo");

        IHost host = builder.Build();

        //host.Environment is not available, great!
        host.Name = "Renamed Server"; // works as expected.
    }

    void ManualConfiguration()
    {
        Host.Environment = EnvironmentTypes.Development;
        Host host = Host.Instance;
        host.Name = "Bingo";

        Host.Environment = EnvironmentTypes.Staging; // throws! Hoped to prevent
                                                     // the developer from doing this.
    }
}

Random Notes: It would be great if I could restrict access from getting to the static properties of Host, so that I can totally avoid anyone trying to set the Host.Environment static property and throwing an exception -- note how the HostBuilder shields that from happening as it is a readonly field.

Disclaimer: I am biased towards singletons. I think it is an anti-pattern, that has no place in modern C#.

First, here is a great article on how singletons become a disaster when you try to unit test a code, that heavily relies on them. Your case is even more complex, because you also have to initialize additional parameters. And you can't change those. So you can't test Host with different "environments" unless you try to bypass your own exception with reflection.

I would just register non-static Host class as singleton inside IoC container, and be done with it. It will solve all your problems:

Parameters of Host are no longer exposed.
Container guaranties, that there is going to be a single instance of Host.
Host is exposed as service (IHost) and not as implementation (Host).
You can mock IHost in unit tests.
You can easily unit-test Host implementation with whatever parameters you want, because now it has public constructor and can be re-created as often as it is required by your tests.
Classes that depend on IHost will now require it as dependency, instead of secretly accessing it via global static property.


Find associated object if exists, otherwise generate it I have a simplified model that looks like this:

class Player < ActiveRecord::Base
  has_many :picks

  def pick_for_game(game)
    id = game.instance_of?(Game) ? game.id : game
    pick = picks.find {|pick| pick.game_id == id}
    if !pick
      pick = picks.build(:game_id => id)
    end
    pick
  end
end

The pick_for_game method usually is called a bunch of times when an action is executed. What would be a good way to make this code work efficiently with a list of games?

You can use find_or_initialize_by dynamic finder, see a guide here

This would be equivalent to your code:

def pick_for_game(game)
  game_id = game.instance_of?(Game) ? game.id : game
  picks.find_or_initialize_by_game_id(game_id)
end

Hope it helps you!.


fix ugly initialisation in while loop over queue I have a loop structure, that I really don't like. It's the initialisation I find appalingly ugly. Is there a nicer way to do the following?

Queue<Integer> queue = getMyQueue();
Integer element = queue.poll();
while (element != null){
  calculate_smart_stuff(element);
  element = queue.poll();
}

A for loop is slightly better, but still has the ugly repetition of the assignment of element in the initialiser and the 'incrementer' I also prefer my for loops to be really easy, and only to be reserved for counting and adding (or substracting in corner cases) 1, as this is how I expect a for loop to behave.

Queue<Integer> queue = getMyQueue();
for (Integer element = queue.poll(); element != null; element = queue.poll()){
  calculate_smart_stuff(element);
}

Or should I just keep checking the size before assignment like so?

Queue<Integer> queue = getMyQueue();
while (queue.size() > 0){
  Integer element = queue.poll();
  calculate_smart_stuff(element);
}

What I don't like about this is that I have to check the size every time before getting an element, which itself has a clear indication that the queue was empty.

Are there any better constructs I can use here, or am I just being picky with the last two options?

It would be better to do

for (Integer element; (element = queue.poll) != null;){

}

Because you don't want element to be in the scope after the loop has ended


Merging two XMLs together in Python I have these two XMLs that I want to merge together based on these conditions:

If any of the element is more than 7 weeks old, it has to be deleted.
If any of the element is newer with the same date (regardless of time), take everything from the new XML.

Sample XML:

<programme start="20150317173000 -0000" stop="20150317180000">
   <something else />
</programme>

I use start field to check if the element should be deleted or not.

old_xml = ET.fromstring(old)
new_xml = ET.fromstring(new)

new_programmes = new_xml.findall('programme')

dates_to_delete = set()
programmes_to_add = []

for new_programme in new_programmes:
    start = new_programme.get('start')
    new_prog_st_t = datetime.datetime.strptime(start, DATETIME_FORMAT)
    new_prog_str_t = new_prog_st_t.strftime('%Y%m%d')

    dates_to_delete.add(new_prog_str_t)
    svn_wks = datetime.datetime.now(TIMEZONE) - datetime.timedelta(weeks=7)
    is_less_than_7_weeks = new_prog_st_t > svn_wks

    if is_less_than_7_weeks:
        programmes_to_add.append(new_programme)

all_programmes = old_xml.findall('programme')
for programme in all_programmes:
    start = programme.get('start')
    start_time = datetime.datetime.strptime(start, DATETIME_FORMAT)
    svn_wks = datetime.datetime.now(TIMEZONE) - datetime.timedelta(weeks=7)
    is_older_than_7_weeks = start_time < svn_wks

    if is_older_than_7_weeks:
        old_xml.remove(programme)
    else:
        start_st_t = start_time.strftime('%Y%m%d')
        if start_st_t in dates_to_delete:
            old_xml.remove(programme)

for p in programmes_to_add:
    old_xml.append(p)

return old_xml

Eliminate for loops where you can

You can probably be more aggressive with list comprehensions. I would replace your for programme in all_programmes with a list comprehension that calculated the date and created a list of indices to delete (but I suspect many others would vehemently disagree with this advice):

enumerated_programmes = enumerate(all_programmes)
to_keep = [i for i,p in enumerated_programmes if datetime.datetime.strptime(p.get('start'), DATETIME_FORMAT) < svn_wks]
...
all_programmes = [all_programmes[i] for i in to_keep]

You can make a second list of indices that should be replaced with a newer record, using similar logic.

Naming

Why not use old_programmes as a name instead of all_programmes to make the binary nature of the code clear?

Confusing code

Why is there a return statement at the bottom of your code? I didn't see any def statements so it's not clear what/why you're returning/

Use available Python functions

There is no need to combine append with a for loop. Instead you can extend one list with another. Say you have list a and list a. Use a.extend(b) rather than appending each element of b. The code will be clearer, shorter, and more efficient.
Similar to my advice above about using list comprehensions more aggressively, you could also using filter rather than if, else statements. This would take more logic out of your for loops, which is always a good thing.

DRY

You compute svn_weeks in each iteration of each of your for loopsbut it's a constant for whatever day you are running it. You should compute it once, before either of your for loops.


How can I improve this Makefile? I am working on creating a Makefile for my ProjectEuler solutions. I need to compile each solution into a separate binary for personal timing purposes. Is it possible to pass the name of the rule to the -o option to remove some repeated code?

CXX = g++
CXXFLAGS = -std=c++0x
INC = -Iinclude/ -I../tools/

binaries=01sum35\
     02evenfibs\
     03largestprimefactor\
     04largestpallyproduct\
     05smallestmultiple\
     06sumsquaredifference\
     07tenthousandfirstprime\
     08largestproductinseries\
     09specialpythagtriplet

all: $(binaries)

01sum35:
    $(CXX) $(CXXFLAGS) 01sum35.cc $(INC) -o 01sum35

02evenfibs:
    $(CXX) $(CXXFLAGS) 02evenfibs.cc $(INC) -o 02evenfibs

03largestprimefactor:
    $(CXX) $(CXXFLAGS) 03largestprimefactor.cc $(INC) -o 03largestprimefactor

04largestpallyproduct:
    $(CXX) $(CXXFLAGS) 04largestpallyproduct.cc ../tools/tools.cc $(INC) -o 04largestpallyproduct

05smallestmultiple:
    $(CXX) $(CXXFLAGS) 05smallestmultiple.cc $(INC) -o 05smallestmultiple

06sumsquaredifference:
    $(CXX) $(CXXFLAGS) 06sumsquaredifference.cc $(INC) -o 06sumsquaredifference

07tenthousandfirstprime:
    $(CXX) $(CXXFLAGS) 07tenthousandfirstprime.cc ../tools/tools.cc $(INC) -o 07tenthousandfirstprime

08largestproductinseries:
    $(CXX) $(CXXFLAGS) 08largestproductinseries.cc ../tools/tools.cc $(INC) -o 08largestproductinseries

09specialpythagtriplet:
    $(CXX) $(CXXFLAGS) 09specialpythagtriplet.cc $(INC) -o 09specialpythagtriplet

clean:
    rm -f $(binaries)

Yes:

CXX       = g++
#
# Notice the +=.  Use CPPFLAGS rather than INC for pre-processor rules.
#
CPPFLAGS += -Iinclude/ -I../tools/
CXXFLAGS += -std=c++0x

#
# Move the backslash
# This will stop it looking like a directory at first glance.
binaries=01sum35              \
     02evenfibs               \
     03largestprimefactor     \
     04largestpallyproduct    \
     05smallestmultiple       \
     06sumsquaredifference    \
     07tenthousandfirstprime  \
     08largestproductinseries \
     09specialpythagtriplet

all: $(binaries)

clean:
    rm -f $(binaries)

# Default rules and dependencies works for all the targets
# except the following which need one more file (dependency)

04largestpallyproduct:      04largestpallyproduct.cc    ../tools/tools.cc
07tenthousandfirstprime:    07tenthousandfirstprime.cc  ../tools/tools.cc
08largestproductinseries:   08largestproductinseries.cc ../tools/tools.cc


Use header application/x-download Zend framework 1. Physically, the file does not exist, it is created based on database.

Okay so use header?

public function downloadAction()
{
    $id = $this->getRequest()->getParam("id");
    $optin = new Application_Model_Optin();
    //$data = $optin->getCodesFromDownload($id);
    $data = array(
        array('code' => 12345),
        array('code' => 12346),
        array('code' => 12347),
        array('code' => 12348),
        array('code' => 12349)
    );

    header("Content-type: application/x-download");
    header("Content-disposition: attachment; filename=code_" . $id . ".csv");

    foreach($data as $item)
    {
        echo $item['code']."\r\n";
    }
    die;
}

Yes. It is perfectly acceptable.

This Stack Overflow post demonstrates that the header is simply used for naming the file.


Find number of divisors for integer and sum of integers This function computes and returns the number of divisors an integer has. But it is very slow and I believe it can be optimized for speed.

unsigned long long int find_no_of_divisors(unsigned long long int myno,FILE *ifp)
{

    unsigned long long int divsrs = 2;
    unsigned long long int k = 2;

    if(myno == 1)
    {
        return 1;
    }

    if(myno == 2)
    {
        return 2;
    }

    while(1)
    {
        if((myno % k) == 0)
        {
            divsrs++;
            if(divsrs == MY_ULLONG_MAX)
            {
                printf("Overflow detected in the calculated value for divisors of a number... Exiting");
                fclose(ifp);
                exit(-1);
            }
        }

        k++;

        if(k > (myno/2))
        {
            break;
        }
    }

    return divsrs;
}

This one computes and returns the sum of first n integers:

unsigned long long int next_no(unsigned long long int idx,unsigned long long int cur_no,FILE *ifp)
{
    unsigned long long int next_no;

    next_no = ((idx)*(idx + 1))/2;

    if((next_no - idx) != cur_no)
    {
        printf("Overflow detected in the value of the calculated number... Exiting");
        fclose(ifp);
        exit(-1);
    }
    return next_no;
}

Could there be any glitches or functionality issues? (PS - I did not want to use a sqrt() function due to my portability constraints.)

The first function can be sped up by just iterating up to sqrt(myno) and adding 2 instead of 1 to divsrs when k divides myno unless k is the square root of myno (in which case you would only add 1).

The second function already takes constant, but you can get rid of the multiplication and division, by simply returning cur_no + idx and using cur_no < ULLONG_MAX - idx to check whether there'd be an overflow.

Now a couple of style notes on the code:

First of all, it seems strange to me to pass the file handle to the functions, if they don't actually write to (or read from) the file. I see that you did so you could close the file before exiting the program in case of an overflow. I would rather recommend that you return an error value when an overflow occurs and then close the file and exit in the calling code.

Further your while(1) loop in find_no_of_divisors is actually a pretty plain for-loop and should be written as one. No need for complicated control flow for something so simple.

Lastly I would not recommend dropping (some) vowels from variable names like you do in divsrs - that just leads to typos.


Compare the index of each char in a string in an alphabet array to a range of numbers I saw this codegolf challenge and I set out to try and write a solution for it. I'm nowhere near an expert codegolfer (or programmer), but it was an interesting exercise.

Now I'm wondering how to improve my code, as it feels really bulky (especially compared to some of the answers to the challenge). Note that I'm not looking for ways to golf this code, I'm merely looking for general improvements and optimizations, hints and tips.

I hope the title is clear as to what the program does, it was pretty hard to describe!

Anyway, what my program does is pretty simple:

It accepts a string as input
It then fetches the index of each char in the string from the array
It checks if that char is within a certain range
Each range corresponds with a classic mobile phone button (0,1,2 = A,B,C)
The current button is compared to the previously used button
If the buttons match, the string does not pass and it returns false

My code:

int previousButton = -1;

char[] letters =
{
    'a', 'b', 'c',       // 2
    'd', 'e', 'f',       // 3
    'g', 'h', 'i',       // 4
    'j', 'k', 'l',       // 5
    'm', 'n', 'o',       // 6
    'p', 'q', 'r', 's',  // 7
    't', 'u', 'v',       // 8
    'w', 'x', 'y', 'z',  // 9
    ' '                  // 0
};

for (int i = 0; i < line.Length; i++) {
    char currentLetter = line[i];
    int index = Array.IndexOf(letters, currentLetter);
    int currentButton = 1;

    if (index >= 0 && index <= 2) {
        currentButton = 2;
    } else if (index >= 3 && index <= 5) {
        currentButton = 3;
    } else if (index >= 6 && index <= 8) {
        currentButton = 4;
    } else if (index >= 9 && index <= 11) {
        currentButton = 5;
    } else if (index >= 12 && index <= 14) {
        currentButton = 6;
    } else if (index >= 15 && index <= 18) {
        currentButton = 7;
    } else if (index >= 19 && index <= 21) {
        currentButton = 8;
    } else if (index >= 22 && index <= 25) {
        currentButton = 9;
    } else if (index == 26) {
        currentButton = 0;
    }

    if (previousButton == currentButton) {
        return false;
    }

    previousButton = currentButton;
}

return true;

There are two alternatives I can recommend, one alternative avoids all the if/else/if cascading, and replaces it with a single 'switch' statement. Switch statements are optimized at compile time so that, effectively, each char/operation takes as long as any other. it makes sense to extract that to a function too.
The second alternative is to trade code space, for memory space.
First though, improving your current solution...
Current version
Your current version has a lot of unnecessary conditions in the cascading if/else system.
If you check for 'low' values first, then you can assume the next value's lower range is already handled. It is easier to explain this by example.... you have:

if (index >= 0 && index <= 2) {
    currentButton = 2;
} else if (index >= 3 && index <= 5) {
    currentButton = 3;
} else if (index >= 6 && index <= 8) {

But this will produce the same results, with half the comparisons:
if (index < 0)
{
    currentButton = 1; //invalid index, char not found?
}
else if (index <= 2)
{
    currentButton = 2;
}
else if (index <= 5)
{
    currentButton = 3;
}
else if (index <= 8)
{

Note how I have also used the conventional C# style there for the braces....
1. Switch:
A switch statement can work on chars:
switch (currentLetter)
{
    case 'a':
    case 'b':
    case 'c':
        return 2;
    case 'd':
    case 'e':
    case 'f':
        return 3;
    .....
    default:
        return 1; // whatever you need for an invalid char
}

The above code (included in a function) will encode each char to a key, and unmapped chars will return 1.
2. In memory lookup
A second common way to do this is to prepopulate an array with the indexes for each char:
int[] keys = new int[]{2,2,2,3,3,3,4,4,4,5,5,5,6,6,6,7,7,7,7,8,8,8,9,9,9,9};

Then use that array as a simple lookup:
if (currentChar < 'a' || currentChar > 'z')
{
    return 1;
}
return keys[currentChar - 'a'];

That converts the char to an offset in the keys array. I have taken the liberty of coding this up in Ideone to ensure it works (which it does, except for the handling of space characters)


Implementing Caesar Cipher in C I am trying to implement a "Caeser cipher" to every lower case character in a string, by using a number to control the amount of character rotation. Here is an algorithm that I came up with. Can you please review and give me some feedback?

#include <stdio.h>
int main()
{
    char string4[80];
    int  rotatorN;

    printf("Enter String: ");
    gets(string4);
    printf("Enter Number: ");
    scanf("%i", &rotatorN);

    int n = 0, rotateSwap = 0;

    int i;
    for(i=0; string4[i] != '\0'; i++)
     {
          if(string4[i] >='a' && string4[i] <='z')
             {
                n = 'z' - string4[i];
                if( rotatorN > n )
                 {
                    rotateSwap = rotatorN - n - 1;
                    string4[i] = 'a';
                    string4[i] += rotateSwap;
                  }
               else
                string4[i] += rotatorN;

               }
       }

      int j;
      for(j = 0; string4[j] !='\0'; j++)
        {
             printf("%c", string4[j]);
        }

        printf("\n");

        return 0;
}

You should never use gets() to read keyboard input, ever. Use fgets(), where you specify the max size of the buffer. That way, you are protecting yourself from somebody hacking your code.

You don't check input values. It is good practice to not assume a benevolent keyboard user, so, for instance, when a user enters rotatorN = -12128, you get all kinds of interesting behavior in your program.

Using scanf for input is hard to get right and therefore error-prone. Instead, use fgets for all input and then use sscanf to get the contents.

Check the calculated value for rotateSwap so that it falls within the expected range.


True or false quiz logic I am creating a true or false quiz game and when the user chooses an answer a tick or cross will appear on the screen telling the user whether they are right or wrong and then automatically bring the next question in.

Currently I am using 2 almost identical functions that draw the next question from my question array (one for each button). It appears to work but seems very cluttered and illogical. Is there a more elegant way to code the logic to this?

Also, the concept of a previous question button has caused a lot of issues. Through a lot of research I am still not sure how I would incorporate one into the program.

@IBAction func trueButton(sender: AnyObject) {
    delay(2) {
        nextQuestionTrue()
    }
}

@IBAction func falseButton(sender: AnyObject) {
    delay(2) {
        nextQuestionFalse()
    }
}

func nextQuestionTrue() -> QuizQuestion {
    // initialize current question

    var currentQuestion: QuizQuestion = QuizQuestion(question: "", answer: false, explanation: "")

    if currentIndex < questions.count {
      currentQuestion =  questions[currentIndex]
    }

    if currentQuestion.answer == true {
        answerLabel.text = "correct"

    } else if currentQuestion.answer == false {
        answerLabel.text = "Incorrect"
    }

    questionLabel.text = currentQuestion.question
    currentIndex++
    return currentQuestion
}

func nextQuestionFalse() -> QuizQuestion {
    // initialize current question

    var currentQuestion: QuizQuestion = QuizQuestion(question: "", answer: false, explanation: "")

    if currentIndex < questions.count {
        currentQuestion =  questions[currentIndex]
    }

    if currentQuestion.answer == false {
        answerLabel.text = "correct"

    } else if currentQuestion.answer == true {
        answerLabel.text = "Incorrect"
    }

    questionLabel.text = currentQuestion.question
    currentIndex++
    return currentQuestion
}

Personally I would combine the two @IBActions into a single one and identify the pressed button from the sender object.

I would also recommend you to look into the Ternary Conditional Operator, which can make your simple if statements much cleaner, like the following:

if currentQuestion.answer == true {
    answerLabel.text = "correct"
} else if currentQuestion.answer == false {
    answerLabel.text = "Incorrect"
}

// The previous is exactly the same as the following:
answerLabel.text = (currentQuestion.answer == true) ? "correct" : "Incorrect"

You can read more about the Ternary Conditional Operator here.

You can see my result from trying to recreate your current setup as inspiration (FYI, my buttons have the titles "True" and "False", which I make use of to turn them into booleans):

//
//  ViewController.swift
//  TrueFalseQuiz
//
//  Created by Stefan Veis Pennerup on 06/06/15.
//  Copyright (c) 2015 Kumuluzz. All rights reserved.
//

import UIKit

class ViewController: UIViewController {

    // MARK: - Question models
    // Notice: This is not the best practice for storing the model in terms of the MVC pattern,
    // this is just for illustration purposes.
    let questions = [
        QuizQuestion(question: "Do I like coffee?", answer: true, explanation: "Because it's awesome!"),
        QuizQuestion(question: "Is bacon god's gift to mankind?", answer: true, explanation: "Because it's awesome!"),
        QuizQuestion(question: "Should I take a nap right now?", answer: false, explanation: "You gotta review some code!")]
    var currentQuestionIndex = 0

    // MARK: - Storyboard outlets

    @IBOutlet weak var questionLabel: UILabel!
    @IBOutlet weak var answerLabel: UILabel!

    // MARK: - Lifecycle methods

    override func viewDidLoad() {
        super.viewDidLoad()

        // Initializes the first question
        self.answerLabel.text = ""
        self.questionLabel.text = questions[currentQuestionIndex].question
    }

    // MARK: - Storyboard actions

    /**
    The target action for both the true false buttons
    */
    @IBAction func answerPressed(sender: UIButton) {
        // Exits if there aren't any questions left
        if currentQuestionIndex >= questions.count { return }

        // Retrieves the user's answer and figures out if it correct
        let userAnswer = sender.currentTitle
        let isAnswerCorrect = userAnswer?.toBool() == questions[currentQuestionIndex].answer

        // Prints appropiate message
        answerLabel.text = (isAnswerCorrect) ? "Correct" : "Incorrect"

        // Updates with a new question
        currentQuestionIndex++
        let isThereAnyQuestionsLeft = currentQuestionIndex < questions.count
        questionLabel.text = (isThereAnyQuestionsLeft) ? questions[currentQuestionIndex].question : "No more questions"
    }
}

// Extends the standard String class to convert a string to a boolean
extension String {
    func toBool() -> Bool? {
        switch self {
        case "True", "true", "yes", "1":
            return true
        case "False", "false", "no", "0":
            return false
        default:
            return nil
        }
    }
}

Could you describe a bit more about the "previous question" button? Should it replace the current question? Should it show the user's answer or just start over with the question?

UPDATE 1:

This update is concerning the "previous question" funcionality outlined by OP in the comments of this answer.

Regarding next and back:

I would add two new buttons (a "next" and "back" button, surprise, surprise) and create two separate storyboard actions for each of them. Each method would respectively increment or decrement the current question index. Both of the methods would then update the currently displayed question.

// MARK: - Next/back methods

@IBAction func nextPressed(sender: UIButton) {
    currentQuestionIndex++
    if currentQuestionIndex == questions.count { currentQuestionIndex = 0 }
    updateCurrentQuestion()
}

@IBAction func backPressed(sender: UIButton) {
    currentQuestionIndex--
    if currentQuestionIndex < 0 { currentQuestionIndex = questions.count - 1 }
    updateCurrentQuestion()
}

func updateCurrentQuestion() {
    answerLabel.text = ""
    questionLabel.text = questions[currentQuestionIndex].question
}

Regarding submitting all:

In the code outline aboved there are 2 main varibles related to your questions model:

let questions: [QuizQuestion]
var currentQuestionIndex: Int

To handle the feature for storing the user's answers, then I would add another property to your QuizQuestion model to hold the user's answers. This means your model would look something similar to the following:

//
//  QuizQuestion.swift
//  TrueFalseQuiz
//
//  Created by Stefan Veis Pennerup on 06/06/15.
//  Copyright (c) 2015 Kumuluzz. All rights reserved.
//

import Foundation

class QuizQuestion {

    let question: String!
    let answer: Bool!
    let explanation: String!
    var usersAnswer: Bool?

    init(question: String, answer: Bool, explanation: String) {
        self.question = question
        self.answer = answer
        self.explanation = explanation
    }
}

Since usersAnswer is an Optional, then it will initially be nil. Whenever the method answerPressed is called, then you should add the following line to store the answer, thus overwriting nil:
    let userAnswer = sender.currentTitle
    questions[currentQuestionIndex].usersAnswer = userAnswer?.toBool()

I would then add a new button called "finish" or "submit all" and connect the following storyboard action, which determines if all the questions have been answered before continuing.

// MARK: - Finishing

@IBAction func submitAll(sender: UIButton) {
    let hasAnsweredAllQuestions = questions.reduce(true) { (x, q) in x && (q.usersAnswer != nil) }
    println("has user answered all questions?: \(hasAnsweredAllQuestions)")
}

UPDATE 2:

Explanation of reduce for OP.

reduce can be quite hard to grasp initially, but once you understand it, then it can be super powerful. In short, it takes all the values of an array and compute it into a single value (could be an Int, Bool or something else). You can read more about it here: https://stackoverflow.com/questions/28283451/ios-swift-reduce-function.

The line

let hasAnsweredAllQuestions = questions.reduce(true) { (x, q) in x && (q.usersAnswer != nil) }

could be expanded to something similar the following:

var hasAnsweredAllQuestions = true
for q in questions {
    if (q.usersAnswer != nil) {
        hasAnsweredAllQuestions = true
    }
    else {
        hasAnsweredAllQuestions = false
        break;
    }
}

As you can see it simply loops over all the questions and checks if the user has given an answer (if the answer is not nil).

Other functions which are equally powerful as reduce are map and filter which you can try to do more research on.

Regarding checking the user's answer with the correct answer, then you could consider using map, but I'll leave you with that hint for now ;)


Android Global Variable Setup I am using the Android library Retrofit for networking in my app. The library calls for creating a RestAdapter for making service calls. I want to use this same instance of the RestAdapter for all of my service calls. How is my setup for this scenario?

Extending Application:

public class CustomApplication extends Application {
    private RestClient restClient = null;

    public RestClient getRestClient() {
        return restClient;
    }

    public void initRestClient() {
        if (restClient == null) {
            restClient = new RestAdapter.Builder().setEndpoint(BASE_URL).build().create(RestClient.class);
        }
    }
}

Initializing instance and making sure I have access to the instance in all my Activities, without explicitly having to get it for each Activity:

public class BaseActivity extends Activity {
    protected RestClient restClient;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        ((CustomApplication) this.getApplication()).initRestClient();
        restClient = ((CustomApplication) this.getApplication()).getRestClient();
    }
}

Then all my Activities will extend BaseActivity.

I agree with both @mjolka and @kyle-falconer. In fact the two points are very similar, because dependency injection frameworks usually work by giving you singletons by default. Mjolka's suggestion of adopting Dagger will be good long-term solution and definitely worth the time investment. Kyle's suggestion is more of a direct, simple solution you can implement right now.

If you go with Kyle's approach, I would point out the modern way of implementing singletons in Java using an enum, that's simpler and more robust:

enum SingletonRestClient {
    INSTANCE;

    private final RestClient restClient;

    private SingletonRestClient() {
        restClient = new RestAdapter.Builder().setEndpoint(BASE_URL).build().create(RestClient.class);
    }

    public RestClient getRestClient() {
        return restClient;
    }
}

You could use this in an activity like this:

public class SomeActivity extends Activity {
    private RestClient restClient;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        restClient = SingletonRestClient.INSTANCE.getRestClient();
    }
}

Of course, you will have to do this in all your activities that need the rest client.
But this is still better than forcing all your activities to extend a base activity.

Don't use inheritance just to share data.
The right reason to use inheritance is when there is an is-a relationship.
Can you say that SomeActivity is a BaseActivity?
This sounds inevitably wrong, because the very nature of a "base activity" is that there is just one such activity.
A good example would be an EmployeeActivity inheriting from a PersonActivity,
which could make sense, since an employee really is a person.

We don't have more details about your context.
If there is no is-a relationship between the classes involved,
then don't use inheritance, and prefer composition instead:
let each class that needs a rest client get it from the singleton.