1
\$\begingroup\$

This is as much a code design question as a code review. I am adding some temporary code to my website to record search terms used. For now I'm also storing a few other fields. This is an evaluation and if we decide to use the data I will convert this to save to a db (or possibly a persisting Redis store?!).

So far I have added some very basic code to get going

@RequestMapping(value = "search", method = RequestMethod.GET)
public ModelAndView searchSite(HttpServletRequest request, HttpServletResponse response, Model model) {

    WriteToFile_Temp(searchRequestBean.getTerm(), "searchSite"
            , (request.getHeader("X-FORWARDED-FOR") == null) ? request.getRemoteAddr() : request.getHeader("X-FORWARDED-FOR"));

    clearSearchRequest(model);

    ...

which calls

private void WriteToFile_Temp(String searchTerm, String callingFunction, String caller) {

    String logToFolder = System.getProperty("log.dir") + "\\SearchHistory.log" ;

    try (BufferedWriter bw = new BufferedWriter(new FileWriter(logToFolder, true))) {

        String content = searchTerm + "," + callingFunction + "," + LocalDateTime.now().toString() + "," + callerIP + System.getProperty("line.separator");

        bw.write(content);

    } catch (IOException e) {
        e.printStackTrace();
    }
}

This is clearly a quick 'get going' piece of code but I'm wondering what the simplest way to make this resilient might be. Is it enough to follow the advice in this similar c# question (Writing to file in a thread safe manner) and do something like this

namespace test
{   
  public class Writer
  {
    public string Filepath { get; set; }
    private static object locker = new Object();

    public void WriteToFile(StringBuilder text)
    {
        lock (locker)
        {
            using (FileStream file = new FileStream(Filepath, FileMode.Append, FileAccess.Write, FileShare.Read))
            using (StreamWriter writer = new StreamWriter(file, Encoding.Unicode))
            {
                writer.Write(text.ToString());
            }
        }

    }
  }
}

Or do you think I should be doing something more sophisticated. I'm wondering for example whether, if and when I switch to a db model, although there will be less need to worry about concurrency, whether it would be worth firing off the write to file/db write on a different thread or whether, for such a small task, the overhead would be heavier than the task itself...

Apologies this is very open ended but I'm working on my own at the moment and any ideas, validation, criticism would be appreciated!

\$\endgroup\$
  • 2
    \$\begingroup\$ You are trying to implement a sort of low level output into a file, but this resembles much a use case of standard logging features. I was just wondering why a logger (logback, log4j etc), configured properly with destination file and message pattern, is not usable in your case? \$\endgroup\$ – Antot Jun 6 '17 at 13:55
  • \$\begingroup\$ Yes good point. Partly I hadn't twigged this, keeping an eye on the future move to db store & partly I hadn't realised how easy it was to maintain a separate log file. \$\endgroup\$ – gringogordo Jun 7 '17 at 12:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.